From 7c9a6b17f17928a02fceb66cafc9121aa3e9799b Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Mon, 26 Oct 2026 23:04:49 +0000 Subject: [PATCH 01/11] feat(pi): select OhMyPi model presets per thread OhMyPi presets set every role model, but neither Pi RPC nor ACP exposes them. Pi discovery now lists omp presets as models, and a thread on a preset starts omp with a per-session --config overlay, so switching presets never touches omp's global config. --- docs/adr/0025-ohmypi-model-presets.md | 33 +++ docs/user/providers-pi.md | 12 + .../provider-pi/src/server/adapter.test.ts | 132 ++++++++++ packages/provider-pi/src/server/adapter.ts | 109 ++++++++- .../provider-pi/src/server/mcpInjection.ts | 3 + packages/provider-pi/src/server/presets.ts | 227 ++++++++++++++++++ .../provider-pi/src/server/status.test.ts | 57 ++++- packages/provider-pi/src/server/status.ts | 20 +- .../src/server/textGeneration.test.ts | 75 ++++-- .../provider-pi/src/server/textGeneration.ts | 49 +++- 10 files changed, 690 insertions(+), 27 deletions(-) create mode 100644 docs/adr/0025-ohmypi-model-presets.md create mode 100644 packages/provider-pi/src/server/presets.ts diff --git a/docs/adr/0025-ohmypi-model-presets.md b/docs/adr/0025-ohmypi-model-presets.md new file mode 100644 index 0000000000000..f8537a6fb3b9a --- /dev/null +++ b/docs/adr/0025-ohmypi-model-presets.md @@ -0,0 +1,33 @@ +# ADR 0025: Select OhMyPi model presets per thread + +- Status: accepted +- Date: 2026-10-26 +- Compared with upstream: `pingdotgg/t3code` at `6ffde5ba3` (2026-10-26) + +OhMyPi (`omp`) is a Pi fork that T3 runs through the Pi provider. Its model +presets assign models to every role (`default`, `task`, `plan`, `smol`, ...) +and a default thinking level. Neither Pi RPC nor omp's ACP mode exposes them, +so T3 could only change the main model. + +When the Pi binary reports an `omp/` version, discovery reads +`omp config get modelPresets --json` and lists each preset as a model with the +slug `preset:` and a "Preset" qualifier. Its Thinking select is the one +for the preset's main model and starts at the preset's level. + +Applying a preset with omp's own `/modelpreset switch` writes roles to the +user's global config, and every running omp process reloads it, so one thread +would change all the others and the user's terminal. T3 instead starts the +thread's omp process with a `--config` overlay file in T3's cache. The overlay +holds the preset's roles, `null` tombstones for roles the preset leaves out (as +`/modelpreset switch` clears them), and its thinking level. The overlay is +rebuilt from the current preset each time the process starts. Changing which +preset a thread uses returns `restart_session`, so the session resumes on a +new process with the new overlay, and the first turn sends `set_model` for the +preset's `default` role. A preset whose main model is an alias rather than +`provider/model` fails with a clear error. Text generation has no per-model +filter, so a preset chosen there uses only its main model. + +When updating from upstream, keep presets inside the Pi provider and keep the +per-process overlay; do not switch presets by writing omp's global config. +Remove this if Pi or omp exposes presets over RPC, or if upstream adds OhMyPi +support. diff --git a/docs/user/providers-pi.md b/docs/user/providers-pi.md index 123258f6ccb43..a271808a12f8e 100644 --- a/docs/user/providers-pi.md +++ b/docs/user/providers-pi.md @@ -51,6 +51,18 @@ Pi loads its normal user and project extensions. Blocking `select`, `confirm`, ` dialogs work in T3 Code. Notifications appear in the work log. Pi terminal decoration such as titles, status lines, and widgets does not have a T3 Code equivalent. +## OhMyPi Model Presets + +If Pi's binary path points to [OhMyPi](https://github.com/can1357/oh-my-pi) (`omp`), its saved +model presets appear in the model picker labelled **Preset**. Save presets in `omp` with +`/modelpreset save `, then refresh the provider in Settings to see new ones. + +Choosing a preset applies all of its role models and its thinking level to that thread only. Other +threads and `omp` sessions outside T3 Code keep their own models. Switching a thread to another +preset restarts its Pi session and resumes the same conversation. Edits to a preset reach a thread +the next time its session starts. Use the picker rather than `/modelpreset switch`, which changes +OhMyPi's global configuration. A preset's main model must be written as `provider/model`. + ## Permission Modes T3 Code applies the composer permission mode through Pi's blocking tool hook: diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index 544ac5cce66a3..5db04e616d37d 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -428,6 +428,47 @@ const makeAdapter = Effect.fnUntraced(function* ( }).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner)); }); +const OMP_CONFIG_VALUES: Record = { + modelPresets: { + Anthropic: { + modelRoles: { default: "anthropic/claude-opus:high", task: "anthropic/claude-sonnet" }, + defaultThinkingLevel: "auto", + }, + }, + modelRoles: { default: "openai/gpt", designer: "openai/gpt-mini" }, +}; + +/** Answers `omp config get --json` and hands every other spawn to the fake Pi. */ +const makeOmpPresetAdapter = (fake: FakePi) => { + const spawner = ChildProcessSpawner.make((command) => { + const args = ChildProcess.isStandardCommand(command) ? command.args : []; + if (args[0] !== "config") return fake.spawner.spawn(command); + const stdout = new TextEncoder().encode( + encodeJsonLine({ key: args[2], value: OMP_CONFIG_VALUES[args[2] ?? ""] }), + ); + return Effect.succeed( + ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(FAKE_PID), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(0)), + isRunning: Effect.succeed(false), + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + stdin: Sink.drain, + stdout: Stream.succeed(stdout), + stderr: Stream.empty, + all: Stream.empty, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }), + ); + }); + return makePiAdapterV2({ + instanceId: PI_INSTANCE_ID, + settings: { enabled: true, binaryPath: "omp", launchArgs: "", customModels: [] }, + environment: {}, + }).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner)); +}; + const openRuntime = Effect.fnUntraced(function* ( fake: FakePi, model = "default", @@ -1818,6 +1859,97 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); + it.effect("launches an OhMyPi preset with its own role overlay and main model", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + const adapter = yield* makeOmpPresetAdapter(fake); + const runtime = yield* adapter.openSession({ + threadId: THREAD_ID, + providerSessionId: SESSION_ID, + modelSelection: modelSelection("preset:Anthropic"), + runtimePolicy, + }); + const args = fake.lastSpawn().args; + const overlayPath = args[args.indexOf("--config") + 1]; + assert.isDefined(overlayPath); + const overlay = decodeJsonLine( + yield* (yield* FileSystem.FileSystem).readFileString(overlayPath!), + ); + // Roles the preset leaves out are tombstoned so global roles cannot leak in. + assert.deepEqual(overlay, { + modelRoles: { + default: "anthropic/claude-opus:high", + designer: null, + task: "anthropic/claude-sonnet", + }, + defaultThinkingLevel: "auto", + }); + + const providerThread = yield* runtime.ensureThread({ + threadId: THREAD_ID, + modelSelection: modelSelection("preset:Anthropic"), + runtimePolicy, + }); + yield* startTurn(runtime, providerThread, "preset:Anthropic"); + const modelRequest = yield* fake.takeRequest("set_model"); + assert.equal(modelRequest["provider"], "anthropic"); + assert.equal(modelRequest["modelId"], "claude-opus"); + const levelRequest = yield* fake.takeRequest("set_thinking_level"); + assert.equal(levelRequest["level"], "high"); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + + it.effect("fails a preset session that names a missing preset", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + const adapter = yield* makeOmpPresetAdapter(fake); + const error = yield* adapter + .openSession({ + threadId: THREAD_ID, + providerSessionId: SESSION_ID, + modelSelection: modelSelection("preset:Gone"), + runtimePolicy, + }) + .pipe(Effect.flip); + assert.include(error.message, "preset 'Gone' was not found"); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + + it.effect("restarts the session only when the selected preset changes", () => + Effect.gen(function* () { + const adapter = yield* makeAdapter(yield* makeFakePi); + const sessionCapabilities = yield* adapter.getCapabilities(); + const plan = (current: ModelSelection, target: ModelSelection) => + adapter + .planSelectionTransition({ + current, + target, + sessionCapabilities, + }) + .pipe(Effect.map((result) => result.type)); + const thinking = (model: string, value: string): ModelSelection => ({ + ...modelSelection(model), + options: [{ id: "thinking", value }], + }); + assert.equal( + yield* plan(modelSelection("preset:Anthropic"), modelSelection("preset:OpenAI")), + "restart_session", + ); + assert.equal( + yield* plan(modelSelection("xai/grok-4.6"), modelSelection("preset:OpenAI")), + "restart_session", + ); + assert.equal( + yield* plan(modelSelection("preset:OpenAI"), thinking("preset:OpenAI", "low")), + "apply_on_next_turn", + ); + assert.equal( + yield* plan(modelSelection("xai/grok-4.6"), modelSelection("default")), + "apply_on_next_turn", + ); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + it.effect.each([ { historical: false, label: "the latest turn" }, { historical: true, label: "a historical turn" }, diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index aca13e35643ed..9fab97495b511 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -71,6 +71,14 @@ import { resolvePiLaunchArgs, } from "./mcpInjection.ts"; import { PI_FILE_CHANGE_TOOLS } from "./mcpExtensionSource.ts"; +import { + ompPresetThinkingLevel, + parseOmpModelSelector, + encodeOmpConfigOverlay, + piPresetNameFromSlug, + resolveOmpPreset, + type OmpModelPreset, +} from "./presets.ts"; export const PI_PROVIDER = ProviderDriverKind.make("pi"); const PI_DRIVER_KIND = PI_PROVIDER; @@ -432,8 +440,15 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( getCapabilities: Effect.fn("PiAdapter.getCapabilities")(function () { return Effect.succeed(PiProviderCapabilitiesV2); }), - planSelectionTransition: Effect.fn("PiAdapter.planSelectionTransition")(function () { - return Effect.succeed(turnScopedSelectionTransition()); + planSelectionTransition: Effect.fn("PiAdapter.planSelectionTransition")(function (input) { + // A preset's role models live in the launch-time `--config` overlay. + const currentPreset = piPresetNameFromSlug(input.current.model); + const targetPreset = piPresetNameFromSlug(input.target.model); + return Effect.succeed( + currentPreset !== targetPreset + ? ({ type: "restart_session" } as const) + : turnScopedSelectionTransition(), + ); }), openSession: Effect.fn("PiAdapterV2.openSession")(function* ( input: ProviderAdapter.ProviderAdapterV2OpenSessionInput, @@ -461,11 +476,41 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( if (!resolvedLaunchArgs.ok) { return yield* protocolError(resolvedLaunchArgs.message); } + const presetName = piPresetNameFromSlug(input.modelSelection.model); + let sessionPreset: OmpModelPreset | null = null; + let configOverlayPath: string | undefined; + if (presetName !== undefined) { + const resolved = yield* resolveOmpPreset({ + binaryPath: options.settings.binaryPath || "pi", + environment: options.environment, + cwd, + name: presetName, + }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.mapError((cause) => protocolError(cause.message)), + ); + sessionPreset = resolved.preset; + const overlayDir = `${host.paths.providerStatusCacheDir.replace(/\\/g, "/")}/pi-presets`; + const overlayPath = `${overlayDir}/${input.providerSessionId.replace(/[^\w-]/g, "_")}.json`; + // JSON is valid YAML, which is what omp reads `--config` overlays as. + yield* provideCacheFs( + Effect.gen(function* () { + yield* fileSystem.makeDirectory(overlayDir, { recursive: true }); + yield* fileSystem.writeFileString( + overlayPath, + encodeOmpConfigOverlay(resolved.overlay), + ); + }), + ); + yield* Effect.addFinalizer(() => fileSystem.remove(overlayPath).pipe(Effect.ignore)); + configOverlayPath = overlayPath; + } const launch = buildPiRpcLaunch({ launchArgs: resolvedLaunchArgs.args, environment: options.environment, mcpSession, extensionPath, + configOverlayPath, runtimeMode: input.runtimePolicy.runtimeMode, }); const scopedLaunch = yield* agentScope.wrap({ @@ -2242,7 +2287,59 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( return providerThread; }); + const applyPresetSelection = Effect.fnUntraced(function* ( + modelSelection: ModelSelection, + presetName: string, + ) { + if (sessionPreset?.name !== presetName) { + return yield* protocolError( + `OhMyPi preset '${presetName}' needs a session restart to apply`, + ); + } + const thinking = + getModelSelectionStringOptionValue(modelSelection, "thinking") ?? + ompPresetThinkingLevel(sessionPreset); + const defaultSelector = sessionPreset.modelRoles.default; + if (modelSelection.model !== appliedModel) { + // Without a `default` role, omp picks the main model itself, as `/modelpreset switch` does. + if (defaultSelector !== undefined) { + const parsed = parseOmpModelSelector(defaultSelector); + if (parsed === null) { + return yield* protocolError( + `OhMyPi preset '${presetName}' main model '${defaultSelector}' must use provider/model format`, + ); + } + const selectedModel = yield* request({ + type: "set_model", + provider: parsed.provider, + modelId: parsed.modelId, + }); + contextWindow = rememberModelContextWindow(selectedModel); + } + appliedModel = modelSelection.model; + const updatedAt = yield* DateTime.now; + sessionEntity = { ...sessionEntity, model: modelSelection.model, updatedAt }; + yield* emit({ + type: "provider_session.updated", + driver: PI_PROVIDER, + providerSession: sessionEntity, + }); + } + if ( + thinking !== undefined && + thinking !== appliedThinking && + PI_THINKING_LEVELS.has(thinking) + ) { + yield* request({ type: "set_thinking_level", level: thinking }); + appliedThinking = thinking; + } + }); + const applySelection = Effect.fnUntraced(function* (modelSelection: ModelSelection) { + const selectedPreset = piPresetNameFromSlug(modelSelection.model); + if (selectedPreset !== undefined) { + return yield* applyPresetSelection(modelSelection, selectedPreset); + } const thinking = getModelSelectionStringOptionValue(modelSelection, "thinking"); if (modelSelection.model === PI_INHERIT_MODEL_SLUG) { if (appliedModel !== null && baselineModel !== null) { @@ -2339,12 +2436,18 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( ), getModelContextWindow: (selection) => { if (selection.instanceId !== options.instanceId) return undefined; + const presetDefault = + piPresetNameFromSlug(selection.model) === undefined + ? null + : parseOmpModelSelector(sessionPreset?.modelRoles.default ?? ""); const slug = selection.model === PI_INHERIT_MODEL_SLUG ? baselineModel === null ? undefined : `${baselineModel.provider}/${baselineModel.modelId}` - : selection.model; + : presetDefault !== null + ? `${presetDefault.provider}/${presetDefault.modelId}` + : selection.model; return slug === undefined ? undefined : modelContextWindows.get(slug); }, ensureThread: (threadInput) => diff --git a/packages/provider-pi/src/server/mcpInjection.ts b/packages/provider-pi/src/server/mcpInjection.ts index 19165a2ccbeee..31460222eb2ce 100644 --- a/packages/provider-pi/src/server/mcpInjection.ts +++ b/packages/provider-pi/src/server/mcpInjection.ts @@ -243,6 +243,8 @@ export function buildPiRpcLaunch(input: { readonly mcpSession: McpProviderSessionConfig | undefined; readonly extensionPath: string | undefined; readonly sessionPath?: string | undefined; + /** OhMyPi `--config` overlay; later overlays win, so it follows user launch args. */ + readonly configOverlayPath?: string | undefined; readonly ephemeral?: boolean; readonly disableExtensions?: boolean; readonly disableTools?: boolean; @@ -269,6 +271,7 @@ export function buildPiRpcLaunch(input: { ? ["--session", input.sessionPath] : []), ...launchArgs, + ...(input.configOverlayPath === undefined ? [] : ["--config", input.configOverlayPath]), // Restrictions must follow launch args so user flags cannot re-enable unattended tools/extensions. ...(input.disableExtensions === true ? ["--no-extensions"] : []), ...(input.disableTools === true ? ["--no-tools"] : []), diff --git a/packages/provider-pi/src/server/presets.ts b/packages/provider-pi/src/server/presets.ts new file mode 100644 index 0000000000000..ab462f24e0e59 --- /dev/null +++ b/packages/provider-pi/src/server/presets.ts @@ -0,0 +1,227 @@ +import type { ServerProviderModel } from "@t3tools/contracts"; +import { resolveSpawnCommand } from "@t3tools/shared/shell"; +import { spawnAndCollect } from "@t3tools/provider-core/server/snapshotProbe"; +import * as Effect from "effect/Effect"; +import * as Predicate from "effect/Predicate"; +import * as Schema from "effect/Schema"; +import * as ChildProcess from "effect/process/ChildProcess"; + +import { PiRpcError } from "./rpc.ts"; +import { + EMPTY_PI_MODEL_CAPABILITIES, + thinkingCapabilitiesForPiModel, +} from "./thinkingCapabilities.ts"; + +// OhMyPi (`omp`) model presets name a whole set of role models; T3 lists them as models. +const PI_PRESET_SLUG_PREFIX = "preset:"; +const PRESET_NAME_PATTERN = /^[a-zA-Z][\w-]*$/; +const OMP_THINKING_SUFFIXES = new Set([ + "off", + "minimal", + "low", + "medium", + "high", + "xhigh", + "max", + "auto", + "inherit", +]); + +const decodeOmpConfigOutput = Schema.decodeUnknownEffect( + Schema.fromJsonString(Schema.Struct({ value: Schema.optional(Schema.Unknown) })), +); +export const encodeOmpConfigOverlay = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); + +export interface OmpModelPreset { + readonly name: string; + readonly modelRoles: Readonly>; + readonly defaultThinkingLevel: string | undefined; +} + +function piPresetSlug(name: string): string { + return `${PI_PRESET_SLUG_PREFIX}${name}`; +} + +export function piPresetNameFromSlug(slug: string): string | undefined { + return slug.startsWith(PI_PRESET_SLUG_PREFIX) + ? slug.slice(PI_PRESET_SLUG_PREFIX.length) + : undefined; +} + +export function isOmpVersionOutput(output: string): boolean { + return /^omp\/\d/m.test(output); +} + +function isStringRecord(input: unknown): input is Record { + return Predicate.isObject(input) && !Array.isArray(input); +} + +/** Mirrors omp's own preset validation; hand-edited config can hold anything. */ +function parseOmpModelPresets(value: unknown): ReadonlyArray { + if (!isStringRecord(value)) return []; + const presets: Array = []; + for (const [name, raw] of Object.entries(value)) { + if (!PRESET_NAME_PATTERN.test(name) || !isStringRecord(raw)) continue; + const roles = raw.modelRoles; + if (!isStringRecord(roles)) continue; + const modelRoles: Record = {}; + let valid = true; + for (const [role, selector] of Object.entries(roles)) { + if (typeof selector !== "string" || selector.trim() === "") { + valid = false; + break; + } + modelRoles[role] = selector; + } + if (!valid) continue; + const level = raw.defaultThinkingLevel; + presets.push({ + name, + modelRoles, + defaultThinkingLevel: typeof level === "string" ? level : undefined, + }); + } + return presets.toSorted((left, right) => left.name.localeCompare(right.name)); +} + +/** + * Splits an omp role selector (`provider/model[:level]`) into what Pi RPC `set_model` takes. + * shortcut: aliases (`@slow`) and bare model patterns need omp's fuzzy resolver, so they are + * reported as unsupported; add an RPC-side resolution if presets commonly use them. + */ +export function parseOmpModelSelector(selector: string): { + readonly provider: string; + readonly modelId: string; + readonly thinking: string | undefined; +} | null { + const colon = selector.lastIndexOf(":"); + const suffix = colon === -1 ? undefined : selector.slice(colon + 1); + const thinking = suffix !== undefined && OMP_THINKING_SUFFIXES.has(suffix) ? suffix : undefined; + const model = thinking === undefined ? selector : selector.slice(0, colon); + const separator = model.indexOf("/"); + if (model.startsWith("@") || separator <= 0 || separator === model.length - 1) return null; + return { provider: model.slice(0, separator), modelId: model.slice(separator + 1), thinking }; +} + +/** The level a preset starts at: the `default` selector's suffix wins over `defaultThinkingLevel`. */ +export function ompPresetThinkingLevel(preset: OmpModelPreset): string | undefined { + const selector = preset.modelRoles.default; + const fromSelector = + selector === undefined ? undefined : parseOmpModelSelector(selector)?.thinking; + return fromSelector ?? preset.defaultThinkingLevel; +} + +/** + * `--config` overlay content for a preset. Roles the preset leaves out are written as `null` + * tombstones so global roles do not leak in, matching omp's own `/modelpreset switch`. + */ +function ompPresetOverlay( + preset: OmpModelPreset, + effectiveRoles: Readonly>, +): Record { + const modelRoles: Record = {}; + for (const role of Object.keys(effectiveRoles)) modelRoles[role] = null; + Object.assign(modelRoles, preset.modelRoles); + return { + modelRoles, + ...(preset.defaultThinkingLevel === undefined + ? {} + : { defaultThinkingLevel: preset.defaultThinkingLevel }), + }; +} + +export function ompPresetModels( + presets: ReadonlyArray, + modelInventory: unknown, +): ReadonlyArray { + const inventory = isStringRecord(modelInventory) ? modelInventory.models : undefined; + const rawModels: ReadonlyArray = Array.isArray(inventory) ? inventory : []; + return presets.map((preset) => { + const selector = preset.modelRoles.default; + const parsed = selector === undefined ? null : parseOmpModelSelector(selector); + const rawModel = + parsed === null + ? undefined + : rawModels.find( + (model) => + isStringRecord(model) && + model.provider === parsed.provider && + model.id === parsed.modelId, + ); + return { + slug: piPresetSlug(preset.name), + name: preset.name, + subProvider: "Preset", + isCustom: false, + capabilities: + rawModel === undefined + ? EMPTY_PI_MODEL_CAPABILITIES + : thinkingCapabilitiesForPiModel(rawModel, ompPresetThinkingLevel(preset)), + }; + }); +} + +const readOmpConfigValue = Effect.fn("PiPresets.readOmpConfigValue")(function* (input: { + readonly binaryPath: string; + readonly environment: NodeJS.ProcessEnv; + readonly cwd: string | undefined; + readonly key: string; +}) { + const args = ["config", "get", input.key, "--json"]; + const spawnCommand = yield* resolveSpawnCommand(input.binaryPath, args, { + env: input.environment, + }); + const result = yield* spawnAndCollect( + input.binaryPath, + ChildProcess.make(spawnCommand.command, spawnCommand.args, { + env: input.environment, + shell: spawnCommand.shell, + ...(input.cwd === undefined ? {} : { cwd: input.cwd }), + }), + ); + const detail = `\`omp config get ${input.key}\``; + if (result.code !== 0) { + return yield* new PiRpcError({ + operation: "omp_config", + detail: `${detail} exited ${result.code}`, + }); + } + const parsed = yield* decodeOmpConfigOutput(result.stdout).pipe( + Effect.mapError( + (cause) => + new PiRpcError({ + operation: "omp_config", + detail: `${detail} printed invalid JSON`, + cause, + }), + ), + ); + return parsed.value; +}); + +export const readOmpModelPresets = (input: { + readonly binaryPath: string; + readonly environment: NodeJS.ProcessEnv; + readonly cwd: string | undefined; +}) => readOmpConfigValue({ ...input, key: "modelPresets" }).pipe(Effect.map(parseOmpModelPresets)); + +/** Resolves a preset by name and builds its overlay against the workspace's current roles. */ +export const resolveOmpPreset = Effect.fn("PiPresets.resolveOmpPreset")(function* (input: { + readonly binaryPath: string; + readonly environment: NodeJS.ProcessEnv; + readonly cwd: string | undefined; + readonly name: string; +}) { + const [presets, roles] = yield* Effect.all( + [readOmpModelPresets(input), readOmpConfigValue({ ...input, key: "modelRoles" })], + { concurrency: "unbounded" }, + ); + const preset = presets.find((candidate) => candidate.name === input.name); + if (preset === undefined) { + return yield* new PiRpcError({ + operation: "omp_preset", + detail: `OhMyPi model preset '${input.name}' was not found or is malformed`, + }); + } + return { preset, overlay: ompPresetOverlay(preset, isStringRecord(roles) ? roles : {}) }; +}); diff --git a/packages/provider-pi/src/server/status.test.ts b/packages/provider-pi/src/server/status.test.ts index cee6442fc9642..9ff0f5c89cb3b 100644 --- a/packages/provider-pi/src/server/status.test.ts +++ b/packages/provider-pi/src/server/status.test.ts @@ -52,7 +52,10 @@ function piProbeSpawner(version: string) { }); } -const piDiscoverySpawner = (inventory: unknown) => +const piDiscoverySpawner = ( + inventory: unknown, + omp?: { readonly modelPresets: unknown; readonly configCalls: Array }, +) => Effect.gen(function* () { const stdout = yield* Queue.unbounded(); let stdinBuffer = ""; @@ -83,9 +86,15 @@ const piDiscoverySpawner = (inventory: unknown) => ); return ChildProcessSpawner.make((command) => { const args = ChildProcess.isStandardCommand(command) ? command.args : []; + if (args[0] === "config") { + omp?.configCalls.push(args.join(" ")); + return Effect.succeed( + processHandle({ stdout: encodeJsonLine({ value: omp?.modelPresets }) }), + ); + } return Effect.succeed( args.includes("--version") - ? processHandle({ stdout: "pi 0.84.3\n" }) + ? processHandle({ stdout: omp === undefined ? "pi 0.84.3\n" : "omp/18.5.1\n" }) : ChildProcessSpawner.makeHandle({ ...processHandle({}), exitCode: Effect.never, @@ -146,6 +155,50 @@ describe("PiProvider", () => { }).pipe(Effect.provide(NodeServices.layer)), ); + it.effect("lists OhMyPi presets as models that start at the preset's thinking level", () => + Effect.gen(function* () { + const configCalls: Array = []; + const spawner = yield* piDiscoverySpawner( + { + models: [ + { provider: "anthropic", id: "claude-opus", reasoning: true }, + { provider: "openai", id: "gpt" }, + ], + }, + { + configCalls, + modelPresets: { + OpenAI: { modelRoles: { default: "openai/gpt" } }, + Anthropic: { modelRoles: { default: "anthropic/claude-opus:low" } }, + Broken: { modelRoles: { default: "" } }, + }, + }, + ); + const snapshot = yield* checkPiProviderStatus({ ...settings, binaryPath: "omp" }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + ); + assert.deepEqual(configCalls, ["config get modelPresets --json"]); + assert.deepEqual( + snapshot.models.map((model) => [model.slug, model.subProvider]), + [ + ["default", undefined], + ["preset:Anthropic", "Preset"], + ["preset:OpenAI", "Preset"], + ["anthropic/claude-opus", "anthropic"], + ["openai/gpt", "openai"], + ], + ); + const thinking = snapshot.models[1]?.capabilities?.optionDescriptors?.[0]; + assert.equal(thinking?.id, "thinking"); + assert.deepEqual( + thinking?.type === "select" + ? thinking.options.filter((option) => option.isDefault).map((option) => option.id) + : [], + ["low"], + ); + }).pipe(Effect.provide(NodeServices.layer)), + ); + // regression: #18498 a malformed model inventory must not restore stale Pi models it.effect.each([ ["missing models", {}], diff --git a/packages/provider-pi/src/server/status.ts b/packages/provider-pi/src/server/status.ts index 3030200c245a3..7063c9d8ed61d 100644 --- a/packages/provider-pi/src/server/status.ts +++ b/packages/provider-pi/src/server/status.ts @@ -42,6 +42,7 @@ import { EMPTY_PI_MODEL_CAPABILITIES, thinkingCapabilitiesForPiModel, } from "./thinkingCapabilities.ts"; +import { isOmpVersionOutput, ompPresetModels, readOmpModelPresets } from "./presets.ts"; import { parsePiDiscoveredCommands, withPiBuiltinSlashCommands, @@ -160,7 +161,8 @@ const discoverPiViaRpc = Effect.fn("PiProviderStatus.discoverPiViaRpc")(function piSettings: PiSettings, environment: NodeJS.ProcessEnv, launchArgs: ReadonlyArray, - cwd?: string, + cwd: string | undefined, + isOmp: boolean, ) { const connection = yield* makePiDiscoveryConnection(piSettings, environment, launchArgs, cwd); const stateData = yield* connection.request({ type: "get_state" }); @@ -170,8 +172,21 @@ const discoverPiViaRpc = Effect.fn("PiProviderStatus.discoverPiViaRpc")(function .pipe(Effect.orElseSucceed(() => undefined)); const discoveredModels = yield* Effect.fromResult(parseDiscoveredModels(modelsData, stateData)); const { slashCommands, skills } = parsePiDiscoveredCommands(commandsData); + // Plain Pi has no `config` subcommand, so only ask binaries that identify as omp. + const presets = isOmp + ? yield* readOmpModelPresets({ + binaryPath: piSettings.binaryPath || "pi", + environment, + cwd, + }).pipe( + Effect.tapError((error) => + Effect.logWarning("OhMyPi model preset discovery failed.", { errorTag: error._tag }), + ), + Effect.orElseSucceed(() => []), + ) + : []; return { - models: discoveredModels, + models: [...ompPresetModels(presets, modelsData), ...discoveredModels], slashCommands: withPiBuiltinSlashCommands(slashCommands), skills, authenticated: discoveredModels.length > 0, @@ -394,6 +409,7 @@ export const checkPiProviderStatus = Effect.fn("checkPiProviderStatus")(function environment, resolvedLaunchArgs.args, cwd, + isOmpVersionOutput(versionOutput.stdout), ).pipe(Effect.timeoutOption(PI_RPC_DISCOVERY_TIMEOUT_MS), Effect.exit); if (Exit.isFailure(discoveryExit)) { yield* Effect.logWarning("Pi RPC discovery failed.", { diff --git a/packages/provider-pi/src/server/textGeneration.test.ts b/packages/provider-pi/src/server/textGeneration.test.ts index f4f9300c73113..193396e289d7e 100644 --- a/packages/provider-pi/src/server/textGeneration.test.ts +++ b/packages/provider-pi/src/server/textGeneration.test.ts @@ -7,6 +7,7 @@ import * as Queue from "effect/Queue"; import * as Schema from "effect/Schema"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; +import * as ChildProcess from "effect/process/ChildProcess"; import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner"; import { makePiTextGeneration } from "./textGeneration.ts"; @@ -14,7 +15,7 @@ import { makePiTextGeneration } from "./textGeneration.ts"; const decodeJsonLine = Schema.decodeUnknownSync(Schema.fromJsonString(Schema.Unknown)); const encodeJsonLine = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); -const makeFakePi = (reply: string, setModelError?: string) => +const makeFakePi = (reply: string, setModelError?: string, modelPresets?: unknown) => Effect.gen(function* () { const stdout = yield* Queue.unbounded(); const received: Array> = []; @@ -42,22 +43,38 @@ const makeFakePi = (reply: string, setModelError?: string) => } } }); - const spawner = ChildProcessSpawner.make(() => + const spawner = ChildProcessSpawner.make((command) => Effect.succeed( - ChildProcessSpawner.makeHandle({ - // Outside the valid pid range, so PiRpc's process-group kill never lands. - pid: ChildProcessSpawner.ProcessId(999_999_999), - exitCode: Effect.never, - isRunning: Effect.succeed(true), - kill: () => Effect.void, - unref: Effect.succeed(Effect.void), - stdin: Sink.forEach(onStdin), - stdout: Stream.fromQueue(stdout), - stderr: Stream.empty, - all: Stream.empty, - getInputFd: () => Sink.drain, - getOutputFd: () => Stream.empty, - }), + ChildProcess.isStandardCommand(command) && command.args[0] === "config" + ? ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(999_999_999), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(0)), + isRunning: Effect.succeed(false), + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + stdin: Sink.drain, + stdout: Stream.succeed( + new TextEncoder().encode(encodeJsonLine({ value: modelPresets })), + ), + stderr: Stream.empty, + all: Stream.empty, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }) + : ChildProcessSpawner.makeHandle({ + // Outside the valid pid range, so PiRpc's process-group kill never lands. + pid: ChildProcessSpawner.ProcessId(999_999_999), + exitCode: Effect.never, + isRunning: Effect.succeed(true), + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + stdin: Sink.forEach(onStdin), + stdout: Stream.fromQueue(stdout), + stderr: Stream.empty, + all: Stream.empty, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }), ), ); return { spawner, received }; @@ -88,3 +105,29 @@ it.effect("names the model and the disabled extensions when Pi cannot load it", assert.isUndefined(pi.received.find((record) => record["type"] === "prompt")); }), ); + +it.effect("generates with an OhMyPi preset's main model", () => + Effect.gen(function* () { + const pi = yield* makeFakePi("Update README", undefined, { + Cheap: { modelRoles: { default: "loem/z-ai/glm-5.2:high", task: "loem/kimi-k3" } }, + }); + const textGeneration = yield* makePiTextGeneration( + { enabled: true, binaryPath: "omp", launchArgs: "", customModels: [] }, + {}, + ).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, pi.spawner)); + + yield* textGeneration + .generateCommitMessage({ + cwd: process.cwd(), + branch: "main", + stagedSummary: "M README.md", + stagedPatch: "diff --git a/README.md b/README.md", + modelSelection: createModelSelection(ProviderInstanceId.make("pi"), "preset:Cheap"), + }) + .pipe(Effect.ignore); + + const setModel = pi.received.find((record) => record["type"] === "set_model"); + assert.equal(setModel?.["provider"], "loem"); + assert.equal(setModel?.["modelId"], "z-ai/glm-5.2"); + }), +); diff --git a/packages/provider-pi/src/server/textGeneration.ts b/packages/provider-pi/src/server/textGeneration.ts index 218a6300a35e1..d697b0f899191 100644 --- a/packages/provider-pi/src/server/textGeneration.ts +++ b/packages/provider-pi/src/server/textGeneration.ts @@ -9,6 +9,7 @@ import type { PiSettings } from "../settings.ts"; import { makePiRpcConnection, parsePiModelSlug } from "./rpc.ts"; import { buildPiRpcLaunch, resolvePiLaunchArgs } from "./mcpInjection.ts"; +import { parseOmpModelSelector, piPresetNameFromSlug, readOmpModelPresets } from "./presets.ts"; import * as TextGenerationOperations from "@t3tools/provider-core/server/textGenerationOperations"; const PI_TIMEOUT_MS = 180_000; @@ -21,6 +22,40 @@ export const makePiTextGeneration = Effect.fn("makePiTextGeneration")(function* ) { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const resolvePresetMainModel = Effect.fnUntraced(function* ( + operation: TextGenerationError["operation"], + cwd: string, + presetName: string, + ) { + const presets = yield* readOmpModelPresets({ + binaryPath: piSettings.binaryPath || "pi", + environment, + cwd, + }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.mapError( + (cause) => new TextGenerationError({ operation, detail: cause.message, cause }), + ), + ); + const preset = presets.find((candidate) => candidate.name === presetName); + if (preset === undefined) { + return yield* new TextGenerationError({ + operation, + detail: `OhMyPi model preset '${presetName}' was not found or is malformed.`, + }); + } + const selector = preset.modelRoles.default; + if (selector === undefined) return "default"; + const parsed = parseOmpModelSelector(selector); + if (parsed === null) { + return yield* new TextGenerationError({ + operation, + detail: `OhMyPi preset '${presetName}' main model '${selector}' must use provider/model format.`, + }); + } + return `${parsed.provider}/${parsed.modelId}`; + }); + const runPiJson: TextGenerationOperations.Runner = Effect.fn("PiTextGeneration.runPiJson")( function* (request) { const { operation, cwd, prompt, modelSelection } = request; @@ -49,13 +84,19 @@ export const makePiTextGeneration = Effect.fn("makePiTextGeneration")(function* env: launch.env, }).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner)); - if (modelSelection.model !== "default") { + // Text generation runs one prompt with no subagents, so a preset only contributes its main model. + const presetName = piPresetNameFromSlug(modelSelection.model); + const model = + presetName === undefined + ? modelSelection.model + : yield* resolvePresetMainModel(operation, cwd, presetName); + if (model !== "default") { // Reject unusable custom slugs; silently using Pi's default would misreport the requested model. - const parsed = parsePiModelSlug(modelSelection.model); + const parsed = parsePiModelSlug(model); if (parsed === null) { return yield* new TextGenerationError({ operation, - detail: `Pi model '${modelSelection.model}' must use provider/model format.`, + detail: `Pi model '${model}' must use provider/model format.`, }); } yield* connection @@ -67,7 +108,7 @@ export const makePiTextGeneration = Effect.fn("makePiTextGeneration")(function* ? new TextGenerationError({ operation, detail: - `Pi rejected model '${modelSelection.model}': ${error.detail ?? "unknown error"}. ` + + `Pi rejected model '${model}': ${error.detail ?? "unknown error"}. ` + "If the model comes from a Pi extension or package, text generation cannot use it " + "because it runs Pi with --no-extensions. Pick a built-in Pi model or one defined " + "in Pi's models.json.", From 55865d386e8c1c13471fa12ca2af3e17f1871402 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Mon, 26 Oct 2026 23:12:30 +0000 Subject: [PATCH 02/11] fix(pi): match OhMyPi preset semantics more closely Preset thinking 'auto' now applies, literal model ids win over thinking suffixes, invalid thinking levels drop the preset, config reads honor --profile and time out on their own. --- docs/adr/0025-ohmypi-model-presets.md | 4 +- docs/user/providers-pi.md | 4 +- .../provider-pi/src/server/adapter.test.ts | 27 ++- packages/provider-pi/src/server/adapter.ts | 28 ++- .../provider-pi/src/server/presets.test.ts | 26 +++ packages/provider-pi/src/server/presets.ts | 190 ++++++++++++------ .../provider-pi/src/server/status.test.ts | 56 +++++- packages/provider-pi/src/server/status.ts | 1 + .../src/server/textGeneration.test.ts | 30 +-- .../provider-pi/src/server/textGeneration.ts | 23 ++- 10 files changed, 297 insertions(+), 92 deletions(-) create mode 100644 packages/provider-pi/src/server/presets.test.ts diff --git a/docs/adr/0025-ohmypi-model-presets.md b/docs/adr/0025-ohmypi-model-presets.md index f8537a6fb3b9a..8dbb974649546 100644 --- a/docs/adr/0025-ohmypi-model-presets.md +++ b/docs/adr/0025-ohmypi-model-presets.md @@ -12,7 +12,9 @@ so T3 could only change the main model. When the Pi binary reports an `omp/` version, discovery reads `omp config get modelPresets --json` and lists each preset as a model with the slug `preset:` and a "Preset" qualifier. Its Thinking select is the one -for the preset's main model and starts at the preset's level. +for the preset's main model and starts at the preset's level. Config reads pass +the instance's `--profile` and time out on their own, so a slow read only loses +the presets, not the discovered models. Applying a preset with omp's own `/modelpreset switch` writes roles to the user's global config, and every running omp process reloads it, so one thread diff --git a/docs/user/providers-pi.md b/docs/user/providers-pi.md index a271808a12f8e..c6e9751718a66 100644 --- a/docs/user/providers-pi.md +++ b/docs/user/providers-pi.md @@ -55,7 +55,9 @@ titles, status lines, and widgets does not have a T3 Code equivalent. If Pi's binary path points to [OhMyPi](https://github.com/can1357/oh-my-pi) (`omp`), its saved model presets appear in the model picker labelled **Preset**. Save presets in `omp` with -`/modelpreset save `, then refresh the provider in Settings to see new ones. +`/modelpreset save `, then refresh the provider in Settings to see new ones. The picker lists +presets saved in your user configuration; presets saved only in a project's configuration do not +appear. Choosing a preset applies all of its role models and its thinking level to that thread only. Other threads and `omp` sessions outside T3 Code keep their own models. Switching a thread to another diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index 5db04e616d37d..e831c4ef1eb0a 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -434,6 +434,7 @@ const OMP_CONFIG_VALUES: Record = { modelRoles: { default: "anthropic/claude-opus:high", task: "anthropic/claude-sonnet" }, defaultThinkingLevel: "auto", }, + Auto: { modelRoles: { default: "openai/gpt" }, defaultThinkingLevel: "auto" }, }, modelRoles: { default: "openai/gpt", designer: "openai/gpt-mini" }, }; @@ -442,9 +443,9 @@ const OMP_CONFIG_VALUES: Record = { const makeOmpPresetAdapter = (fake: FakePi) => { const spawner = ChildProcessSpawner.make((command) => { const args = ChildProcess.isStandardCommand(command) ? command.args : []; - if (args[0] !== "config") return fake.spawner.spawn(command); + if (!args.includes("config")) return fake.spawner.spawn(command); const stdout = new TextEncoder().encode( - encodeJsonLine({ key: args[2], value: OMP_CONFIG_VALUES[args[2] ?? ""] }), + encodeJsonLine({ value: OMP_CONFIG_VALUES[args[args.indexOf("config") + 2] ?? ""] }), ); return Effect.succeed( ChildProcessSpawner.makeHandle({ @@ -1899,6 +1900,28 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); + it.effect("applies a preset's auto thinking over the resumed session's level", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + fake.queueState({ thinkingLevel: "high" }); + const adapter = yield* makeOmpPresetAdapter(fake); + const runtime = yield* adapter.openSession({ + threadId: THREAD_ID, + providerSessionId: SESSION_ID, + modelSelection: modelSelection("preset:Auto"), + runtimePolicy, + }); + const providerThread = yield* runtime.ensureThread({ + threadId: THREAD_ID, + modelSelection: modelSelection("preset:Auto"), + runtimePolicy, + }); + yield* startTurn(runtime, providerThread, "preset:Auto"); + const levelRequest = yield* fake.takeRequest("set_thinking_level"); + assert.equal(levelRequest["level"], "auto"); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + it.effect("fails a preset session that names a missing preset", () => Effect.gen(function* () { const fake = yield* makeFakePi; diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index 9fab97495b511..41f16f642bbe1 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -75,6 +75,8 @@ import { ompPresetThinkingLevel, parseOmpModelSelector, encodeOmpConfigOverlay, + OMP_AUTO_THINKING, + piModelInventorySlugs, piPresetNameFromSlug, resolveOmpPreset, type OmpModelPreset, @@ -484,6 +486,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( binaryPath: options.settings.binaryPath || "pi", environment: options.environment, cwd, + launchArgs: resolvedLaunchArgs.args, name: presetName, }).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), @@ -2287,6 +2290,8 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( return providerThread; }); + let presetKnownModels: ReadonlySet | null = null; + let presetMainModel: string | null = null; const applyPresetSelection = Effect.fnUntraced(function* ( modelSelection: ModelSelection, presetName: string, @@ -2296,14 +2301,20 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( `OhMyPi preset '${presetName}' needs a session restart to apply`, ); } + const defaultSelector = sessionPreset.modelRoles.default; + if (presetKnownModels === null && defaultSelector !== undefined) { + presetKnownModels = piModelInventorySlugs( + yield* request({ type: "get_available_models" }), + ); + } + const knownModels = presetKnownModels ?? new Set(); const thinking = getModelSelectionStringOptionValue(modelSelection, "thinking") ?? - ompPresetThinkingLevel(sessionPreset); - const defaultSelector = sessionPreset.modelRoles.default; + ompPresetThinkingLevel(sessionPreset, knownModels); if (modelSelection.model !== appliedModel) { // Without a `default` role, omp picks the main model itself, as `/modelpreset switch` does. if (defaultSelector !== undefined) { - const parsed = parseOmpModelSelector(defaultSelector); + const parsed = parseOmpModelSelector(defaultSelector, knownModels); if (parsed === null) { return yield* protocolError( `OhMyPi preset '${presetName}' main model '${defaultSelector}' must use provider/model format`, @@ -2315,6 +2326,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( modelId: parsed.modelId, }); contextWindow = rememberModelContextWindow(selectedModel); + presetMainModel = `${parsed.provider}/${parsed.modelId}`; } appliedModel = modelSelection.model; const updatedAt = yield* DateTime.now; @@ -2328,7 +2340,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( if ( thinking !== undefined && thinking !== appliedThinking && - PI_THINKING_LEVELS.has(thinking) + (PI_THINKING_LEVELS.has(thinking) || thinking === OMP_AUTO_THINKING) ) { yield* request({ type: "set_thinking_level", level: thinking }); appliedThinking = thinking; @@ -2436,17 +2448,13 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( ), getModelContextWindow: (selection) => { if (selection.instanceId !== options.instanceId) return undefined; - const presetDefault = - piPresetNameFromSlug(selection.model) === undefined - ? null - : parseOmpModelSelector(sessionPreset?.modelRoles.default ?? ""); const slug = selection.model === PI_INHERIT_MODEL_SLUG ? baselineModel === null ? undefined : `${baselineModel.provider}/${baselineModel.modelId}` - : presetDefault !== null - ? `${presetDefault.provider}/${presetDefault.modelId}` + : piPresetNameFromSlug(selection.model) !== undefined + ? (presetMainModel ?? undefined) : selection.model; return slug === undefined ? undefined : modelContextWindows.get(slug); }, diff --git a/packages/provider-pi/src/server/presets.test.ts b/packages/provider-pi/src/server/presets.test.ts new file mode 100644 index 0000000000000..c6a7f698901da --- /dev/null +++ b/packages/provider-pi/src/server/presets.test.ts @@ -0,0 +1,26 @@ +import { assert, describe, it } from "@effect/vitest"; + +import { parseOmpModelSelector } from "./presets.ts"; + +describe("parseOmpModelSelector", () => { + it("reads a known thinking suffix and keeps slashes in the model id", () => { + assert.deepEqual(parseOmpModelSelector("loem/z-ai/glm-5.2:high", new Set()), { + provider: "loem", + modelId: "z-ai/glm-5.2", + thinking: "high", + }); + }); + + it("keeps a suffix that is part of an exact model id, as omp does", () => { + assert.deepEqual(parseOmpModelSelector("openrouter/glm:max", new Set(["openrouter/glm:max"])), { + provider: "openrouter", + modelId: "glm:max", + thinking: undefined, + }); + }); + + it("rejects aliases and bare model names it cannot hand to set_model", () => { + assert.isNull(parseOmpModelSelector("@slow", new Set())); + assert.isNull(parseOmpModelSelector("claude-opus:high", new Set())); + }); +}); diff --git a/packages/provider-pi/src/server/presets.ts b/packages/provider-pi/src/server/presets.ts index ab462f24e0e59..7ab77770b7256 100644 --- a/packages/provider-pi/src/server/presets.ts +++ b/packages/provider-pi/src/server/presets.ts @@ -6,7 +6,7 @@ import * as Predicate from "effect/Predicate"; import * as Schema from "effect/Schema"; import * as ChildProcess from "effect/process/ChildProcess"; -import { PiRpcError } from "./rpc.ts"; +import { PiRpcError, PiRpcTimeoutError } from "./rpc.ts"; import { EMPTY_PI_MODEL_CAPABILITIES, thinkingCapabilitiesForPiModel, @@ -15,6 +15,17 @@ import { // OhMyPi (`omp`) model presets name a whole set of role models; T3 lists them as models. const PI_PRESET_SLUG_PREFIX = "preset:"; const PRESET_NAME_PATTERN = /^[a-zA-Z][\w-]*$/; +const OMP_CONFIG_TIMEOUT_MS = 10_000; +// omp accepts these for a preset's `defaultThinkingLevel`; `off` is only a selector suffix. +const OMP_DEFAULT_THINKING_LEVELS = new Set([ + "minimal", + "low", + "medium", + "high", + "xhigh", + "max", + "auto", +]); const OMP_THINKING_SUFFIXES = new Set([ "off", "minimal", @@ -32,6 +43,8 @@ const decodeOmpConfigOutput = Schema.decodeUnknownEffect( ); export const encodeOmpConfigOverlay = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); +export const OMP_AUTO_THINKING = "auto"; + export interface OmpModelPreset { readonly name: string; readonly modelRoles: Readonly>; @@ -73,30 +86,52 @@ function parseOmpModelPresets(value: unknown): ReadonlyArray { } modelRoles[role] = selector; } - if (!valid) continue; const level = raw.defaultThinkingLevel; - presets.push({ - name, - modelRoles, - defaultThinkingLevel: typeof level === "string" ? level : undefined, - }); + if ( + !valid || + (level !== undefined && + (typeof level !== "string" || !OMP_DEFAULT_THINKING_LEVELS.has(level))) + ) { + continue; + } + presets.push({ name, modelRoles, defaultThinkingLevel: level }); } return presets.toSorted((left, right) => left.name.localeCompare(right.name)); } +/** `provider/id` slugs from a Pi RPC `get_available_models` response. */ +export function piModelInventorySlugs(modelInventory: unknown): ReadonlySet { + const models = isStringRecord(modelInventory) ? modelInventory.models : undefined; + if (!Array.isArray(models)) return new Set(); + return new Set( + models.flatMap((model) => + isStringRecord(model) && typeof model.provider === "string" && typeof model.id === "string" + ? [`${model.provider}/${model.id}`] + : [], + ), + ); +} + /** * Splits an omp role selector (`provider/model[:level]`) into what Pi RPC `set_model` takes. + * Like omp, an exact model id wins over reading its last `:segment` as a thinking level. * shortcut: aliases (`@slow`) and bare model patterns need omp's fuzzy resolver, so they are * reported as unsupported; add an RPC-side resolution if presets commonly use them. */ -export function parseOmpModelSelector(selector: string): { +export function parseOmpModelSelector( + selector: string, + knownModels: ReadonlySet, +): { readonly provider: string; readonly modelId: string; readonly thinking: string | undefined; } | null { const colon = selector.lastIndexOf(":"); const suffix = colon === -1 ? undefined : selector.slice(colon + 1); - const thinking = suffix !== undefined && OMP_THINKING_SUFFIXES.has(suffix) ? suffix : undefined; + const thinking = + suffix !== undefined && OMP_THINKING_SUFFIXES.has(suffix) && !knownModels.has(selector) + ? suffix + : undefined; const model = thinking === undefined ? selector : selector.slice(0, colon); const separator = model.indexOf("/"); if (model.startsWith("@") || separator <= 0 || separator === model.length - 1) return null; @@ -104,10 +139,13 @@ export function parseOmpModelSelector(selector: string): { } /** The level a preset starts at: the `default` selector's suffix wins over `defaultThinkingLevel`. */ -export function ompPresetThinkingLevel(preset: OmpModelPreset): string | undefined { +export function ompPresetThinkingLevel( + preset: OmpModelPreset, + knownModels: ReadonlySet, +): string | undefined { const selector = preset.modelRoles.default; const fromSelector = - selector === undefined ? undefined : parseOmpModelSelector(selector)?.thinking; + selector === undefined ? undefined : parseOmpModelSelector(selector, knownModels)?.thinking; return fromSelector ?? preset.defaultThinkingLevel; } @@ -130,15 +168,35 @@ function ompPresetOverlay( }; } +/** Pi models have no `auto` level; a preset that uses omp's `auto` offers it as its default. */ +function presetThinkingCapabilities(rawModel: unknown, level: string | undefined) { + const capabilities = thinkingCapabilitiesForPiModel(rawModel, level); + if (level !== OMP_AUTO_THINKING) return capabilities; + return { + optionDescriptors: capabilities.optionDescriptors?.map((descriptor) => + descriptor.type === "select" + ? { + ...descriptor, + options: [ + { id: OMP_AUTO_THINKING, label: "Auto", isDefault: true }, + ...descriptor.options, + ], + } + : descriptor, + ), + }; +} + export function ompPresetModels( presets: ReadonlyArray, modelInventory: unknown, ): ReadonlyArray { const inventory = isStringRecord(modelInventory) ? modelInventory.models : undefined; const rawModels: ReadonlyArray = Array.isArray(inventory) ? inventory : []; + const knownModels = piModelInventorySlugs(modelInventory); return presets.map((preset) => { const selector = preset.modelRoles.default; - const parsed = selector === undefined ? null : parseOmpModelSelector(selector); + const parsed = selector === undefined ? null : parseOmpModelSelector(selector, knownModels); const rawModel = parsed === null ? undefined @@ -156,62 +214,82 @@ export function ompPresetModels( capabilities: rawModel === undefined ? EMPTY_PI_MODEL_CAPABILITIES - : thinkingCapabilitiesForPiModel(rawModel, ompPresetThinkingLevel(preset)), + : presetThinkingCapabilities(rawModel, ompPresetThinkingLevel(preset, knownModels)), }; }); } -const readOmpConfigValue = Effect.fn("PiPresets.readOmpConfigValue")(function* (input: { +interface OmpConfigReadInput { readonly binaryPath: string; readonly environment: NodeJS.ProcessEnv; readonly cwd: string | undefined; - readonly key: string; -}) { - const args = ["config", "get", input.key, "--json"]; - const spawnCommand = yield* resolveSpawnCommand(input.binaryPath, args, { - env: input.environment, - }); - const result = yield* spawnAndCollect( - input.binaryPath, - ChildProcess.make(spawnCommand.command, spawnCommand.args, { + /** Resolved Pi launch args; their `--profile` selects which omp config is read. */ + readonly launchArgs: ReadonlyArray; +} + +function ompProfileArgs(launchArgs: ReadonlyArray): ReadonlyArray { + const index = launchArgs.findIndex((arg) => arg === "--profile" || arg.startsWith("--profile=")); + if (index === -1) return []; + const arg = launchArgs[index]!; + if (arg !== "--profile") return [arg]; + const value = launchArgs[index + 1]; + return value === undefined ? [] : [arg, value]; +} + +const readOmpConfigValue = Effect.fn("PiPresets.readOmpConfigValue")( + function* (input: OmpConfigReadInput & { readonly key: string }) { + // omp only reads `--profile` before the subcommand. + const args = [...ompProfileArgs(input.launchArgs), "config", "get", input.key, "--json"]; + const spawnCommand = yield* resolveSpawnCommand(input.binaryPath, args, { env: input.environment, - shell: spawnCommand.shell, - ...(input.cwd === undefined ? {} : { cwd: input.cwd }), - }), - ); - const detail = `\`omp config get ${input.key}\``; - if (result.code !== 0) { - return yield* new PiRpcError({ - operation: "omp_config", - detail: `${detail} exited ${result.code}`, }); - } - const parsed = yield* decodeOmpConfigOutput(result.stdout).pipe( - Effect.mapError( - (cause) => - new PiRpcError({ - operation: "omp_config", - detail: `${detail} printed invalid JSON`, - cause, - }), - ), - ); - return parsed.value; -}); + const result = yield* spawnAndCollect( + input.binaryPath, + ChildProcess.make(spawnCommand.command, spawnCommand.args, { + env: input.environment, + shell: spawnCommand.shell, + ...(input.cwd === undefined ? {} : { cwd: input.cwd }), + }), + ); + const detail = `\`omp config get ${input.key}\``; + if (result.code !== 0) { + return yield* new PiRpcError({ + operation: "omp_config", + detail: `${detail} exited ${result.code}`, + }); + } + const parsed = yield* decodeOmpConfigOutput(result.stdout).pipe( + Effect.mapError( + (cause) => + new PiRpcError({ + operation: "omp_config", + detail: `${detail} printed invalid JSON`, + cause, + }), + ), + ); + return parsed.value; + }, + Effect.timeoutOrElse({ + duration: OMP_CONFIG_TIMEOUT_MS, + orElse: () => + Effect.fail( + new PiRpcTimeoutError({ operation: "omp_config", timeoutMs: OMP_CONFIG_TIMEOUT_MS }), + ), + }), +); -export const readOmpModelPresets = (input: { - readonly binaryPath: string; - readonly environment: NodeJS.ProcessEnv; - readonly cwd: string | undefined; -}) => readOmpConfigValue({ ...input, key: "modelPresets" }).pipe(Effect.map(parseOmpModelPresets)); +/** + * shortcut: `omp config get` returns presets deep-merged across config layers, while omp takes a + * same-name preset whole from its highest layer; read the layer files if that ever matters. + */ +export const readOmpModelPresets = (input: OmpConfigReadInput) => + readOmpConfigValue({ ...input, key: "modelPresets" }).pipe(Effect.map(parseOmpModelPresets)); /** Resolves a preset by name and builds its overlay against the workspace's current roles. */ -export const resolveOmpPreset = Effect.fn("PiPresets.resolveOmpPreset")(function* (input: { - readonly binaryPath: string; - readonly environment: NodeJS.ProcessEnv; - readonly cwd: string | undefined; - readonly name: string; -}) { +export const resolveOmpPreset = Effect.fn("PiPresets.resolveOmpPreset")(function* ( + input: OmpConfigReadInput & { readonly name: string }, +) { const [presets, roles] = yield* Effect.all( [readOmpModelPresets(input), readOmpConfigValue({ ...input, key: "modelRoles" })], { concurrency: "unbounded" }, diff --git a/packages/provider-pi/src/server/status.test.ts b/packages/provider-pi/src/server/status.test.ts index 9ff0f5c89cb3b..0bf54c728c695 100644 --- a/packages/provider-pi/src/server/status.test.ts +++ b/packages/provider-pi/src/server/status.test.ts @@ -1,11 +1,14 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; import * as Cause from "effect/Cause"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as Queue from "effect/Queue"; import * as Schema from "effect/Schema"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; +import * as TestClock from "effect/testing/TestClock"; import * as ChildProcess from "effect/process/ChildProcess"; import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner"; @@ -54,7 +57,11 @@ function piProbeSpawner(version: string) { const piDiscoverySpawner = ( inventory: unknown, - omp?: { readonly modelPresets: unknown; readonly configCalls: Array }, + omp?: { + readonly modelPresets: unknown; + readonly configCalls: Array; + readonly stall?: Deferred.Deferred; + }, ) => Effect.gen(function* () { const stdout = yield* Queue.unbounded(); @@ -86,8 +93,19 @@ const piDiscoverySpawner = ( ); return ChildProcessSpawner.make((command) => { const args = ChildProcess.isStandardCommand(command) ? command.args : []; - if (args[0] === "config") { + if (args.includes("config")) { omp?.configCalls.push(args.join(" ")); + if (omp?.stall !== undefined) { + return Deferred.succeed(omp.stall, undefined).pipe( + Effect.as( + ChildProcessSpawner.makeHandle({ + ...processHandle({}), + exitCode: Effect.never, + stdout: Stream.never, + }), + ), + ); + } return Effect.succeed( processHandle({ stdout: encodeJsonLine({ value: omp?.modelPresets }) }), ); @@ -171,13 +189,17 @@ describe("PiProvider", () => { OpenAI: { modelRoles: { default: "openai/gpt" } }, Anthropic: { modelRoles: { default: "anthropic/claude-opus:low" } }, Broken: { modelRoles: { default: "" } }, + NumericThinking: { modelRoles: { default: "openai/gpt" }, defaultThinkingLevel: 42 }, + UnknownThinking: { modelRoles: { default: "openai/gpt" }, defaultThinkingLevel: "x" }, }, }, ); - const snapshot = yield* checkPiProviderStatus({ ...settings, binaryPath: "omp" }).pipe( - Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), - ); - assert.deepEqual(configCalls, ["config get modelPresets --json"]); + const snapshot = yield* checkPiProviderStatus({ + ...settings, + binaryPath: "omp", + launchArgs: "--profile work", + }).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner)); + assert.deepEqual(configCalls, ["--profile work config get modelPresets --json"]); assert.deepEqual( snapshot.models.map((model) => [model.slug, model.subProvider]), [ @@ -199,6 +221,28 @@ describe("PiProvider", () => { }).pipe(Effect.provide(NodeServices.layer)), ); + it.effect("keeps discovered models when OhMyPi preset discovery stalls", () => + Effect.gen(function* () { + const stall = yield* Deferred.make(); + const spawner = yield* piDiscoverySpawner( + { models: [{ provider: "openai", id: "gpt" }] }, + { configCalls: [], modelPresets: {}, stall }, + ); + const fiber = yield* checkPiProviderStatus({ ...settings, binaryPath: "omp" }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.forkChild, + ); + yield* Deferred.await(stall); + yield* TestClock.adjust("10 seconds"); + const snapshot = yield* Fiber.join(fiber); + assert.equal(snapshot.auth.status, "authenticated"); + assert.deepEqual( + snapshot.models.map((model) => model.slug), + ["default", "openai/gpt"], + ); + }).pipe(Effect.provide(NodeServices.layer)), + ); + // regression: #18498 a malformed model inventory must not restore stale Pi models it.effect.each([ ["missing models", {}], diff --git a/packages/provider-pi/src/server/status.ts b/packages/provider-pi/src/server/status.ts index 7063c9d8ed61d..d17b385dc0f02 100644 --- a/packages/provider-pi/src/server/status.ts +++ b/packages/provider-pi/src/server/status.ts @@ -178,6 +178,7 @@ const discoverPiViaRpc = Effect.fn("PiProviderStatus.discoverPiViaRpc")(function binaryPath: piSettings.binaryPath || "pi", environment, cwd, + launchArgs, }).pipe( Effect.tapError((error) => Effect.logWarning("OhMyPi model preset discovery failed.", { errorTag: error._tag }), diff --git a/packages/provider-pi/src/server/textGeneration.test.ts b/packages/provider-pi/src/server/textGeneration.test.ts index 193396e289d7e..033d3d5afadd8 100644 --- a/packages/provider-pi/src/server/textGeneration.test.ts +++ b/packages/provider-pi/src/server/textGeneration.test.ts @@ -45,7 +45,7 @@ const makeFakePi = (reply: string, setModelError?: string, modelPresets?: unknow }); const spawner = ChildProcessSpawner.make((command) => Effect.succeed( - ChildProcess.isStandardCommand(command) && command.args[0] === "config" + ChildProcess.isStandardCommand(command) && command.args.includes("config") ? ChildProcessSpawner.makeHandle({ pid: ChildProcessSpawner.ProcessId(999_999_999), exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(0)), @@ -108,23 +108,27 @@ it.effect("names the model and the disabled extensions when Pi cannot load it", it.effect("generates with an OhMyPi preset's main model", () => Effect.gen(function* () { - const pi = yield* makeFakePi("Update README", undefined, { - Cheap: { modelRoles: { default: "loem/z-ai/glm-5.2:high", task: "loem/kimi-k3" } }, - }); + const pi = yield* makeFakePi( + encodeJsonLine({ subject: "Update README", body: "" }), + undefined, + { + Cheap: { modelRoles: { default: "loem/z-ai/glm-5.2:high", task: "loem/kimi-k3" } }, + }, + ); const textGeneration = yield* makePiTextGeneration( { enabled: true, binaryPath: "omp", launchArgs: "", customModels: [] }, {}, ).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, pi.spawner)); - yield* textGeneration - .generateCommitMessage({ - cwd: process.cwd(), - branch: "main", - stagedSummary: "M README.md", - stagedPatch: "diff --git a/README.md b/README.md", - modelSelection: createModelSelection(ProviderInstanceId.make("pi"), "preset:Cheap"), - }) - .pipe(Effect.ignore); + const generated = yield* textGeneration.generateCommitMessage({ + cwd: process.cwd(), + branch: "main", + stagedSummary: "M README.md", + stagedPatch: "diff --git a/README.md b/README.md", + modelSelection: createModelSelection(ProviderInstanceId.make("pi"), "preset:Cheap"), + }); + + assert.equal(generated.subject, "Update README"); const setModel = pi.received.find((record) => record["type"] === "set_model"); assert.equal(setModel?.["provider"], "loem"); diff --git a/packages/provider-pi/src/server/textGeneration.ts b/packages/provider-pi/src/server/textGeneration.ts index d697b0f899191..91b02cb962ddb 100644 --- a/packages/provider-pi/src/server/textGeneration.ts +++ b/packages/provider-pi/src/server/textGeneration.ts @@ -9,7 +9,12 @@ import type { PiSettings } from "../settings.ts"; import { makePiRpcConnection, parsePiModelSlug } from "./rpc.ts"; import { buildPiRpcLaunch, resolvePiLaunchArgs } from "./mcpInjection.ts"; -import { parseOmpModelSelector, piPresetNameFromSlug, readOmpModelPresets } from "./presets.ts"; +import { + parseOmpModelSelector, + piModelInventorySlugs, + piPresetNameFromSlug, + readOmpModelPresets, +} from "./presets.ts"; import * as TextGenerationOperations from "@t3tools/provider-core/server/textGenerationOperations"; const PI_TIMEOUT_MS = 180_000; @@ -25,12 +30,15 @@ export const makePiTextGeneration = Effect.fn("makePiTextGeneration")(function* const resolvePresetMainModel = Effect.fnUntraced(function* ( operation: TextGenerationError["operation"], cwd: string, + launchArgs: ReadonlyArray, presetName: string, + listModels: Effect.Effect, ) { const presets = yield* readOmpModelPresets({ binaryPath: piSettings.binaryPath || "pi", environment, cwd, + launchArgs, }).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), Effect.mapError( @@ -46,7 +54,8 @@ export const makePiTextGeneration = Effect.fn("makePiTextGeneration")(function* } const selector = preset.modelRoles.default; if (selector === undefined) return "default"; - const parsed = parseOmpModelSelector(selector); + const knownModels = piModelInventorySlugs(yield* listModels); + const parsed = parseOmpModelSelector(selector, knownModels); if (parsed === null) { return yield* new TextGenerationError({ operation, @@ -89,7 +98,15 @@ export const makePiTextGeneration = Effect.fn("makePiTextGeneration")(function* const model = presetName === undefined ? modelSelection.model - : yield* resolvePresetMainModel(operation, cwd, presetName); + : yield* resolvePresetMainModel( + operation, + cwd, + resolvedLaunchArgs.args, + presetName, + connection + .request({ type: "get_available_models" }) + .pipe(Effect.orElseSucceed(() => undefined)), + ); if (model !== "default") { // Reject unusable custom slugs; silently using Pi's default would misreport the requested model. const parsed = parsePiModelSlug(model); From e99606b4870854f22b936ed087d1108b9d4c6fb5 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Mon, 26 Oct 2026 23:22:15 +0000 Subject: [PATCH 03/11] fix(pi): switch OhMyPi presets inside the live session Message dispatch never consults planSelectionTransition, so preset switches from the composer could not rely on a session restart. Every omp session now launches with its own overlay, which the adapter rewrites when the selected preset changes. Presets need a default role. --- docs/adr/0025-ohmypi-model-presets.md | 27 ++- docs/user/providers-pi.md | 12 +- .../provider-pi/src/server/adapter.test.ts | 218 +++++++++--------- packages/provider-pi/src/server/adapter.ts | 145 ++++++------ packages/provider-pi/src/server/presets.ts | 25 +- .../provider-pi/src/server/textGeneration.ts | 3 +- 6 files changed, 237 insertions(+), 193 deletions(-) diff --git a/docs/adr/0025-ohmypi-model-presets.md b/docs/adr/0025-ohmypi-model-presets.md index 8dbb974649546..bf800346f808d 100644 --- a/docs/adr/0025-ohmypi-model-presets.md +++ b/docs/adr/0025-ohmypi-model-presets.md @@ -18,16 +18,23 @@ the presets, not the discovered models. Applying a preset with omp's own `/modelpreset switch` writes roles to the user's global config, and every running omp process reloads it, so one thread -would change all the others and the user's terminal. T3 instead starts the -thread's omp process with a `--config` overlay file in T3's cache. The overlay -holds the preset's roles, `null` tombstones for roles the preset leaves out (as -`/modelpreset switch` clears them), and its thinking level. The overlay is -rebuilt from the current preset each time the process starts. Changing which -preset a thread uses returns `restart_session`, so the session resumes on a -new process with the new overlay, and the first turn sends `set_model` for the -preset's `default` role. A preset whose main model is an alias rather than -`provider/model` fails with a clear error. Text generation has no per-model -filter, so a preset chosen there uses only its main model. +would change all the others and the user's terminal. T3 instead starts every +omp session with its own `--config` overlay file in T3's cache, empty until a +turn selects a preset. Selecting one rewrites the overlay with the preset's +roles, `null` tombstones for roles the preset leaves out (as `/modelpreset +switch` clears them), and its thinking level, read fresh from omp; then the +turn sets the main model and thinking over RPC. Selecting a plain model empties +the overlay again. omp watches overlay files and reloads them about 200ms after +a write, so role models switch from the next role lookup. A restart would avoid +that delay, but ordinary message dispatch never consults the adapter's +selection-transition plan, so switching has to work inside the live session. + +Presets without a `default` role are not listed, because T3 sets the main model +from it, and one written as an alias rather than `provider/model` fails with a +clear error. omp's `--smol`, `--slow`, and `--plan` flags and their +`PI_*_MODEL` variables are runtime overrides that outrank any overlay. Text +generation has no per-model filter, so a preset chosen there uses only its +main model. When updating from upstream, keep presets inside the Pi provider and keep the per-process overlay; do not switch presets by writing omp's global config. diff --git a/docs/user/providers-pi.md b/docs/user/providers-pi.md index c6e9751718a66..85d05497318cc 100644 --- a/docs/user/providers-pi.md +++ b/docs/user/providers-pi.md @@ -60,10 +60,14 @@ presets saved in your user configuration; presets saved only in a project's conf appear. Choosing a preset applies all of its role models and its thinking level to that thread only. Other -threads and `omp` sessions outside T3 Code keep their own models. Switching a thread to another -preset restarts its Pi session and resumes the same conversation. Edits to a preset reach a thread -the next time its session starts. Use the picker rather than `/modelpreset switch`, which changes -OhMyPi's global configuration. A preset's main model must be written as `provider/model`. +threads and `omp` sessions outside T3 Code keep their own models. You can switch presets, or switch +between a preset and a single model, at any point in a thread. Edits to a preset apply the next time +a thread switches to it or its session restarts. Use the picker rather than `/modelpreset switch`, +which changes OhMyPi's global configuration. + +A preset needs a main (`default`) model written as `provider/model`. The `--smol`, `--slow`, and +`--plan` launch arguments, and the matching `PI_SMOL_MODEL`, `PI_SLOW_MODEL`, and `PI_PLAN_MODEL` +environment variables, override those roles in every preset. ## Permission Modes diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index e831c4ef1eb0a..01dcef662764a 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -190,6 +190,21 @@ const recordedIdleState = (sessionFile: string) => ({ pendingMessageCount: 0, }); +const exitedHandle = (stdout: string) => + ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(FAKE_PID), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(0)), + isRunning: Effect.succeed(false), + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + stdin: Sink.drain, + stdout: Stream.succeed(new TextEncoder().encode(stdout)), + stderr: Stream.empty, + all: Stream.empty, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }); + const makeFakePi: Effect.Effect = Effect.gen(function* () { const stdout = yield* Queue.unbounded(); const requests = yield* Queue.unbounded(); @@ -310,6 +325,9 @@ const makeFakePi: Effect.Effect = Effect.gen(function* () { }; const spawner = ChildProcessSpawner.make((command) => Effect.sync(() => { + if (ChildProcess.isStandardCommand(command) && command.args.includes("--version")) { + return exitedHandle("pi 0.84.3\n"); + } if (ChildProcess.isStandardCommand(command)) { lastSpawn = { args: command.args, @@ -443,24 +461,12 @@ const OMP_CONFIG_VALUES: Record = { const makeOmpPresetAdapter = (fake: FakePi) => { const spawner = ChildProcessSpawner.make((command) => { const args = ChildProcess.isStandardCommand(command) ? command.args : []; + if (args.includes("--version")) return Effect.succeed(exitedHandle("omp/18.5.1\n")); if (!args.includes("config")) return fake.spawner.spawn(command); - const stdout = new TextEncoder().encode( - encodeJsonLine({ value: OMP_CONFIG_VALUES[args[args.indexOf("config") + 2] ?? ""] }), - ); return Effect.succeed( - ChildProcessSpawner.makeHandle({ - pid: ChildProcessSpawner.ProcessId(FAKE_PID), - exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(0)), - isRunning: Effect.succeed(false), - kill: () => Effect.void, - unref: Effect.succeed(Effect.void), - stdin: Sink.drain, - stdout: Stream.succeed(stdout), - stderr: Stream.empty, - all: Stream.empty, - getInputFd: () => Sink.drain, - getOutputFd: () => Stream.empty, - }), + exitedHandle( + encodeJsonLine({ value: OMP_CONFIG_VALUES[args[args.indexOf("config") + 2] ?? ""] }), + ), ); }); return makePiAdapterV2({ @@ -470,6 +476,29 @@ const makeOmpPresetAdapter = (fake: FakePi) => { }).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner)); }; +const openOmpRuntime = Effect.fnUntraced(function* (fake: FakePi, model: string) { + const adapter = yield* makeOmpPresetAdapter(fake); + const runtime = yield* adapter.openSession({ + threadId: THREAD_ID, + providerSessionId: SESSION_ID, + modelSelection: modelSelection(model), + runtimePolicy, + }); + const emitted = yield* Queue.unbounded(); + yield* runtime.events.pipe( + Stream.runForEach((event) => Queue.offer(emitted, event)), + Effect.forkScoped, + ); + const takeEvent = (predicate: (event: ProviderAdapter.ProviderAdapterV2Event) => boolean) => + Effect.gen(function* () { + while (true) { + const event = yield* Queue.take(emitted); + if (predicate(event)) return event; + } + }); + return { runtime, takeEvent }; +}); + const openRuntime = Effect.fnUntraced(function* ( fake: FakePi, model = "default", @@ -1506,19 +1535,22 @@ describe("PiAdapterV2", () => { yield* openRuntime({ ...fake, spawner: ChildProcessSpawner.make((command) => - Effect.acquireRelease( - fake.spawner - .spawn(command) - .pipe( - Effect.map((child) => - ChildProcessSpawner.makeHandle({ ...child, exitCode: Deferred.await(exited) }), + ChildProcess.isStandardCommand(command) && command.args.includes("--version") + ? fake.spawner.spawn(command) + : Effect.acquireRelease( + fake.spawner.spawn(command).pipe( + Effect.map((child) => + ChildProcessSpawner.makeHandle({ + ...child, + exitCode: Deferred.await(exited), + }), + ), ), + () => + Effect.sync(() => { + processReleased = true; + }), ), - () => - Effect.sync(() => { - processReleased = true; - }), - ), ), }); assert.deepStrictEqual(stopped, []); @@ -1860,24 +1892,37 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); - it.effect("launches an OhMyPi preset with its own role overlay and main model", () => + it.effect("switches an OhMyPi thread between presets and models through its own overlay", () => Effect.gen(function* () { const fake = yield* makeFakePi; - const adapter = yield* makeOmpPresetAdapter(fake); - const runtime = yield* adapter.openSession({ + const { runtime, takeEvent } = yield* openOmpRuntime(fake, "default"); + const args = fake.lastSpawn().args; + const overlayPath = args[args.indexOf("--config") + 1]!; + const readOverlay = Effect.gen(function* () { + return decodeJsonLine(yield* (yield* FileSystem.FileSystem).readFileString(overlayPath)); + }); + assert.deepEqual(yield* readOverlay, {}); + const providerThread = yield* runtime.ensureThread({ threadId: THREAD_ID, - providerSessionId: SESSION_ID, - modelSelection: modelSelection("preset:Anthropic"), + modelSelection: modelSelection("default"), runtimePolicy, }); - const args = fake.lastSpawn().args; - const overlayPath = args[args.indexOf("--config") + 1]; - assert.isDefined(overlayPath); - const overlay = decodeJsonLine( - yield* (yield* FileSystem.FileSystem).readFileString(overlayPath!), + const settle = Effect.gen(function* () { + yield* fake.emit({ type: "agent_start" }); + yield* fake.emit({ type: "agent_end", messages: [], willRetry: false }); + yield* fake.emit({ type: "agent_settled" }); + yield* takeEvent((event) => event.type === "turn.terminal"); + }); + + yield* startTurn(runtime, providerThread, "preset:Anthropic"); + const anthropicModel = yield* fake.takeRequest("set_model"); + assert.deepEqual( + [anthropicModel["provider"], anthropicModel["modelId"]], + ["anthropic", "claude-opus"], ); + assert.equal((yield* fake.takeRequest("set_thinking_level"))["level"], "high"); // Roles the preset leaves out are tombstoned so global roles cannot leak in. - assert.deepEqual(overlay, { + assert.deepEqual(yield* readOverlay, { modelRoles: { default: "anthropic/claude-opus:high", designer: null, @@ -1885,91 +1930,52 @@ describe("PiAdapterV2", () => { }, defaultThinkingLevel: "auto", }); + yield* settle; - const providerThread = yield* runtime.ensureThread({ - threadId: THREAD_ID, - modelSelection: modelSelection("preset:Anthropic"), - runtimePolicy, + yield* startTurn(runtime, providerThread, "preset:Auto", [], "Hello pi", undefined, 2); + const autoModel = yield* fake.takeRequest("set_model"); + assert.deepEqual([autoModel["provider"], autoModel["modelId"]], ["openai", "gpt"]); + assert.equal((yield* fake.takeRequest("set_thinking_level"))["level"], "auto"); + assert.deepEqual(yield* readOverlay, { + modelRoles: { default: "openai/gpt", designer: null }, + defaultThinkingLevel: "auto", }); - yield* startTurn(runtime, providerThread, "preset:Anthropic"); - const modelRequest = yield* fake.takeRequest("set_model"); - assert.equal(modelRequest["provider"], "anthropic"); - assert.equal(modelRequest["modelId"], "claude-opus"); - const levelRequest = yield* fake.takeRequest("set_thinking_level"); - assert.equal(levelRequest["level"], "high"); + yield* settle; + + yield* startTurn(runtime, providerThread, "xai/grok-4.6", [], "Hello pi", undefined, 3); + const plainModel = yield* fake.takeRequest("set_model"); + assert.deepEqual([plainModel["provider"], plainModel["modelId"]], ["xai", "grok-4.6"]); + assert.deepEqual(yield* readOverlay, {}); }).pipe(Effect.scoped, Effect.provide(layerTest)), ); - it.effect("applies a preset's auto thinking over the resumed session's level", () => + it.effect("fails a turn on a missing preset without changing the model", () => Effect.gen(function* () { const fake = yield* makeFakePi; - fake.queueState({ thinkingLevel: "high" }); - const adapter = yield* makeOmpPresetAdapter(fake); - const runtime = yield* adapter.openSession({ - threadId: THREAD_ID, - providerSessionId: SESSION_ID, - modelSelection: modelSelection("preset:Auto"), - runtimePolicy, - }); + const { runtime } = yield* openOmpRuntime(fake, "preset:Gone"); const providerThread = yield* runtime.ensureThread({ threadId: THREAD_ID, - modelSelection: modelSelection("preset:Auto"), + modelSelection: modelSelection("preset:Gone"), runtimePolicy, }); - yield* startTurn(runtime, providerThread, "preset:Auto"); - const levelRequest = yield* fake.takeRequest("set_thinking_level"); - assert.equal(levelRequest["level"], "auto"); + const error = yield* startTurn(runtime, providerThread, "preset:Gone").pipe(Effect.flip); + assert.include(String(error.cause), "preset 'Gone' was not found"); + assert.isUndefined(fake.allRequests().find((record) => record["type"] === "set_model")); }).pipe(Effect.scoped, Effect.provide(layerTest)), ); - it.effect("fails a preset session that names a missing preset", () => + it.effect("rejects a preset on plain Pi, which gets no config overlay", () => Effect.gen(function* () { const fake = yield* makeFakePi; - const adapter = yield* makeOmpPresetAdapter(fake); - const error = yield* adapter - .openSession({ - threadId: THREAD_ID, - providerSessionId: SESSION_ID, - modelSelection: modelSelection("preset:Gone"), - runtimePolicy, - }) - .pipe(Effect.flip); - assert.include(error.message, "preset 'Gone' was not found"); - }).pipe(Effect.scoped, Effect.provide(layerTest)), - ); - - it.effect("restarts the session only when the selected preset changes", () => - Effect.gen(function* () { - const adapter = yield* makeAdapter(yield* makeFakePi); - const sessionCapabilities = yield* adapter.getCapabilities(); - const plan = (current: ModelSelection, target: ModelSelection) => - adapter - .planSelectionTransition({ - current, - target, - sessionCapabilities, - }) - .pipe(Effect.map((result) => result.type)); - const thinking = (model: string, value: string): ModelSelection => ({ - ...modelSelection(model), - options: [{ id: "thinking", value }], + const { runtime } = yield* openRuntime(fake); + assert.isFalse(fake.lastSpawn().args.includes("--config")); + const providerThread = yield* runtime.ensureThread({ + threadId: THREAD_ID, + modelSelection: modelSelection("default"), + runtimePolicy, }); - assert.equal( - yield* plan(modelSelection("preset:Anthropic"), modelSelection("preset:OpenAI")), - "restart_session", - ); - assert.equal( - yield* plan(modelSelection("xai/grok-4.6"), modelSelection("preset:OpenAI")), - "restart_session", - ); - assert.equal( - yield* plan(modelSelection("preset:OpenAI"), thinking("preset:OpenAI", "low")), - "apply_on_next_turn", - ); - assert.equal( - yield* plan(modelSelection("xai/grok-4.6"), modelSelection("default")), - "apply_on_next_turn", - ); + const error = yield* startTurn(runtime, providerThread, "preset:Anthropic").pipe(Effect.flip); + assert.include(String(error.cause), "point to OhMyPi"); }).pipe(Effect.scoped, Effect.provide(layerTest)), ); diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index 41f16f642bbe1..55f78f862dad7 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -72,14 +72,13 @@ import { } from "./mcpInjection.ts"; import { PI_FILE_CHANGE_TOOLS } from "./mcpExtensionSource.ts"; import { - ompPresetThinkingLevel, parseOmpModelSelector, encodeOmpConfigOverlay, + isOmpBinary, OMP_AUTO_THINKING, piModelInventorySlugs, piPresetNameFromSlug, resolveOmpPreset, - type OmpModelPreset, } from "./presets.ts"; export const PI_PROVIDER = ProviderDriverKind.make("pi"); @@ -422,6 +421,11 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( options: PiAdapterV2Options, ) { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const detectOmpBinary = yield* Effect.cached( + isOmpBinary(options.settings.binaryPath || "pi", options.environment).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + ), + ); const agentScope = yield* AgentScope; const fileSystem = yield* FileSystem.FileSystem; const idAllocator = yield* IdAllocator.IdAllocatorV2; @@ -442,15 +446,8 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( getCapabilities: Effect.fn("PiAdapter.getCapabilities")(function () { return Effect.succeed(PiProviderCapabilitiesV2); }), - planSelectionTransition: Effect.fn("PiAdapter.planSelectionTransition")(function (input) { - // A preset's role models live in the launch-time `--config` overlay. - const currentPreset = piPresetNameFromSlug(input.current.model); - const targetPreset = piPresetNameFromSlug(input.target.model); - return Effect.succeed( - currentPreset !== targetPreset - ? ({ type: "restart_session" } as const) - : turnScopedSelectionTransition(), - ); + planSelectionTransition: Effect.fn("PiAdapter.planSelectionTransition")(function () { + return Effect.succeed(turnScopedSelectionTransition()); }), openSession: Effect.fn("PiAdapterV2.openSession")(function* ( input: ProviderAdapter.ProviderAdapterV2OpenSessionInput, @@ -478,42 +475,32 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( if (!resolvedLaunchArgs.ok) { return yield* protocolError(resolvedLaunchArgs.message); } - const presetName = piPresetNameFromSlug(input.modelSelection.model); - let sessionPreset: OmpModelPreset | null = null; - let configOverlayPath: string | undefined; - if (presetName !== undefined) { - const resolved = yield* resolveOmpPreset({ - binaryPath: options.settings.binaryPath || "pi", - environment: options.environment, - cwd, - launchArgs: resolvedLaunchArgs.args, - name: presetName, - }).pipe( - Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), - Effect.mapError((cause) => protocolError(cause.message)), - ); - sessionPreset = resolved.preset; - const overlayDir = `${host.paths.providerStatusCacheDir.replace(/\\/g, "/")}/pi-presets`; - const overlayPath = `${overlayDir}/${input.providerSessionId.replace(/[^\w-]/g, "_")}.json`; - // JSON is valid YAML, which is what omp reads `--config` overlays as. - yield* provideCacheFs( - Effect.gen(function* () { - yield* fileSystem.makeDirectory(overlayDir, { recursive: true }); - yield* fileSystem.writeFileString( - overlayPath, - encodeOmpConfigOverlay(resolved.overlay), - ); - }), - ); + // Every omp session gets its own `--config` overlay so a preset never touches omp's global config. + const overlayPath = (yield* detectOmpBinary) + ? `${host.paths.providerStatusCacheDir.replace(/\\/g, "/")}/pi-presets/${input.providerSessionId.replace(/[^\w-]/g, "_")}.json` + : undefined; + const writeOverlay = (overlay: Record) => + overlayPath === undefined + ? Effect.void + : Effect.gen(function* () { + const temporaryPath = `${overlayPath}.tmp`; + yield* fileSystem.makeDirectory(overlayPath.slice(0, overlayPath.lastIndexOf("/")), { + recursive: true, + }); + // JSON is valid YAML; the rename keeps omp's file watcher from reading a partial write. + yield* fileSystem.writeFileString(temporaryPath, encodeOmpConfigOverlay(overlay)); + yield* fileSystem.rename(temporaryPath, overlayPath); + }); + if (overlayPath !== undefined) { + yield* provideCacheFs(writeOverlay({})); yield* Effect.addFinalizer(() => fileSystem.remove(overlayPath).pipe(Effect.ignore)); - configOverlayPath = overlayPath; } const launch = buildPiRpcLaunch({ launchArgs: resolvedLaunchArgs.args, environment: options.environment, mcpSession, extensionPath, - configOverlayPath, + configOverlayPath: overlayPath, runtimeMode: input.runtimePolicy.runtimeMode, }); const scopedLaunch = yield* agentScope.wrap({ @@ -2290,44 +2277,58 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( return providerThread; }); - let presetKnownModels: ReadonlySet | null = null; - let presetMainModel: string | null = null; + // The preset whose roles the overlay holds; null when it holds none. + let appliedPreset: { + readonly mainModel: string; + readonly thinking: string | undefined; + } | null = null; + const writeSessionOverlay = (overlay: Record) => + writeOverlay(overlay).pipe(Effect.mapError((cause) => protocolError(cause.message))); const applyPresetSelection = Effect.fnUntraced(function* ( modelSelection: ModelSelection, presetName: string, ) { - if (sessionPreset?.name !== presetName) { + if (overlayPath === undefined) { return yield* protocolError( - `OhMyPi preset '${presetName}' needs a session restart to apply`, + `Model preset '${presetName}' needs Pi's binary path to point to OhMyPi (omp)`, ); } - const defaultSelector = sessionPreset.modelRoles.default; - if (presetKnownModels === null && defaultSelector !== undefined) { - presetKnownModels = piModelInventorySlugs( + if (modelSelection.model !== appliedModel) { + const resolved = yield* resolveOmpPreset({ + binaryPath: options.settings.binaryPath || "pi", + environment: options.environment, + cwd, + launchArgs: resolvedLaunchArgs.args, + name: presetName, + }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.mapError((cause) => protocolError(cause.message)), + ); + const knownModels = piModelInventorySlugs( yield* request({ type: "get_available_models" }), ); - } - const knownModels = presetKnownModels ?? new Set(); - const thinking = - getModelSelectionStringOptionValue(modelSelection, "thinking") ?? - ompPresetThinkingLevel(sessionPreset, knownModels); - if (modelSelection.model !== appliedModel) { - // Without a `default` role, omp picks the main model itself, as `/modelpreset switch` does. - if (defaultSelector !== undefined) { - const parsed = parseOmpModelSelector(defaultSelector, knownModels); - if (parsed === null) { - return yield* protocolError( - `OhMyPi preset '${presetName}' main model '${defaultSelector}' must use provider/model format`, - ); - } - const selectedModel = yield* request({ - type: "set_model", - provider: parsed.provider, - modelId: parsed.modelId, - }); - contextWindow = rememberModelContextWindow(selectedModel); - presetMainModel = `${parsed.provider}/${parsed.modelId}`; + const mainSelector = resolved.preset.modelRoles.default ?? ""; + const parsed = parseOmpModelSelector(mainSelector, knownModels); + if (parsed === null) { + return yield* protocolError( + `OhMyPi preset '${presetName}' main model '${mainSelector}' must use provider/model format`, + ); } + // shortcut: omp reloads a rewritten overlay ~200ms later, so role models switch from the + // next role lookup; the main model and thinking switch now over RPC. Wait for a reload + // signal if omp ever exposes one. + yield* writeSessionOverlay(resolved.overlay); + const selectedModel = yield* request({ + type: "set_model", + provider: parsed.provider, + modelId: parsed.modelId, + }); + contextWindow = rememberModelContextWindow(selectedModel); + appliedPreset = { + mainModel: `${parsed.provider}/${parsed.modelId}`, + thinking: parsed.thinking ?? resolved.preset.defaultThinkingLevel, + }; + appliedThinking = null; appliedModel = modelSelection.model; const updatedAt = yield* DateTime.now; sessionEntity = { ...sessionEntity, model: modelSelection.model, updatedAt }; @@ -2337,6 +2338,8 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( providerSession: sessionEntity, }); } + const thinking = + getModelSelectionStringOptionValue(modelSelection, "thinking") ?? appliedPreset?.thinking; if ( thinking !== undefined && thinking !== appliedThinking && @@ -2352,6 +2355,10 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( if (selectedPreset !== undefined) { return yield* applyPresetSelection(modelSelection, selectedPreset); } + if (appliedPreset !== null) { + yield* writeSessionOverlay({}); + appliedPreset = null; + } const thinking = getModelSelectionStringOptionValue(modelSelection, "thinking"); if (modelSelection.model === PI_INHERIT_MODEL_SLUG) { if (appliedModel !== null && baselineModel !== null) { @@ -2454,7 +2461,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( ? undefined : `${baselineModel.provider}/${baselineModel.modelId}` : piPresetNameFromSlug(selection.model) !== undefined - ? (presetMainModel ?? undefined) + ? appliedPreset?.mainModel : selection.model; return slug === undefined ? undefined : modelContextWindows.get(slug); }, diff --git a/packages/provider-pi/src/server/presets.ts b/packages/provider-pi/src/server/presets.ts index 7ab77770b7256..6e1573a78aecd 100644 --- a/packages/provider-pi/src/server/presets.ts +++ b/packages/provider-pi/src/server/presets.ts @@ -65,6 +65,25 @@ export function isOmpVersionOutput(output: string): boolean { return /^omp\/\d/m.test(output); } +/** Whether the configured Pi binary is OhMyPi; any failure reads as plain Pi. */ +export const isOmpBinary = Effect.fn("PiPresets.isOmpBinary")( + function* (binaryPath: string, environment: NodeJS.ProcessEnv) { + const spawnCommand = yield* resolveSpawnCommand(binaryPath, ["--version"], { + env: environment, + }); + const result = yield* spawnAndCollect( + binaryPath, + ChildProcess.make(spawnCommand.command, spawnCommand.args, { + env: environment, + shell: spawnCommand.shell, + }), + ); + return result.code === 0 && isOmpVersionOutput(result.stdout); + }, + Effect.timeoutOrElse({ duration: OMP_CONFIG_TIMEOUT_MS, orElse: () => Effect.succeed(false) }), + Effect.orElseSucceed(() => false), +); + function isStringRecord(input: unknown): input is Record { return Predicate.isObject(input) && !Array.isArray(input); } @@ -87,8 +106,10 @@ function parseOmpModelPresets(value: unknown): ReadonlyArray { modelRoles[role] = selector; } const level = raw.defaultThinkingLevel; + // T3 sets the main model from `default`; without one omp would pick a model T3 cannot see. if ( !valid || + modelRoles.default === undefined || (level !== undefined && (typeof level !== "string" || !OMP_DEFAULT_THINKING_LEVELS.has(level))) ) { @@ -139,7 +160,7 @@ export function parseOmpModelSelector( } /** The level a preset starts at: the `default` selector's suffix wins over `defaultThinkingLevel`. */ -export function ompPresetThinkingLevel( +function ompPresetThinkingLevel( preset: OmpModelPreset, knownModels: ReadonlySet, ): string | undefined { @@ -298,7 +319,7 @@ export const resolveOmpPreset = Effect.fn("PiPresets.resolveOmpPreset")(function if (preset === undefined) { return yield* new PiRpcError({ operation: "omp_preset", - detail: `OhMyPi model preset '${input.name}' was not found or is malformed`, + detail: `OhMyPi model preset '${input.name}' was not found, is malformed, or has no main (default) model`, }); } return { preset, overlay: ompPresetOverlay(preset, isStringRecord(roles) ? roles : {}) }; diff --git a/packages/provider-pi/src/server/textGeneration.ts b/packages/provider-pi/src/server/textGeneration.ts index 91b02cb962ddb..77514a5d32095 100644 --- a/packages/provider-pi/src/server/textGeneration.ts +++ b/packages/provider-pi/src/server/textGeneration.ts @@ -52,8 +52,7 @@ export const makePiTextGeneration = Effect.fn("makePiTextGeneration")(function* detail: `OhMyPi model preset '${presetName}' was not found or is malformed.`, }); } - const selector = preset.modelRoles.default; - if (selector === undefined) return "default"; + const selector = preset.modelRoles.default ?? ""; const knownModels = piModelInventorySlugs(yield* listModels); const parsed = parseOmpModelSelector(selector, knownModels); if (parsed === null) { From 6b508f2dc420d2fe5ec494831dc0b6a34752c9a3 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Mon, 26 Oct 2026 23:25:39 +0000 Subject: [PATCH 04/11] fix(pi): recover the overlay after a failed preset switch The overlay's preset is tracked separately from the applied model, so returning to the previous selection after a failed set_model clears or rewrites the overlay. Context-window lookups only answer for the applied preset. --- .../provider-pi/src/server/adapter.test.ts | 32 +++++++++++++++++++ packages/provider-pi/src/server/adapter.ts | 15 ++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index 01dcef662764a..e1f7e0c7d53c4 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -165,6 +165,7 @@ interface FakePi { readonly failNextState: () => void; readonly deferNextLifecycle: (type: "switch_session" | "new_session" | "fork") => void; readonly deferNextModelSelection: () => void; + readonly failNextModelSelection: () => void; readonly queueModels: (models: ReadonlyArray) => void; readonly vetoNextNewSession: () => void; readonly allRequests: () => ReadonlyArray; @@ -216,6 +217,7 @@ const makeFakePi: Effect.Effect = Effect.gen(function* () { let deferState = false; let deferredStateRequest: PiRpcRecord | undefined; let failState = false; + let failModelSelection = false; let vetoSwitch = false; let vetoNewSession = false; let deferredLifecycle: string | undefined; @@ -269,6 +271,10 @@ const makeFakePi: Effect.Effect = Effect.gen(function* () { return base; case "get_available_models": return { ...base, data: { models } }; + case "set_model": + if (!failModelSelection) return base; + failModelSelection = false; + return { ...base, success: false, error: "No API key for anthropic" }; case "new_session": { const cancelled = vetoNewSession; vetoNewSession = false; @@ -389,6 +395,9 @@ const makeFakePi: Effect.Effect = Effect.gen(function* () { deferNextModelSelection: () => { deferredLifecycle = "set_model"; }, + failNextModelSelection: () => { + failModelSelection = true; + }, queueModels: (value) => { models = value; }, @@ -1949,6 +1958,29 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); + it.effect("clears a preset overlay whose switch failed when the thread switches back", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + const { runtime } = yield* openOmpRuntime(fake, "xai/grok-4.6"); + const args = fake.lastSpawn().args; + const overlayPath = args[args.indexOf("--config") + 1]!; + const providerThread = yield* runtime.ensureThread({ + threadId: THREAD_ID, + modelSelection: modelSelection("xai/grok-4.6"), + runtimePolicy, + }); + fake.failNextModelSelection(); + yield* startTurn(runtime, providerThread, "preset:Anthropic").pipe(Effect.flip); + + yield* startTurn(runtime, providerThread, "xai/grok-4.6", [], "Hello pi", undefined, 2); + yield* fake.takeRequest("prompt"); + assert.deepEqual( + decodeJsonLine(yield* (yield* FileSystem.FileSystem).readFileString(overlayPath)), + {}, + ); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + it.effect("fails a turn on a missing preset without changing the model", () => Effect.gen(function* () { const fake = yield* makeFakePi; diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index 55f78f862dad7..607bb6e23af54 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -2277,8 +2277,10 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( return providerThread; }); - // The preset whose roles the overlay holds; null when it holds none. + // Tracked apart from the applied model: a failed set_model leaves the overlay already rewritten. + let overlayPreset: string | null = null; let appliedPreset: { + readonly slug: string; readonly mainModel: string; readonly thinking: string | undefined; } | null = null; @@ -2293,7 +2295,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( `Model preset '${presetName}' needs Pi's binary path to point to OhMyPi (omp)`, ); } - if (modelSelection.model !== appliedModel) { + if (overlayPreset !== presetName || modelSelection.model !== appliedModel) { const resolved = yield* resolveOmpPreset({ binaryPath: options.settings.binaryPath || "pi", environment: options.environment, @@ -2318,6 +2320,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( // next role lookup; the main model and thinking switch now over RPC. Wait for a reload // signal if omp ever exposes one. yield* writeSessionOverlay(resolved.overlay); + overlayPreset = presetName; const selectedModel = yield* request({ type: "set_model", provider: parsed.provider, @@ -2325,6 +2328,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( }); contextWindow = rememberModelContextWindow(selectedModel); appliedPreset = { + slug: modelSelection.model, mainModel: `${parsed.provider}/${parsed.modelId}`, thinking: parsed.thinking ?? resolved.preset.defaultThinkingLevel, }; @@ -2355,8 +2359,9 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( if (selectedPreset !== undefined) { return yield* applyPresetSelection(modelSelection, selectedPreset); } - if (appliedPreset !== null) { + if (overlayPreset !== null) { yield* writeSessionOverlay({}); + overlayPreset = null; appliedPreset = null; } const thinking = getModelSelectionStringOptionValue(modelSelection, "thinking"); @@ -2461,7 +2466,9 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( ? undefined : `${baselineModel.provider}/${baselineModel.modelId}` : piPresetNameFromSlug(selection.model) !== undefined - ? appliedPreset?.mainModel + ? appliedPreset?.slug === selection.model + ? appliedPreset.mainModel + : undefined : selection.model; return slug === undefined ? undefined : modelContextWindows.get(slug); }, From 9a9c4accb959620876dcea1ce7e2571d4f64eb66 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Mon, 26 Oct 2026 23:27:26 +0000 Subject: [PATCH 05/11] fix(pi): parse OhMyPi thinking abbreviations like omp --- docs/adr/0025-ohmypi-model-presets.md | 5 +- .../provider-pi/src/server/presets.test.ts | 5 ++ packages/provider-pi/src/server/presets.ts | 51 ++++++++----------- .../provider-pi/src/server/status.test.ts | 3 +- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/docs/adr/0025-ohmypi-model-presets.md b/docs/adr/0025-ohmypi-model-presets.md index bf800346f808d..acc9d55a88851 100644 --- a/docs/adr/0025-ohmypi-model-presets.md +++ b/docs/adr/0025-ohmypi-model-presets.md @@ -32,7 +32,10 @@ selection-transition plan, so switching has to work inside the live session. Presets without a `default` role are not listed, because T3 sets the main model from it, and one written as an alias rather than `provider/model` fails with a clear error. omp's `--smol`, `--slow`, and `--plan` flags and their -`PI_*_MODEL` variables are runtime overrides that outrank any overlay. Text +`PI_*_MODEL` variables are runtime overrides that outrank any overlay. `omp +config get` cannot read `--config` overlays from the instance's launch +arguments and returns same-name presets merged across config layers, where omp +takes the highest layer's entry whole; both are accepted gaps. Text generation has no per-model filter, so a preset chosen there uses only its main model. diff --git a/packages/provider-pi/src/server/presets.test.ts b/packages/provider-pi/src/server/presets.test.ts index c6a7f698901da..338160741dd93 100644 --- a/packages/provider-pi/src/server/presets.test.ts +++ b/packages/provider-pi/src/server/presets.test.ts @@ -19,6 +19,11 @@ describe("parseOmpModelSelector", () => { }); }); + it("expands an unambiguous thinking abbreviation, as omp does", () => { + assert.equal(parseOmpModelSelector("openai/gpt:hi", new Set())?.thinking, "high"); + assert.equal(parseOmpModelSelector("openai/gpt:m", new Set())?.modelId, "gpt:m"); + }); + it("rejects aliases and bare model names it cannot hand to set_model", () => { assert.isNull(parseOmpModelSelector("@slow", new Set())); assert.isNull(parseOmpModelSelector("claude-opus:high", new Set())); diff --git a/packages/provider-pi/src/server/presets.ts b/packages/provider-pi/src/server/presets.ts index 6e1573a78aecd..cdde1e147799b 100644 --- a/packages/provider-pi/src/server/presets.ts +++ b/packages/provider-pi/src/server/presets.ts @@ -16,27 +16,22 @@ import { const PI_PRESET_SLUG_PREFIX = "preset:"; const PRESET_NAME_PATTERN = /^[a-zA-Z][\w-]*$/; const OMP_CONFIG_TIMEOUT_MS = 10_000; -// omp accepts these for a preset's `defaultThinkingLevel`; `off` is only a selector suffix. -const OMP_DEFAULT_THINKING_LEVELS = new Set([ - "minimal", - "low", - "medium", - "high", - "xhigh", - "max", - "auto", -]); -const OMP_THINKING_SUFFIXES = new Set([ - "off", - "minimal", - "low", - "medium", - "high", - "xhigh", - "max", - "auto", - "inherit", -]); +const OMP_THINKING_LEVELS = ["inherit", "off", "minimal", "low", "medium", "high", "xhigh", "max"]; + +/** omp's thinking selector parsing: exact, or an unambiguous prefix of two or more characters. */ +function parseOmpThinkingLevel(value: string): string | undefined { + if (OMP_THINKING_LEVELS.includes(value)) return value; + if (value.length < 2) return undefined; + const matches = OMP_THINKING_LEVELS.filter((level) => level.startsWith(value)); + return matches.length === 1 ? matches[0] : undefined; +} + +/** omp's rule for a preset's `defaultThinkingLevel`: `auto` or an effort, never `off`/`inherit`. */ +function parseOmpDefaultThinkingLevel(value: string): string | undefined { + if (value === "auto") return value; + const level = parseOmpThinkingLevel(value); + return level === "off" || level === "inherit" ? undefined : level; +} const decodeOmpConfigOutput = Schema.decodeUnknownEffect( Schema.fromJsonString(Schema.Struct({ value: Schema.optional(Schema.Unknown) })), @@ -105,14 +100,10 @@ function parseOmpModelPresets(value: unknown): ReadonlyArray { } modelRoles[role] = selector; } - const level = raw.defaultThinkingLevel; + const rawLevel = raw.defaultThinkingLevel; + const level = typeof rawLevel === "string" ? parseOmpDefaultThinkingLevel(rawLevel) : undefined; // T3 sets the main model from `default`; without one omp would pick a model T3 cannot see. - if ( - !valid || - modelRoles.default === undefined || - (level !== undefined && - (typeof level !== "string" || !OMP_DEFAULT_THINKING_LEVELS.has(level))) - ) { + if (!valid || modelRoles.default === undefined || (rawLevel !== undefined && !level)) { continue; } presets.push({ name, modelRoles, defaultThinkingLevel: level }); @@ -150,9 +141,7 @@ export function parseOmpModelSelector( const colon = selector.lastIndexOf(":"); const suffix = colon === -1 ? undefined : selector.slice(colon + 1); const thinking = - suffix !== undefined && OMP_THINKING_SUFFIXES.has(suffix) && !knownModels.has(selector) - ? suffix - : undefined; + suffix !== undefined && !knownModels.has(selector) ? parseOmpThinkingLevel(suffix) : undefined; const model = thinking === undefined ? selector : selector.slice(0, colon); const separator = model.indexOf("/"); if (model.startsWith("@") || separator <= 0 || separator === model.length - 1) return null; diff --git a/packages/provider-pi/src/server/status.test.ts b/packages/provider-pi/src/server/status.test.ts index 0bf54c728c695..5c349499b34c2 100644 --- a/packages/provider-pi/src/server/status.test.ts +++ b/packages/provider-pi/src/server/status.test.ts @@ -186,11 +186,12 @@ describe("PiProvider", () => { { configCalls, modelPresets: { - OpenAI: { modelRoles: { default: "openai/gpt" } }, + OpenAI: { modelRoles: { default: "openai/gpt" }, defaultThinkingLevel: "min" }, Anthropic: { modelRoles: { default: "anthropic/claude-opus:low" } }, Broken: { modelRoles: { default: "" } }, NumericThinking: { modelRoles: { default: "openai/gpt" }, defaultThinkingLevel: 42 }, UnknownThinking: { modelRoles: { default: "openai/gpt" }, defaultThinkingLevel: "x" }, + NoMainModel: { modelRoles: { task: "openai/gpt" } }, }, }, ); From f558b0221dd69161a5bbe881e0e071136def99ac Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Mon, 26 Oct 2026 23:33:46 +0000 Subject: [PATCH 06/11] fix(pi): keep preset thinking and context lookups in step with the session A preset without a level falls back to the session's configured level, a switch marks thinking unknown so Pi default can restore it after a partial failure, and resume or rollback forget the applied preset. --- .../provider-pi/src/server/adapter.test.ts | 23 ++++++++++++++++ packages/provider-pi/src/server/adapter.ts | 27 ++++++++++++------- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index e1f7e0c7d53c4..ea3d636811ac0 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -462,6 +462,7 @@ const OMP_CONFIG_VALUES: Record = { defaultThinkingLevel: "auto", }, Auto: { modelRoles: { default: "openai/gpt" }, defaultThinkingLevel: "auto" }, + Unleveled: { modelRoles: { default: "openai/gpt" } }, }, modelRoles: { default: "openai/gpt", designer: "openai/gpt-mini" }, }; @@ -1958,6 +1959,28 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); + it.effect("returns a preset without a thinking level to the session's configured level", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + fake.queueState({ thinkingLevel: "medium" }); + const { runtime, takeEvent } = yield* openOmpRuntime(fake, "preset:Anthropic"); + const providerThread = yield* runtime.ensureThread({ + threadId: THREAD_ID, + modelSelection: modelSelection("preset:Anthropic"), + runtimePolicy, + }); + yield* startTurn(runtime, providerThread, "preset:Anthropic"); + assert.equal((yield* fake.takeRequest("set_thinking_level"))["level"], "high"); + yield* fake.emit({ type: "agent_start" }); + yield* fake.emit({ type: "agent_end", messages: [], willRetry: false }); + yield* fake.emit({ type: "agent_settled" }); + yield* takeEvent((event) => event.type === "turn.terminal"); + + yield* startTurn(runtime, providerThread, "preset:Unleveled", [], "Hello pi", undefined, 2); + assert.equal((yield* fake.takeRequest("set_thinking_level"))["level"], "medium"); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + it.effect("clears a preset overlay whose switch failed when the thread switches back", () => Effect.gen(function* () { const fake = yield* makeFakePi; diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index 607bb6e23af54..1db19530f8a49 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -313,6 +313,7 @@ function providerRef( return { driver: PI_PROVIDER, nativeId, strength }; } +const UNKNOWN_PI_THINKING = ""; const PI_THINKING_LEVELS = new Set(["off", "minimal", "low", "medium", "high", "xhigh", "max"]); interface PiStreamItemState { @@ -582,6 +583,13 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( }), ); let appliedModel: string | null = null; + // Tracked apart from the applied model: a failed set_model leaves the overlay already rewritten. + let overlayPreset: string | null = null; + let appliedPreset: { + readonly slug: string; + readonly mainModel: string; + readonly thinking: string | undefined; + } | null = null; let appliedThinking: string | null = null; let appliedSessionName: string | null = null; const outOfTurnExtensionErrors: Array = []; @@ -2183,6 +2191,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( // Failed lifecycle operations can still change native sessions; invalidate bindings and defaults. threadState = null; appliedModel = null; + appliedPreset = null; appliedThinking = null; appliedSessionName = null; baselineModel = null; @@ -2277,13 +2286,6 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( return providerThread; }); - // Tracked apart from the applied model: a failed set_model leaves the overlay already rewritten. - let overlayPreset: string | null = null; - let appliedPreset: { - readonly slug: string; - readonly mainModel: string; - readonly thinking: string | undefined; - } | null = null; const writeSessionOverlay = (overlay: Record) => writeOverlay(overlay).pipe(Effect.mapError((cause) => protocolError(cause.message))); const applyPresetSelection = Effect.fnUntraced(function* ( @@ -2330,9 +2332,15 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( appliedPreset = { slug: modelSelection.model, mainModel: `${parsed.provider}/${parsed.modelId}`, - thinking: parsed.thinking ?? resolved.preset.defaultThinkingLevel, + // Like omp, a preset without a level falls back to the configured one. + thinking: + parsed.thinking ?? + resolved.preset.defaultThinkingLevel ?? + baselineThinking ?? + undefined, }; - appliedThinking = null; + // set_model may change the level; a non-null unknown still lets Pi default restore it. + appliedThinking = UNKNOWN_PI_THINKING; appliedModel = modelSelection.model; const updatedAt = yield* DateTime.now; sessionEntity = { ...sessionEntity, model: modelSelection.model, updatedAt }; @@ -3027,6 +3035,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( ); lastNativeThreadId = forkSessionFile; appliedModel = null; + appliedPreset = null; appliedThinking = null; appliedSessionName = null; lastKnownLeaf = leafId; From 1ccc6727149e9466336f29474e3b0b9ba772112d Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Mon, 26 Oct 2026 23:38:12 +0000 Subject: [PATCH 07/11] fix(pi): fall back to omp's configured thinking for unleveled presets --- .../provider-pi/src/server/adapter.test.ts | 8 +++++--- packages/provider-pi/src/server/adapter.ts | 3 +-- packages/provider-pi/src/server/presets.ts | 18 +++++++++++++++--- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index ea3d636811ac0..93a9711731d5d 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -465,6 +465,7 @@ const OMP_CONFIG_VALUES: Record = { Unleveled: { modelRoles: { default: "openai/gpt" } }, }, modelRoles: { default: "openai/gpt", designer: "openai/gpt-mini" }, + defaultThinkingLevel: "low", }; /** Answers `omp config get --json` and hands every other spawn to the fake Pi. */ @@ -1959,10 +1960,11 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); - it.effect("returns a preset without a thinking level to the session's configured level", () => + it.effect("returns a preset without a thinking level to omp's configured level", () => Effect.gen(function* () { const fake = yield* makeFakePi; - fake.queueState({ thinkingLevel: "medium" }); + // A resumed transcript's level must not stand in for omp's configured default. + fake.queueState({ thinkingLevel: "high" }); const { runtime, takeEvent } = yield* openOmpRuntime(fake, "preset:Anthropic"); const providerThread = yield* runtime.ensureThread({ threadId: THREAD_ID, @@ -1977,7 +1979,7 @@ describe("PiAdapterV2", () => { yield* takeEvent((event) => event.type === "turn.terminal"); yield* startTurn(runtime, providerThread, "preset:Unleveled", [], "Hello pi", undefined, 2); - assert.equal((yield* fake.takeRequest("set_thinking_level"))["level"], "medium"); + assert.equal((yield* fake.takeRequest("set_thinking_level"))["level"], "low"); }).pipe(Effect.scoped, Effect.provide(layerTest)), ); diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index 1db19530f8a49..586eeeb3c9421 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -2336,8 +2336,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( thinking: parsed.thinking ?? resolved.preset.defaultThinkingLevel ?? - baselineThinking ?? - undefined, + resolved.configuredThinkingLevel, }; // set_model may change the level; a non-null unknown still lets Pi default restore it. appliedThinking = UNKNOWN_PI_THINKING; diff --git a/packages/provider-pi/src/server/presets.ts b/packages/provider-pi/src/server/presets.ts index cdde1e147799b..84b455fcd6e78 100644 --- a/packages/provider-pi/src/server/presets.ts +++ b/packages/provider-pi/src/server/presets.ts @@ -300,8 +300,12 @@ export const readOmpModelPresets = (input: OmpConfigReadInput) => export const resolveOmpPreset = Effect.fn("PiPresets.resolveOmpPreset")(function* ( input: OmpConfigReadInput & { readonly name: string }, ) { - const [presets, roles] = yield* Effect.all( - [readOmpModelPresets(input), readOmpConfigValue({ ...input, key: "modelRoles" })], + const [presets, roles, configuredThinking] = yield* Effect.all( + [ + readOmpModelPresets(input), + readOmpConfigValue({ ...input, key: "modelRoles" }), + readOmpConfigValue({ ...input, key: "defaultThinkingLevel" }), + ], { concurrency: "unbounded" }, ); const preset = presets.find((candidate) => candidate.name === input.name); @@ -311,5 +315,13 @@ export const resolveOmpPreset = Effect.fn("PiPresets.resolveOmpPreset")(function detail: `OhMyPi model preset '${input.name}' was not found, is malformed, or has no main (default) model`, }); } - return { preset, overlay: ompPresetOverlay(preset, isStringRecord(roles) ? roles : {}) }; + return { + preset, + overlay: ompPresetOverlay(preset, isStringRecord(roles) ? roles : {}), + // What omp falls back to when the preset sets no level. + configuredThinkingLevel: + typeof configuredThinking === "string" + ? parseOmpDefaultThinkingLevel(configuredThinking) + : undefined, + }; }); From 244b9fe9809d3070fb7869070e66aaa8ed1b681f Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 27 Oct 2026 00:08:38 +0000 Subject: [PATCH 08/11] fix(pi): pass the OhMyPi overlay through PI_CONFIG_FILES Probing the binary with --version added a process that Pi replay fixtures do not expect, and per-session overlay names could exceed NAME_MAX. omp reads overlays from PI_CONFIG_FILES and plain Pi ignores it, so every session gets a short-named overlay there with no probe. --- docs/adr/0025-ohmypi-model-presets.md | 5 +- .../provider-pi/src/server/adapter.test.ts | 53 ++++++------------- packages/provider-pi/src/server/adapter.ts | 52 +++++++----------- .../src/server/mcpInjection.test.ts | 14 +++++ .../provider-pi/src/server/mcpInjection.ts | 11 ++-- packages/provider-pi/src/server/presets.ts | 19 ------- 6 files changed, 60 insertions(+), 94 deletions(-) diff --git a/docs/adr/0025-ohmypi-model-presets.md b/docs/adr/0025-ohmypi-model-presets.md index acc9d55a88851..6facf4cbefc6f 100644 --- a/docs/adr/0025-ohmypi-model-presets.md +++ b/docs/adr/0025-ohmypi-model-presets.md @@ -19,8 +19,9 @@ the presets, not the discovered models. Applying a preset with omp's own `/modelpreset switch` writes roles to the user's global config, and every running omp process reloads it, so one thread would change all the others and the user's terminal. T3 instead starts every -omp session with its own `--config` overlay file in T3's cache, empty until a -turn selects a preset. Selecting one rewrites the overlay with the preset's +Pi session with its own overlay file in T3's cache, listed last in +`PI_CONFIG_FILES` (omp reads it like `--config`; plain Pi ignores it), empty +until a turn selects a preset. Selecting one rewrites the overlay with the preset's roles, `null` tombstones for roles the preset leaves out (as `/modelpreset switch` clears them), and its thinking level, read fresh from omp; then the turn sets the main model and thinking over RPC. Selecting a plain model empties diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index 93a9711731d5d..a1c128271c358 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -331,9 +331,6 @@ const makeFakePi: Effect.Effect = Effect.gen(function* () { }; const spawner = ChildProcessSpawner.make((command) => Effect.sync(() => { - if (ChildProcess.isStandardCommand(command) && command.args.includes("--version")) { - return exitedHandle("pi 0.84.3\n"); - } if (ChildProcess.isStandardCommand(command)) { lastSpawn = { args: command.args, @@ -472,7 +469,6 @@ const OMP_CONFIG_VALUES: Record = { const makeOmpPresetAdapter = (fake: FakePi) => { const spawner = ChildProcessSpawner.make((command) => { const args = ChildProcess.isStandardCommand(command) ? command.args : []; - if (args.includes("--version")) return Effect.succeed(exitedHandle("omp/18.5.1\n")); if (!args.includes("config")) return fake.spawner.spawn(command); return Effect.succeed( exitedHandle( @@ -1546,22 +1542,20 @@ describe("PiAdapterV2", () => { yield* openRuntime({ ...fake, spawner: ChildProcessSpawner.make((command) => - ChildProcess.isStandardCommand(command) && command.args.includes("--version") - ? fake.spawner.spawn(command) - : Effect.acquireRelease( - fake.spawner.spawn(command).pipe( - Effect.map((child) => - ChildProcessSpawner.makeHandle({ - ...child, - exitCode: Deferred.await(exited), - }), - ), - ), - () => - Effect.sync(() => { - processReleased = true; - }), + Effect.acquireRelease( + fake.spawner.spawn(command).pipe( + Effect.map((child) => + ChildProcessSpawner.makeHandle({ + ...child, + exitCode: Deferred.await(exited), + }), ), + ), + () => + Effect.sync(() => { + processReleased = true; + }), + ), ), }); assert.deepStrictEqual(stopped, []); @@ -1907,8 +1901,7 @@ describe("PiAdapterV2", () => { Effect.gen(function* () { const fake = yield* makeFakePi; const { runtime, takeEvent } = yield* openOmpRuntime(fake, "default"); - const args = fake.lastSpawn().args; - const overlayPath = args[args.indexOf("--config") + 1]!; + const overlayPath = fake.lastSpawn().env.PI_CONFIG_FILES!; const readOverlay = Effect.gen(function* () { return decodeJsonLine(yield* (yield* FileSystem.FileSystem).readFileString(overlayPath)); }); @@ -1987,8 +1980,7 @@ describe("PiAdapterV2", () => { Effect.gen(function* () { const fake = yield* makeFakePi; const { runtime } = yield* openOmpRuntime(fake, "xai/grok-4.6"); - const args = fake.lastSpawn().args; - const overlayPath = args[args.indexOf("--config") + 1]!; + const overlayPath = fake.lastSpawn().env.PI_CONFIG_FILES!; const providerThread = yield* runtime.ensureThread({ threadId: THREAD_ID, modelSelection: modelSelection("xai/grok-4.6"), @@ -2021,21 +2013,6 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); - it.effect("rejects a preset on plain Pi, which gets no config overlay", () => - Effect.gen(function* () { - const fake = yield* makeFakePi; - const { runtime } = yield* openRuntime(fake); - assert.isFalse(fake.lastSpawn().args.includes("--config")); - const providerThread = yield* runtime.ensureThread({ - threadId: THREAD_ID, - modelSelection: modelSelection("default"), - runtimePolicy, - }); - const error = yield* startTurn(runtime, providerThread, "preset:Anthropic").pipe(Effect.flip); - assert.include(String(error.cause), "point to OhMyPi"); - }).pipe(Effect.scoped, Effect.provide(layerTest)), - ); - it.effect.each([ { historical: false, label: "the latest turn" }, { historical: true, label: "a historical turn" }, diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index 586eeeb3c9421..2c97274ec2c09 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -30,6 +30,7 @@ import * as Option from "effect/Option"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; +import * as Hash from "effect/Hash"; import * as Fiber from "effect/Fiber"; import * as Queue from "effect/Queue"; import * as Schema from "effect/Schema"; @@ -74,7 +75,6 @@ import { PI_FILE_CHANGE_TOOLS } from "./mcpExtensionSource.ts"; import { parseOmpModelSelector, encodeOmpConfigOverlay, - isOmpBinary, OMP_AUTO_THINKING, piModelInventorySlugs, piPresetNameFromSlug, @@ -422,11 +422,6 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( options: PiAdapterV2Options, ) { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; - const detectOmpBinary = yield* Effect.cached( - isOmpBinary(options.settings.binaryPath || "pi", options.environment).pipe( - Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), - ), - ); const agentScope = yield* AgentScope; const fileSystem = yield* FileSystem.FileSystem; const idAllocator = yield* IdAllocator.IdAllocatorV2; @@ -476,32 +471,30 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( if (!resolvedLaunchArgs.ok) { return yield* protocolError(resolvedLaunchArgs.message); } - // Every omp session gets its own `--config` overlay so a preset never touches omp's global config. - const overlayPath = (yield* detectOmpBinary) - ? `${host.paths.providerStatusCacheDir.replace(/\\/g, "/")}/pi-presets/${input.providerSessionId.replace(/[^\w-]/g, "_")}.json` - : undefined; - const writeOverlay = (overlay: Record) => - overlayPath === undefined - ? Effect.void - : Effect.gen(function* () { - const temporaryPath = `${overlayPath}.tmp`; - yield* fileSystem.makeDirectory(overlayPath.slice(0, overlayPath.lastIndexOf("/")), { - recursive: true, - }); - // JSON is valid YAML; the rename keeps omp's file watcher from reading a partial write. - yield* fileSystem.writeFileString(temporaryPath, encodeOmpConfigOverlay(overlay)); - yield* fileSystem.rename(temporaryPath, overlayPath); - }); - if (overlayPath !== undefined) { - yield* provideCacheFs(writeOverlay({})); - yield* Effect.addFinalizer(() => fileSystem.remove(overlayPath).pipe(Effect.ignore)); - } + // Each session gets its own OhMyPi config overlay so a preset never touches omp's global config. + const overlayDir = `${host.paths.providerStatusCacheDir.replace(/\\/g, "/")}/pi-presets`; + // Session ids embed thread and project ids; keep the file name well under NAME_MAX. + const overlayName = `${input.providerSessionId.replace(/[^\w-]/g, "_").slice(-64)}-${(Hash.string(input.providerSessionId) >>> 0).toString(36)}`; + const overlayPath = `${overlayDir}/${overlayName}.json`; + const writeOverlay = Effect.fnUntraced(function* (overlay: Record) { + const temporaryPath = `${overlayPath}.tmp`; + // JSON is valid YAML; the rename keeps omp's file watcher from reading a partial write. + yield* fileSystem.writeFileString(temporaryPath, encodeOmpConfigOverlay(overlay)); + yield* fileSystem.rename(temporaryPath, overlayPath); + }); + // omp refuses to start when an overlay file is missing. + yield* provideCacheFs( + fileSystem + .makeDirectory(overlayDir, { recursive: true }) + .pipe(Effect.andThen(writeOverlay({}))), + ); + yield* Effect.addFinalizer(() => fileSystem.remove(overlayPath).pipe(Effect.ignore)); const launch = buildPiRpcLaunch({ launchArgs: resolvedLaunchArgs.args, environment: options.environment, mcpSession, extensionPath, - configOverlayPath: overlayPath, + configOverlay: { path: overlayPath, platform: yield* HostProcess.Platform }, runtimeMode: input.runtimePolicy.runtimeMode, }); const scopedLaunch = yield* agentScope.wrap({ @@ -2292,11 +2285,6 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( modelSelection: ModelSelection, presetName: string, ) { - if (overlayPath === undefined) { - return yield* protocolError( - `Model preset '${presetName}' needs Pi's binary path to point to OhMyPi (omp)`, - ); - } if (overlayPreset !== presetName || modelSelection.model !== appliedModel) { const resolved = yield* resolveOmpPreset({ binaryPath: options.settings.binaryPath || "pi", diff --git a/packages/provider-pi/src/server/mcpInjection.test.ts b/packages/provider-pi/src/server/mcpInjection.test.ts index 26d01b983039e..dda0ce4d59e85 100644 --- a/packages/provider-pi/src/server/mcpInjection.test.ts +++ b/packages/provider-pi/src/server/mcpInjection.test.ts @@ -22,6 +22,20 @@ const mcpSession = { }; describe("pi T3 MCP injection", () => { + it("lists T3's OhMyPi overlay after inherited ones so it wins", () => { + const launch = buildPiRpcLaunch({ + launchArgs: [], + environment: { PI_CONFIG_FILES: "/home/user/work.yml" }, + mcpSession: undefined, + extensionPath: undefined, + configOverlay: { path: "/tmp/cache/pi-presets/session.json", platform: "linux" }, + }); + assert.equal( + launch.env.PI_CONFIG_FILES, + "/home/user/work.yml:/tmp/cache/pi-presets/session.json", + ); + }); + it("always adds the permission bridge and configures MCP when available", () => { const resolvedArgs = resolvePiLaunchArgs( "--extension=/home/user/.pi/agent/extensions/demo.ts --session-dir=/tmp/pi-sessions --provider=anthropic --model=claude-sonnet --tools='' --name=-review --extension-flag=kept", diff --git a/packages/provider-pi/src/server/mcpInjection.ts b/packages/provider-pi/src/server/mcpInjection.ts index 31460222eb2ce..6c83f0340a1d4 100644 --- a/packages/provider-pi/src/server/mcpInjection.ts +++ b/packages/provider-pi/src/server/mcpInjection.ts @@ -243,8 +243,8 @@ export function buildPiRpcLaunch(input: { readonly mcpSession: McpProviderSessionConfig | undefined; readonly extensionPath: string | undefined; readonly sessionPath?: string | undefined; - /** OhMyPi `--config` overlay; later overlays win, so it follows user launch args. */ - readonly configOverlayPath?: string | undefined; + /** OhMyPi config overlay, passed in `PI_CONFIG_FILES`, which plain Pi ignores. */ + readonly configOverlay?: { readonly path: string; readonly platform: NodeJS.Platform }; readonly ephemeral?: boolean; readonly disableExtensions?: boolean; readonly disableTools?: boolean; @@ -271,7 +271,6 @@ export function buildPiRpcLaunch(input: { ? ["--session", input.sessionPath] : []), ...launchArgs, - ...(input.configOverlayPath === undefined ? [] : ["--config", input.configOverlayPath]), // Restrictions must follow launch args so user flags cannot re-enable unattended tools/extensions. ...(input.disableExtensions === true ? ["--no-extensions"] : []), ...(input.disableTools === true ? ["--no-tools"] : []), @@ -288,6 +287,12 @@ export function buildPiRpcLaunch(input: { delete environment[T3_MCP_URL_ENV]; delete environment[T3_MCP_BEARER_ENV]; delete environment[T3_PI_MCP_EXTENSION_PATH_ENV]; + if (input.configOverlay !== undefined) { + // Later overlays win, so T3's goes after any the user already passes. + environment.PI_CONFIG_FILES = [environment.PI_CONFIG_FILES, input.configOverlay.path] + .filter(Boolean) + .join(input.configOverlay.platform === "win32" ? ";" : ":"); + } return { args, diff --git a/packages/provider-pi/src/server/presets.ts b/packages/provider-pi/src/server/presets.ts index 84b455fcd6e78..370f85997c40e 100644 --- a/packages/provider-pi/src/server/presets.ts +++ b/packages/provider-pi/src/server/presets.ts @@ -60,25 +60,6 @@ export function isOmpVersionOutput(output: string): boolean { return /^omp\/\d/m.test(output); } -/** Whether the configured Pi binary is OhMyPi; any failure reads as plain Pi. */ -export const isOmpBinary = Effect.fn("PiPresets.isOmpBinary")( - function* (binaryPath: string, environment: NodeJS.ProcessEnv) { - const spawnCommand = yield* resolveSpawnCommand(binaryPath, ["--version"], { - env: environment, - }); - const result = yield* spawnAndCollect( - binaryPath, - ChildProcess.make(spawnCommand.command, spawnCommand.args, { - env: environment, - shell: spawnCommand.shell, - }), - ); - return result.code === 0 && isOmpVersionOutput(result.stdout); - }, - Effect.timeoutOrElse({ duration: OMP_CONFIG_TIMEOUT_MS, orElse: () => Effect.succeed(false) }), - Effect.orElseSucceed(() => false), -); - function isStringRecord(input: unknown): input is Record { return Predicate.isObject(input) && !Array.isArray(input); } From 8ae81d2228dd533ec9d2f69d3f0679ab809a18ca Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 27 Oct 2026 00:22:51 +0000 Subject: [PATCH 09/11] fix(pi): resolve preset context windows before their first turn Thread registration on a preset maps every preset to its main model, so handoff budgeting sees the right capacity before startTurn applies the preset. Adds a regression for long session ids, whose overlay names are now bounded. --- .../provider-pi/src/server/adapter.test.ts | 43 ++++++++++++++++++- packages/provider-pi/src/server/adapter.ts | 31 ++++++++++++- packages/provider-pi/src/server/presets.ts | 2 +- 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/packages/provider-pi/src/server/adapter.test.ts b/packages/provider-pi/src/server/adapter.test.ts index a1c128271c358..8545c63898350 100644 --- a/packages/provider-pi/src/server/adapter.test.ts +++ b/packages/provider-pi/src/server/adapter.test.ts @@ -483,11 +483,15 @@ const makeOmpPresetAdapter = (fake: FakePi) => { }).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner)); }; -const openOmpRuntime = Effect.fnUntraced(function* (fake: FakePi, model: string) { +const openOmpRuntime = Effect.fnUntraced(function* ( + fake: FakePi, + model: string, + providerSessionId = SESSION_ID, +) { const adapter = yield* makeOmpPresetAdapter(fake); const runtime = yield* adapter.openSession({ threadId: THREAD_ID, - providerSessionId: SESSION_ID, + providerSessionId, modelSelection: modelSelection(model), runtimePolicy, }); @@ -1976,6 +1980,41 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(layerTest)), ); + it.effect("keeps the overlay file name short for long session ids", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + // Delegated-task thread ids embed request keys of up to 256 characters. + yield* openOmpRuntime( + fake, + "default", + ProviderSessionId.make(`provider-session:${"k".repeat(300)}`), + ); + const overlayPath = fake.lastSpawn().env.PI_CONFIG_FILES!; + assert.isBelow(overlayPath.slice(overlayPath.lastIndexOf("/") + 1).length, 100); + assert.deepEqual( + decodeJsonLine(yield* (yield* FileSystem.FileSystem).readFileString(overlayPath)), + {}, + ); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + + it.effect("knows a preset's context window before its first turn", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + fake.queueModels([ + { provider: "anthropic", id: "claude-opus", contextWindow: 1_000_000 }, + { provider: "openai", id: "gpt", contextWindow: 200_000 }, + ]); + const { runtime } = yield* openOmpRuntime(fake, "preset:Anthropic"); + yield* runtime.ensureThread({ + threadId: THREAD_ID, + modelSelection: modelSelection("preset:Anthropic"), + runtimePolicy, + }); + assert.equal(runtime.getModelContextWindow?.(modelSelection("preset:Auto")), 200_000); + }).pipe(Effect.scoped, Effect.provide(layerTest)), + ); + it.effect("clears a preset overlay whose switch failed when the thread switches back", () => Effect.gen(function* () { const fake = yield* makeFakePi; diff --git a/packages/provider-pi/src/server/adapter.ts b/packages/provider-pi/src/server/adapter.ts index 2c97274ec2c09..dc901300fe253 100644 --- a/packages/provider-pi/src/server/adapter.ts +++ b/packages/provider-pi/src/server/adapter.ts @@ -78,6 +78,8 @@ import { OMP_AUTO_THINKING, piModelInventorySlugs, piPresetNameFromSlug, + piPresetSlug, + readOmpModelPresets, resolveOmpPreset, } from "./presets.ts"; @@ -578,6 +580,8 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( let appliedModel: string | null = null; // Tracked apart from the applied model: a failed set_model leaves the overlay already rewritten. let overlayPreset: string | null = null; + // Preset slug -> main model slug, so context lookups work before a preset's first turn. + let presetMainModels: Map | null = null; let appliedPreset: { readonly slug: string; readonly mainModel: string; @@ -2211,6 +2215,30 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( } } contextWindow = rememberModelContextWindow(recordField(stateData, "model")); + // Plain Pi has no `config` subcommand; a preset selection means the binary is omp. + if ( + presetMainModels === null && + piPresetNameFromSlug(threadInput.modelSelection.model) !== undefined + ) { + const presets = yield* readOmpModelPresets({ + binaryPath: options.settings.binaryPath || "pi", + environment: options.environment, + cwd, + launchArgs: resolvedLaunchArgs.args, + }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.orElseSucceed(() => []), + ); + const knownModels = new Set(modelContextWindows.keys()); + presetMainModels = new Map( + presets.flatMap((preset) => { + const parsed = parseOmpModelSelector(preset.modelRoles.default ?? "", knownModels); + return parsed === null + ? [] + : [[piPresetSlug(preset.name), `${parsed.provider}/${parsed.modelId}`] as const]; + }), + ); + } // Capture each baseline before applying selections so our own choices cannot become Pi defaults. if (baselineModel === null && appliedModel === null) { const stateModel = recordField(stateData, "model"); @@ -2317,6 +2345,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( modelId: parsed.modelId, }); contextWindow = rememberModelContextWindow(selectedModel); + presetMainModels?.set(modelSelection.model, `${parsed.provider}/${parsed.modelId}`); appliedPreset = { slug: modelSelection.model, mainModel: `${parsed.provider}/${parsed.modelId}`, @@ -2463,7 +2492,7 @@ export const makePiAdapterV2 = Effect.fn("makePiAdapterV2")(function* ( : piPresetNameFromSlug(selection.model) !== undefined ? appliedPreset?.slug === selection.model ? appliedPreset.mainModel - : undefined + : presetMainModels?.get(selection.model) : selection.model; return slug === undefined ? undefined : modelContextWindows.get(slug); }, diff --git a/packages/provider-pi/src/server/presets.ts b/packages/provider-pi/src/server/presets.ts index 370f85997c40e..cae1a879aaf41 100644 --- a/packages/provider-pi/src/server/presets.ts +++ b/packages/provider-pi/src/server/presets.ts @@ -46,7 +46,7 @@ export interface OmpModelPreset { readonly defaultThinkingLevel: string | undefined; } -function piPresetSlug(name: string): string { +export function piPresetSlug(name: string): string { return `${PI_PRESET_SLUG_PREFIX}${name}`; } From bf7a099a594fc280c8d9c653f6a56d72ab4a0fe1 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 27 Oct 2026 00:24:37 +0000 Subject: [PATCH 10/11] fix(pi): read OhMyPi presets after the RPC discovery deadline A preset read that started late in the 30-second discovery window could be cut off by it, discarding the discovered models. Presets are now read after discovery succeeds, under their own soft timeout. --- .../provider-pi/src/server/status.test.ts | 14 ++++++- packages/provider-pi/src/server/status.ts | 40 ++++++++++--------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/packages/provider-pi/src/server/status.test.ts b/packages/provider-pi/src/server/status.test.ts index 5c349499b34c2..921836c1e021e 100644 --- a/packages/provider-pi/src/server/status.test.ts +++ b/packages/provider-pi/src/server/status.test.ts @@ -61,6 +61,8 @@ const piDiscoverySpawner = ( readonly modelPresets: unknown; readonly configCalls: Array; readonly stall?: Deferred.Deferred; + /** Completed when models are requested; the reply then waits 25 test-clock seconds. */ + readonly slowModels?: Deferred.Deferred; }, ) => Effect.gen(function* () { @@ -76,6 +78,10 @@ const piDiscoverySpawner = ( stdinBuffer = stdinBuffer.slice(newline + 1); if (line.length === 0) continue; const request = decodeRpcRequest(line); + if (request.type === "get_available_models" && omp?.slowModels !== undefined) { + yield* Deferred.succeed(omp.slowModels, undefined); + yield* Effect.sleep("25 seconds"); + } const data = request.type === "get_available_models" ? inventory @@ -222,17 +228,21 @@ describe("PiProvider", () => { }).pipe(Effect.provide(NodeServices.layer)), ); - it.effect("keeps discovered models when OhMyPi preset discovery stalls", () => + it.effect("keeps discovered models when OhMyPi preset discovery stalls after slow RPC", () => Effect.gen(function* () { const stall = yield* Deferred.make(); + const slowModels = yield* Deferred.make(); const spawner = yield* piDiscoverySpawner( { models: [{ provider: "openai", id: "gpt" }] }, - { configCalls: [], modelPresets: {}, stall }, + { configCalls: [], modelPresets: {}, stall, slowModels }, ); const fiber = yield* checkPiProviderStatus({ ...settings, binaryPath: "omp" }).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), Effect.forkChild, ); + // RPC discovery uses 25 of its 30 seconds before the preset read starts. + yield* Deferred.await(slowModels); + yield* TestClock.adjust("25 seconds"); yield* Deferred.await(stall); yield* TestClock.adjust("10 seconds"); const snapshot = yield* Fiber.join(fiber); diff --git a/packages/provider-pi/src/server/status.ts b/packages/provider-pi/src/server/status.ts index d17b385dc0f02..c7fcf243c911d 100644 --- a/packages/provider-pi/src/server/status.ts +++ b/packages/provider-pi/src/server/status.ts @@ -74,6 +74,7 @@ const PI_DEFAULT_MODEL: ServerProviderModel = { interface PiDiscovery extends PiDiscoveredCommands { readonly models: ReadonlyArray; + readonly modelInventory: unknown; readonly authenticated: boolean; } @@ -162,7 +163,6 @@ const discoverPiViaRpc = Effect.fn("PiProviderStatus.discoverPiViaRpc")(function environment: NodeJS.ProcessEnv, launchArgs: ReadonlyArray, cwd: string | undefined, - isOmp: boolean, ) { const connection = yield* makePiDiscoveryConnection(piSettings, environment, launchArgs, cwd); const stateData = yield* connection.request({ type: "get_state" }); @@ -172,22 +172,9 @@ const discoverPiViaRpc = Effect.fn("PiProviderStatus.discoverPiViaRpc")(function .pipe(Effect.orElseSucceed(() => undefined)); const discoveredModels = yield* Effect.fromResult(parseDiscoveredModels(modelsData, stateData)); const { slashCommands, skills } = parsePiDiscoveredCommands(commandsData); - // Plain Pi has no `config` subcommand, so only ask binaries that identify as omp. - const presets = isOmp - ? yield* readOmpModelPresets({ - binaryPath: piSettings.binaryPath || "pi", - environment, - cwd, - launchArgs, - }).pipe( - Effect.tapError((error) => - Effect.logWarning("OhMyPi model preset discovery failed.", { errorTag: error._tag }), - ), - Effect.orElseSucceed(() => []), - ) - : []; return { - models: [...ompPresetModels(presets, modelsData), ...discoveredModels], + models: discoveredModels, + modelInventory: modelsData, slashCommands: withPiBuiltinSlashCommands(slashCommands), skills, authenticated: discoveredModels.length > 0, @@ -410,7 +397,6 @@ export const checkPiProviderStatus = Effect.fn("checkPiProviderStatus")(function environment, resolvedLaunchArgs.args, cwd, - isOmpVersionOutput(versionOutput.stdout), ).pipe(Effect.timeoutOption(PI_RPC_DISCOVERY_TIMEOUT_MS), Effect.exit); if (Exit.isFailure(discoveryExit)) { yield* Effect.logWarning("Pi RPC discovery failed.", { @@ -449,7 +435,25 @@ export const checkPiProviderStatus = Effect.fn("checkPiProviderStatus")(function } const discovery = discoveryExit.value.value; - const models = piModelsFromSettings(piSettings.customModels, discovery.models); + // Outside the RPC deadline so a slow read only loses the presets. Plain Pi has no `config` + // subcommand, so only binaries that identify as omp are asked. + const presets = isOmpVersionOutput(versionOutput.stdout) + ? yield* readOmpModelPresets({ + binaryPath: piSettings.binaryPath || "pi", + environment, + cwd, + launchArgs: resolvedLaunchArgs.args, + }).pipe( + Effect.tapError((error) => + Effect.logWarning("OhMyPi model preset discovery failed.", { errorTag: error._tag }), + ), + Effect.orElseSucceed(() => []), + ) + : []; + const models = piModelsFromSettings(piSettings.customModels, [ + ...ompPresetModels(presets, discovery.modelInventory), + ...discovery.models, + ]); return buildServerProvider({ presentation: PI_PRESENTATION, enabled: piSettings.enabled, From c07c80198da8de6f5455acb2c7c87bb1c48bb89c Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 27 Oct 2026 00:26:08 +0000 Subject: [PATCH 11/11] test(pi): gate the slow discovery reply on a Deferred --- .../provider-pi/src/server/status.test.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/provider-pi/src/server/status.test.ts b/packages/provider-pi/src/server/status.test.ts index 921836c1e021e..0d4782f6cd38f 100644 --- a/packages/provider-pi/src/server/status.test.ts +++ b/packages/provider-pi/src/server/status.test.ts @@ -61,8 +61,11 @@ const piDiscoverySpawner = ( readonly modelPresets: unknown; readonly configCalls: Array; readonly stall?: Deferred.Deferred; - /** Completed when models are requested; the reply then waits 25 test-clock seconds. */ - readonly slowModels?: Deferred.Deferred; + /** The models reply is held from `requested` until the test completes `release`. */ + readonly slowModels?: { + readonly requested: Deferred.Deferred; + readonly release: Deferred.Deferred; + }; }, ) => Effect.gen(function* () { @@ -79,8 +82,8 @@ const piDiscoverySpawner = ( if (line.length === 0) continue; const request = decodeRpcRequest(line); if (request.type === "get_available_models" && omp?.slowModels !== undefined) { - yield* Deferred.succeed(omp.slowModels, undefined); - yield* Effect.sleep("25 seconds"); + yield* Deferred.succeed(omp.slowModels.requested, undefined); + yield* Deferred.await(omp.slowModels.release); } const data = request.type === "get_available_models" @@ -231,7 +234,10 @@ describe("PiProvider", () => { it.effect("keeps discovered models when OhMyPi preset discovery stalls after slow RPC", () => Effect.gen(function* () { const stall = yield* Deferred.make(); - const slowModels = yield* Deferred.make(); + const slowModels = { + requested: yield* Deferred.make(), + release: yield* Deferred.make(), + }; const spawner = yield* piDiscoverySpawner( { models: [{ provider: "openai", id: "gpt" }] }, { configCalls: [], modelPresets: {}, stall, slowModels }, @@ -241,8 +247,9 @@ describe("PiProvider", () => { Effect.forkChild, ); // RPC discovery uses 25 of its 30 seconds before the preset read starts. - yield* Deferred.await(slowModels); + yield* Deferred.await(slowModels.requested); yield* TestClock.adjust("25 seconds"); + yield* Deferred.succeed(slowModels.release, undefined); yield* Deferred.await(stall); yield* TestClock.adjust("10 seconds"); const snapshot = yield* Fiber.join(fiber);