From ca96849790b628c154c2962ca247316f909e68b9 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 10:46:04 +0000 Subject: [PATCH 01/18] docs: record OhMyPi provider option and skill discovery decisions - ADR 0005: session toggles launch as provider options, not commands - ADR 0006: skills and commands come from an ephemeral ACP probe - CONTEXT.md: glossary for provider option, provider setting, and skill --- CONTEXT.md | 22 ++++++++++ ...ypi-session-toggles-as-provider-options.md | 44 +++++++++++++++++++ ...mypi-skills-and-commands-from-acp-probe.md | 31 +++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 CONTEXT.md create mode 100644 docs/adr/0005-ohmypi-session-toggles-as-provider-options.md create mode 100644 docs/adr/0006-ohmypi-skills-and-commands-from-acp-probe.md diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 000000000000..b87025f12e6a --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,22 @@ +# T3 Code + +A GUI that drives coding agents through provider CLIs. This glossary holds product and +provider-integration terms; orchestration vocabulary (command, decider, event, projector, +adapter, reactor, receipt, checkpoint) stays in `docs/internals/glossary.md`. + +## Language + +**Provider option**: +A per-thread choice that T3 Code persists with the thread and re-applies to the provider +session every time that session starts, such as reasoning effort or OhMyPi's advisor. +_Avoid_: model option, trait, session toggle + +**Provider setting**: +Global configuration of a provider instance, such as its binary path or enabled state. It +applies to every thread that uses the instance. +_Avoid_: provider config, provider option + +**Skill**: +A named instruction bundle the provider discovers on disk and the user starts with a +`$name` mention. How a provider runs it, such as OhMyPi's `/skill:name`, is not part of the term. +_Avoid_: skill command, slash skill diff --git a/docs/adr/0005-ohmypi-session-toggles-as-provider-options.md b/docs/adr/0005-ohmypi-session-toggles-as-provider-options.md new file mode 100644 index 000000000000..a9be587a0a48 --- /dev/null +++ b/docs/adr/0005-ohmypi-session-toggles-as-provider-options.md @@ -0,0 +1,44 @@ +# ADR 0005: OhMyPi session toggles are provider options applied at process launch + +- Status: accepted +- Date: 2026-09-21 +- Compared with: OhMyPi 18.2.7 over ACP + +OhMyPi's computer use, advisor, and prewalk are session behaviors that its CLI +users enable with slash commands before their first message. Over ACP they are +not config options, and enabling them through `/computer on`, `/advisor on`, or +`/prewalk` only changes the running process: the omp session file never records +them, and a `session/load` in a fresh process comes back with all three off. +T3 Code stops idle provider sessions and resumes them by session id, so a state +set once by commands silently decays. + +The fork therefore treats these as provider options: per-thread choices that +T3 persists with the thread and re-applies whenever the session starts. They +appear as boolean option descriptors on every OhMyPi model, so the existing +traits picker, mobile thread settings, new-thread defaults, and project model +defaults carry them with no OhMyPi-specific UI. + +The adapter applies them at launch rather than by sending commands. `omp acp` +forwards `--advisor` and `--prewalk`, and computer use is set through +`--config` with a small YAML overlay that T3 writes under its own home. The +overlay merges on top of omp's global config instead of replacing it. A process +launched this way reports the toggles on for new and resumed sessions alike, +and the same session resumed without the flags reports them off, so the desired +state is exactly what the process was told. Every launch passes all three +explicitly, on or off, so the control never depends on omp's global config and a +user preference lives in T3's new-thread and project defaults. Changing a toggle +mid-thread reuses the reactor's restart-with-resume path that already handles +permission mode and Claude model selection changes. The reactor learns which +options need a restart from an adapter capability that lists their ids, so a +thinking-level change keeps its in-session path and does not reset the +advisor's accumulated context. + +Sending the commands as hidden prompts was rejected: prewalk has no off command, +each command is a separate round trip that can fail halfway, and the adapter +would have to swallow the agent output chunks those commands emit. Vibe mode is +not exposed over ACP at all in 18.2.7 and is out of scope. + +See [OhMyPiAcpSupport.ts](../../apps/server/src/provider/acp/OhMyPiAcpSupport.ts) +for the launch arguments and +[ProviderCommandReactor.ts](../../apps/server/src/orchestration/Layers/ProviderCommandReactor.ts) +for the restart decision. diff --git a/docs/adr/0006-ohmypi-skills-and-commands-from-acp-probe.md b/docs/adr/0006-ohmypi-skills-and-commands-from-acp-probe.md new file mode 100644 index 000000000000..eeb8ea153c5d --- /dev/null +++ b/docs/adr/0006-ohmypi-skills-and-commands-from-acp-probe.md @@ -0,0 +1,31 @@ +# ADR 0006: OhMyPi skills and commands come from an ephemeral ACP probe + +- Status: accepted +- Date: 2026-09-21 +- Compared with: OhMyPi 18.2.7 over ACP + +Claude and Cursor populate the `$` skill picker by scanning skill directories +on disk. OhMyPi does not get the same treatment. Its CLI has no command that +lists skills, and its discovery walks a dozen directory families, each gated +by its own settings, plus plugin and managed-skill packages. A scan that +mirrors that would drift, and a false positive is worse than a miss: an +unknown `/skill:name` is not rejected, it reaches the model as literal text. + +Instead the driver refreshes a workspace by starting a throwaway ACP session: +spawn `omp acp --session-dir `, create a session for the +cwd, read the `available_commands_update` that omp sends about fifty +milliseconds later, then close and kill. That notification is omp's own view +of its skills, as `skill:` entries with descriptions, and of every other +command, so the `/` menu is complete before the first turn too. The skill +entries are surfaced as skills and dropped from the slash command list. A +skill's only identifier is omp's `skill://`, which the clients use just +to pick a source badge. + +The session directory override matters: `omp acp` ignores `--no-session`, and +a probe without the override leaves an empty session in the user's omp resume +list on every refresh. A live session's own `available_commands_update` keeps +replacing the probed snapshot, so a running thread never sees stale data. + +See [OhMyPiDriver.ts](../../apps/server/src/provider/Drivers/OhMyPiDriver.ts) +and, for the pattern this mirrors, the Claude capabilities probe in +[ClaudeProvider.ts](../../apps/server/src/provider/Layers/ClaudeProvider.ts). From 9420eddc1ebda1257b2e1fbf02311fb4be6e8eb6 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 11:19:37 +0000 Subject: [PATCH 02/18] feat(ohmypi): skills and commands before first turn, launch toggles - Probe workspace commands without a session so skills/slash commands appear before the first turn - Split omp's skill: commands into skills; rewrite $mentions to /skill: prompts - Apply advisor/computer-use/prewalk toggles via launch config overlays - Restart sessions only when launch-time options change, per-provider restart option ids --- apps/server/scripts/acp-mock-agent.ts | 16 ++ .../Layers/ProviderCommandReactor.test.ts | 61 ++++++++ .../Layers/ProviderCommandReactor.ts | 38 +++-- .../src/provider/Drivers/OhMyPiDriver.test.ts | 97 ++++++++++++- .../src/provider/Drivers/OhMyPiDriver.ts | 111 ++++++++++---- .../src/provider/Drivers/OhMyPiModels.ts | 11 +- .../Drivers/OhMyPiSkillDispatch.test.ts | 82 +++++++++++ .../provider/Drivers/OhMyPiSkillDispatch.ts | 137 ++++++++++++++++++ .../src/provider/Layers/ClaudeAdapter.ts | 3 + .../src/provider/Layers/OhMyPiAdapter.ts | 85 +++++++++-- .../src/provider/OhMyPiSessionOptions.test.ts | 58 ++++++++ .../src/provider/OhMyPiSessionOptions.ts | 86 +++++++++++ .../src/provider/Services/ProviderAdapter.ts | 6 + .../src/provider/acp/OhMyPiAcpSupport.ts | 61 ++++++++ .../src/components/chat/TraitsPicker.test.ts | 32 +++- apps/web/src/components/chat/TraitsPicker.tsx | 3 + ...ypi-session-toggles-as-provider-options.md | 19 ++- docs/user/install.md | 10 +- 18 files changed, 850 insertions(+), 66 deletions(-) create mode 100644 apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts create mode 100644 apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts create mode 100644 apps/server/src/provider/OhMyPiSessionOptions.test.ts create mode 100644 apps/server/src/provider/OhMyPiSessionOptions.ts diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index 0e8d3997d94f..ac492973ee10 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -15,6 +15,8 @@ import type * as AcpSchema from "effect-acp/schema"; const requestLogPath = process.env.T3_ACP_REQUEST_LOG_PATH; const exitLogPath = process.env.T3_ACP_EXIT_LOG_PATH; const antigravityProfile = process.env.T3_ACP_ANTIGRAVITY === "1"; +/** JSON `AvailableCommand[]` published after session setup, like omp's bootstrap update. */ +const availableCommandsJson = process.env.T3_ACP_AVAILABLE_COMMANDS; const emitToolCalls = process.env.T3_ACP_EMIT_TOOL_CALLS === "1"; const emitInterleavedAssistantToolCalls = process.env.T3_ACP_EMIT_INTERLEAVED_ASSISTANT_TOOL_CALLS === "1"; @@ -436,11 +438,23 @@ const program = Effect.gen(function* () { yield* agent.handleLogout(() => Effect.succeed({})); } + const publishConfiguredCommands = (targetSessionId: string) => + availableCommandsJson === undefined + ? Effect.void + : agent.client.sessionUpdate({ + sessionId: targetSessionId, + update: { + sessionUpdate: "available_commands_update", + availableCommands: JSON.parse(availableCommandsJson), + }, + }); + yield* agent.handleCreateSession(() => Effect.gen(function* () { if (antigravityProfile) { yield* publishAntigravityCommands(sessionId); } + yield* publishConfiguredCommands(sessionId); return { sessionId, modes: modeState(), @@ -465,6 +479,7 @@ const program = Effect.gen(function* () { if (antigravityProfile) { yield* publishAntigravityCommands(request.sessionId); } + yield* publishConfiguredCommands(request.sessionId); return { modes: modeState(), models: modelState(), @@ -528,6 +543,7 @@ const program = Effect.gen(function* () { content: { type: "text", text: "replay" }, }, }); + yield* publishConfiguredCommands(requestedSessionId); return { modes: modeState(), models: modelState(), diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts index 9bc701af0837..7d871ee2a244 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts @@ -172,6 +172,7 @@ describe("ProviderCommandReactor", () => { readonly deferReactorStart?: boolean; readonly threadModelSelection?: ModelSelection; readonly sessionModelSwitch?: "unsupported" | "in-session"; + readonly sessionRestartOptionIds?: ReadonlyArray; readonly requiresNewThreadForModelChange?: boolean; readonly unreadableHistory?: boolean; readonly titleRegenerationCompletionDispatchFailures?: number; @@ -367,6 +368,9 @@ describe("ProviderCommandReactor", () => { getCapabilities: (_provider) => Effect.succeed({ sessionModelSwitch: input?.sessionModelSwitch ?? "in-session", + ...(input?.sessionRestartOptionIds + ? { sessionRestartOptionIds: input.sessionRestartOptionIds } + : {}), }), assertConversationRollbackSupported: () => unsupported(), getInstanceInfo: (instanceId) => { @@ -3187,6 +3191,7 @@ describe("ProviderCommandReactor", () => { instanceId: ProviderInstanceId.make("claudeAgent"), model: "claude-sonnet-4-6", }, + sessionRestartOptionIds: ["effort", "fastMode", "contextWindow", "thinking"], }); const now = "2026-01-01T00:00:00.000Z"; @@ -3249,6 +3254,62 @@ describe("ProviderCommandReactor", () => { }); }); + it("restarts only when a launch-time option changes, not an in-session one", async () => { + const instanceId = ProviderInstanceId.make("omp"); + const harness = await createHarness({ + threadModelSelection: { instanceId, model: "oh-my-pi-default" }, + sessionRestartOptionIds: ["advisor", "computerUse", "prewalk"], + }); + const now = "2026-01-01T00:00:00.000Z"; + const startTurn = ( + suffix: string, + options: ReadonlyArray<{ id: string; value: string | boolean }>, + ) => + Effect.runPromise( + harness.engine.dispatch({ + type: "thread.turn.start", + commandId: CommandId.make(`cmd-turn-start-launch-option-${suffix}`), + threadId: ThreadId.make("thread-1"), + message: { + messageId: asMessageId(`user-message-launch-option-${suffix}`), + role: "user", + text: `turn ${suffix}`, + attachments: [], + }, + modelSelection: createModelSelection(instanceId, "oh-my-pi-default", options), + interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, + runtimeMode: "approval-required", + createdAt: now, + }), + ); + + await startTurn("1", [{ id: "thinking", value: "low" }]); + await waitFor(() => harness.sendTurn.mock.calls.length === 1); + expect(harness.startSession.mock.calls.length).toBe(1); + + // Thinking applies in-session, and an explicit off equals the unset default. + await startTurn("2", [ + { id: "thinking", value: "high" }, + { id: "advisor", value: false }, + ]); + await waitFor(() => harness.sendTurn.mock.calls.length === 2); + expect(harness.startSession.mock.calls.length).toBe(1); + + await startTurn("3", [ + { id: "thinking", value: "high" }, + { id: "advisor", value: true }, + ]); + await waitFor(() => harness.startSession.mock.calls.length === 2); + await waitFor(() => harness.sendTurn.mock.calls.length === 3); + expect(harness.startSession.mock.calls[1]?.[1]).toMatchObject({ + resumeCursor: { opaque: "resume-1" }, + modelSelection: createModelSelection(instanceId, "oh-my-pi-default", [ + { id: "thinking", value: "high" }, + { id: "advisor", value: true }, + ]), + }); + }); + it("restarts the provider session when runtime mode is updated on the thread", async () => { const harness = await createHarness(); const now = "2026-01-01T00:00:00.000Z"; diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index 6a42b7c67ae1..b333cb822ef9 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -23,7 +23,6 @@ import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; -import * as Equal from "effect/Equal"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; @@ -209,6 +208,25 @@ function buildGeneratedWorktreeBranchName(raw: string): string { return `${WORKTREE_BRANCH_PREFIX}/${safeFragment}`; } +/** + * Whether an option the provider applies only at launch (its adapter's + * `sessionRestartOptionIds`) differs between the selection the session was last + * given and the requested one. An unknown previous selection reads as every + * listed option unset, and `false` equals unset, so a cold memo never restarts + * a session over toggles that are off. + */ +function haveSessionRestartOptionsChanged( + optionIds: ReadonlyArray | undefined, + previous: ModelSelection | undefined, + requested: ModelSelection, +): boolean { + const valueOf = (selection: ModelSelection | undefined, id: string) => { + const value = selection?.options?.find((option) => option.id === id)?.value; + return value === false ? undefined : value; + }; + return (optionIds ?? []).some((id) => valueOf(previous, id) !== valueOf(requested, id)); +} + const make = Effect.gen(function* () { const crypto = yield* Crypto.Crypto; const orchestrationEngine = yield* OrchestrationEngineService; @@ -754,8 +772,8 @@ const make = Effect.gen(function* () { if (existingSessionThreadId) { const runtimeModeChanged = thread.runtimeMode !== thread.session?.runtimeMode; const cwdChanged = effectiveCwd !== activeSession?.cwd; - const sessionModelSwitch = (yield* providerService.getCapabilities(desiredInstanceId)) - .sessionModelSwitch; + const capabilities = yield* providerService.getCapabilities(desiredInstanceId); + const sessionModelSwitch = capabilities.sessionModelSwitch; const modelChanged = requestedModelSelection !== undefined && requestedModelSelection.model !== activeSession?.model; @@ -763,18 +781,20 @@ const make = Effect.gen(function* () { requestedModelSelection !== undefined && activeSession?.providerInstanceId !== requestedModelSelection.instanceId; const shouldRestartForModelChange = modelChanged && sessionModelSwitch === "unsupported"; - const previousModelSelection = threadModelSelections.get(threadId); - const shouldRestartForModelSelectionChange = - preferredProvider === "claudeAgent" && + const shouldRestartForLaunchOptionChange = requestedModelSelection !== undefined && - !Equal.equals(previousModelSelection, requestedModelSelection); + haveSessionRestartOptionsChanged( + capabilities.sessionRestartOptionIds, + threadModelSelections.get(threadId), + requestedModelSelection, + ); if ( !runtimeModeChanged && !cwdChanged && !instanceChanged && !shouldRestartForModelChange && - !shouldRestartForModelSelectionChange + !shouldRestartForLaunchOptionChange ) { yield* refreshWorkspaceSnapshot; return existingSessionThreadId; @@ -799,7 +819,7 @@ const make = Effect.gen(function* () { modelChanged, instanceChanged, shouldRestartForModelChange, - shouldRestartForModelSelectionChange, + shouldRestartForLaunchOptionChange, hasResumeCursor: resumeCursor !== undefined, }); const restartedSession = yield* startProviderSession( diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts index 31465d09678d..feb17ff6f2bc 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts @@ -113,6 +113,14 @@ it.layer(testLayer)("OhMyPi driver", (it) => { id: "thinking", options: [{ id: "off" }, { id: "auto" }, { id: "low" }, { id: "high" }], }, + { id: "advisor", type: "boolean" }, + { id: "computerUse", type: "boolean" }, + { id: "prewalk", type: "boolean" }, + ]); + expect(first.models[0]?.capabilities?.optionDescriptors?.map((d) => d.id)).toEqual([ + "advisor", + "computerUse", + "prewalk", ]); for (const output of ["not json", "exit", '{"models":[{"name":"missing ID"}]}']) { yield* fs.writeFileString(catalogFile, output); @@ -733,6 +741,12 @@ it.layer(testLayer)("OhMyPi driver", (it) => { T3_ACP_REQUEST_LOG_PATH: logPath, T3_ACP_EMIT_TOOL_CALLS: "1", T3_ACP_ALLOW_ONCE_OPTION_ID: "omp-allow-42", + // @effect-diagnostics-next-line preferSchemaOverJson:off + T3_ACP_AVAILABLE_COMMANDS: JSON.stringify([ + { name: "compact", description: "Compact the conversation" }, + { name: "skill:grill-me", description: "Interview relentlessly" }, + { name: "advisor", description: "Toggle advisor", input: { hint: "[on|off]" } }, + ]), }, source: ` @@ -770,6 +784,29 @@ it.layer(testLayer)("OhMyPi driver", (it) => { expect(refreshed.models.map((model) => model.slug)).toContain("openai/gpt"); expect(refreshed.version).toBe("18.1.14"); expect(yield* fs.exists(logPath)).toBe(false); + + // The workspace probe reads omp's command list without a thread. + const { stateDir } = yield* ServerConfig; + const instanceDir = path.join(stateDir, "ohmypi", instanceId); + const scoped = yield* instance.snapshotForCwd!(directory); + expect(scoped.skills).toEqual([ + { + name: "grill-me", + description: "Interview relentlessly", + path: "skill://grill-me", + enabled: true, + }, + ]); + expect(scoped.slashCommands.map((command) => command.name)).toEqual(["compact", "advisor"]); + expect((yield* instance.snapshot.getSnapshot).workspaceSnapshots?.[0]?.cwd).toBe(directory); + const probeArgv = (yield* fs.readFileString(argvPath)).trim().split("\n"); + expect(probeArgv).toHaveLength(1); + expect(probeArgv[0]).toContain( + `acp\t--session-dir\t${path.join(instanceDir, "probe-sessions")}`, + ); + expect(yield* fs.readDirectory(path.join(instanceDir, "probe-sessions"))).toEqual([]); + expect(yield* fs.readFileString(logPath)).toContain('"method":"session/close"'); + const events = yield* Queue.unbounded(); yield* instance.adapter.streamEvents.pipe( Stream.runForEach((event) => Queue.offer(events, event)), @@ -779,9 +816,20 @@ it.layer(testLayer)("OhMyPi driver", (it) => { threadId, cwd: directory, runtimeMode: "approval-required", - modelSelection: { instanceId, model: OH_MY_PI_DEFAULT_MODEL }, + modelSelection: { + instanceId, + model: OH_MY_PI_DEFAULT_MODEL, + options: [{ id: "advisor", value: true }], + }, }); expect(session.provider).toBe("ohMyPi"); + const overlayPath = path.join(instanceDir, "config", "advisor-on.computer-off.yml"); + expect((yield* fs.readFileString(argvPath)).trim().split("\n")[1]).toBe( + `acp\t--approval-mode\talways-ask\t--no-prewalk\t--config\t${overlayPath}`, + ); + expect(yield* fs.readFileString(overlayPath)).toBe( + "advisor:\n enabled: true\ncomputer:\n enabled: false\n", + ); expect((yield* instance.snapshot.getSnapshot).models).toEqual(refreshed.models); const turn = yield* instance.adapter .sendTurn({ @@ -809,13 +857,46 @@ it.layer(testLayer)("OhMyPi driver", (it) => { expect((yield* instance.snapshot.getSnapshot).models).toEqual(refreshed.models); expect(seen.some((event) => event.type === "content.delta")).toBe(true); expect(seen.some((event) => event.type === "request.resolved")).toBe(true); + + // A skill mention becomes omp's invocation and travels without the runtime block. + const skillTurn = yield* instance.adapter + .sendTurn({ threadId, input: "please $grill-me now", attachments: [] }) + .pipe(Effect.forkChild); + while (true) { + const event = yield* Queue.take(events); + if (event.type === "request.opened") { + yield* instance.adapter.respondToRequest( + threadId, + ApprovalRequestId.make(event.requestId!), + "accept", + ); + } + if (event.type === "turn.completed") break; + } + yield* Fiber.join(skillTurn); + yield* instance.adapter.stopSession(threadId); yield* instance.adapter.startSession({ threadId, cwd: directory, runtimeMode: "approval-required", resumeCursor: session.resumeCursor, + modelSelection: { + instanceId, + model: OH_MY_PI_DEFAULT_MODEL, + options: [ + { id: "computerUse", value: true }, + { id: "prewalk", value: true }, + ], + }, }); + const resumeOverlayPath = path.join(instanceDir, "config", "advisor-off.computer-on.yml"); + expect((yield* fs.readFileString(argvPath)).trim().split("\n")[2]).toBe( + `acp\t--approval-mode\talways-ask\t--prewalk\t--config\t${resumeOverlayPath}`, + ); + expect(yield* fs.readFileString(resumeOverlayPath)).toBe( + "advisor:\n enabled: false\ncomputer:\n enabled: true\n", + ); const interruptedTurn = yield* instance.adapter .sendTurn({ threadId, input: "wait for approval", attachments: [] }) .pipe(Effect.forkChild); @@ -827,6 +908,20 @@ it.layer(testLayer)("OhMyPi driver", (it) => { expect(requests).toContain('"methodId":"agent"'); expect(requests).toContain('"method":"session/load"'); expect(requests).not.toContain('"value":"oh-my-pi-default"'); + const promptTexts = requests + .split("\n") + .filter((line) => line.includes('"method":"session/prompt"')) + .map((line) => + // @effect-diagnostics-next-line preferSchemaOverJson:off + ( + JSON.parse(line) as { + params: { prompt: Array<{ type: string; text?: string }> }; + } + ).params.prompt.flatMap((block) => (block.type === "text" ? [block.text] : [])), + ); + expect(promptTexts[0]?.[0]).toBe("hello"); + expect(promptTexts[0]?.[1]).toContain(""); + expect(promptTexts[1]).toEqual(["please /skill:grill-me now"]); expect(yield* fs.readFileString(argvPath)).toContain("acp\t--approval-mode\talways-ask"); yield* instance.adapter.stopAll(); expect(yield* instance.adapter.listSessions()).toEqual([]); diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.ts index fc810e196a1a..293bfb16e9e6 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.ts @@ -18,7 +18,12 @@ import * as BackgroundPolicy from "../../background/BackgroundPolicy.ts"; import { ServerConfig } from "../../config.ts"; import { ServerSettingsService } from "../../serverSettings.ts"; import { ProviderDriverError } from "../Errors.ts"; +import { probeOhMyPiWorkspaceCommands } from "../acp/OhMyPiAcpSupport.ts"; import { makeOhMyPiAdapter } from "../Layers/OhMyPiAdapter.ts"; +import { + OH_MY_PI_SESSION_OPTION_DESCRIPTORS, + ohMyPiInstanceStateDir, +} from "../OhMyPiSessionOptions.ts"; import { ProviderEventLoggers } from "../Layers/ProviderEventLoggers.ts"; import { makeManagedServerProvider } from "../makeManagedServerProvider.ts"; import { @@ -34,11 +39,16 @@ import { type ServerProviderDraft, } from "../providerSnapshot.ts"; import { probeOhMyPiModels } from "./OhMyPiModels.ts"; +import { splitOhMyPiAvailableCommands } from "./OhMyPiSkillDispatch.ts"; import { withInstanceIdentity } from "./instanceIdentity.ts"; const DRIVER = ProviderDriverKind.make("ohMyPi"); const decodeSettings = Schema.decodeSync(OhMyPiSettings); -const capabilities = createModelCapabilities({ optionDescriptors: [] }); +const capabilities = createModelCapabilities({ + optionDescriptors: OH_MY_PI_SESSION_OPTION_DESCRIPTORS, +}); +/** Live sessions and probes both report per workspace; keep a bounded set. */ +const MAX_WORKSPACE_SNAPSHOTS = 32; export type OhMyPiDriverEnv = | BackgroundPolicy.BackgroundPolicy @@ -60,6 +70,13 @@ export const OhMyPiDriver: ProviderDriver = { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; const serverConfig = yield* ServerConfig; const eventLoggers = yield* ProviderEventLoggers; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const crypto = yield* Crypto.Crypto; + const probeSessionsDir = path.join( + ohMyPiInstanceStateDir(path, serverConfig.stateDir, instanceId), + "probe-sessions", + ); const effectiveConfig = { ...config, enabled }; const processEnv = mergeProviderInstanceEnvironment(environment); const continuationIdentity = defaultProviderContinuationIdentity({ @@ -102,6 +119,59 @@ export const OhMyPiDriver: ProviderDriver = { } satisfies ServerProviderDraft; const metadata = yield* SubscriptionRef.make(initial); const getSnapshot = SubscriptionRef.get(metadata).pipe(Effect.map(stampIdentity)); + const findWorkspace = (cwd: string) => + SubscriptionRef.get(metadata).pipe( + Effect.map((draft) => + draft.workspaceSnapshots?.find((workspace) => workspace.cwd === cwd), + ), + ); + // omp's command list is the one source for a workspace's skills and slash + // commands, whether a live session or the probe reported it; a live session + // always replaces what the probe found. + const recordWorkspaceCommands = ( + cwd: string, + commands: ReadonlyArray<{ + readonly name: string; + readonly description?: string | null; + readonly input?: { readonly hint: string } | null; + }>, + ) => + SubscriptionRef.update(metadata, (draft) => ({ + ...draft, + workspaceSnapshots: [ + ...(draft.workspaceSnapshots ?? []).filter((workspace) => workspace.cwd !== cwd), + { cwd, checkedAt: draft.checkedAt, ...splitOhMyPiAvailableCommands(commands) }, + ].slice(-MAX_WORKSPACE_SNAPSHOTS), + })); + // A throwaway ACP session in a T3-owned session directory; see ADR 0006. + const probeWorkspace = (cwd: string) => + Effect.gen(function* () { + yield* fileSystem.makeDirectory(probeSessionsDir, { recursive: true }); + const sessionDir = yield* fileSystem.makeTempDirectoryScoped({ + directory: probeSessionsDir, + prefix: "session-", + }); + const commands = yield* probeOhMyPiWorkspaceCommands({ + childProcessSpawner: spawner, + ohMyPiSettings: effectiveConfig, + environment: processEnv, + cwd, + sessionDir, + }); + yield* recordWorkspaceCommands(cwd, commands); + }).pipe( + Effect.scoped, + Effect.provideService(Crypto.Crypto, crypto), + Effect.mapError( + (cause) => + new ProviderDriverError({ + driver: DRIVER, + instanceId, + detail: `Could not read OhMyPi commands for '${cwd}'.`, + cause, + }), + ), + ); const checkProvider = Effect.gen(function* () { if (!enabled) return yield* getSnapshot; const result = yield* probeOhMyPiModels(effectiveConfig, processEnv, serverConfig.cwd).pipe( @@ -169,25 +239,8 @@ export const OhMyPiDriver: ProviderDriver = { instanceId, environment: processEnv, ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), - onAvailableCommands: (commands, cwd) => - SubscriptionRef.update(metadata, (draft) => ({ - ...draft, - workspaceSnapshots: [ - ...(draft.workspaceSnapshots ?? []).filter((workspace) => workspace.cwd !== cwd), - { - cwd, - checkedAt: draft.checkedAt, - skills: [], - slashCommands: commands - .filter((command) => command.name.trim()) - .map((command) => ({ - name: command.name, - description: command.description, - ...(command.input ? { input: command.input } : {}), - })), - }, - ].slice(-32), - })), + onAvailableCommands: (commands, cwd) => recordWorkspaceCommands(cwd, commands), + workspaceCatalog: findWorkspace, }); const unsupported = (operation: string) => Effect.fail( @@ -206,14 +259,18 @@ export const OhMyPiDriver: ProviderDriver = { enabled, snapshot: { ...snapshot, getSnapshot }, snapshotForCwd: (cwd) => - getSnapshot.pipe( - Effect.map((snapshot) => ({ + Effect.gen(function* () { + if (enabled && (yield* findWorkspace(cwd)) === undefined) { + yield* probeWorkspace(cwd); + } + const snapshot = yield* getSnapshot; + const workspace = snapshot.workspaceSnapshots?.find((entry) => entry.cwd === cwd); + return { ...snapshot, - slashCommands: - snapshot.workspaceSnapshots?.find((workspace) => workspace.cwd === cwd) - ?.slashCommands ?? [], - })), - ), + slashCommands: workspace?.slashCommands ?? [], + skills: workspace?.skills ?? [], + }; + }), adapter, textGeneration: { generateCommitMessage: () => unsupported("generateCommitMessage"), diff --git a/apps/server/src/provider/Drivers/OhMyPiModels.ts b/apps/server/src/provider/Drivers/OhMyPiModels.ts index f5adedcd25ce..51480d5d3bd8 100644 --- a/apps/server/src/provider/Drivers/OhMyPiModels.ts +++ b/apps/server/src/provider/Drivers/OhMyPiModels.ts @@ -4,6 +4,7 @@ import { resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import { ChildProcess } from "effect/unstable/process"; +import { OH_MY_PI_SESSION_OPTION_DESCRIPTORS } from "../OhMyPiSessionOptions.ts"; import { parseGenericCliVersion, spawnAndCollect } from "../providerSnapshot.ts"; const ModelsOutput = Schema.Struct({ @@ -72,20 +73,22 @@ export const probeOhMyPiModels = Effect.fn("probeOhMyPiModels")(function* ( subProvider: model.provider, isCustom: false, capabilities: createModelCapabilities({ - optionDescriptors: - thinking.length > 0 + optionDescriptors: [ + ...(thinking.length > 0 ? [ { id: "thinking", label: "Thinking", - type: "select", + type: "select" as const, options: [...new Set(["off", "auto", ...thinking])].map((id) => ({ id, label: id === "off" ? "Off" : id === "auto" ? "Auto" : id, })), }, ] - : [], + : []), + ...OH_MY_PI_SESSION_OPTION_DESCRIPTORS, + ], }), }, ]; diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts new file mode 100644 index 000000000000..c81b3f186427 --- /dev/null +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "@effect/vitest"; +import { + ohMyPiWorkspaceCatalog, + prepareOhMyPiPrompt, + splitOhMyPiAvailableCommands, +} from "./OhMyPiSkillDispatch.ts"; + +const catalog = ohMyPiWorkspaceCatalog({ + slashCommands: [{ name: "compact" }, { name: "computer", input: { hint: "[on|off|status]" } }], + skills: [ + { name: "grill-me", path: "skill://grill-me", enabled: true }, + { name: "retired", path: "skill://retired", enabled: false }, + ], +}); + +describe("prepareOhMyPiPrompt", () => { + it("rewrites known skill mentions to omp's invocation and sends them alone", () => { + expect(prepareOhMyPiPrompt("please $grill-me the plan", catalog)).toEqual({ + text: "please /skill:grill-me the plan", + consumedByCommand: true, + }); + expect(prepareOhMyPiPrompt("$grill-me", catalog)).toEqual({ + text: "/skill:grill-me", + consumedByCommand: true, + }); + }); + + it("leaves unknown, disabled, and money-looking mentions as prose", () => { + expect(prepareOhMyPiPrompt("$HOME is set, $retired too, costs $5k", catalog)).toEqual({ + text: "$HOME is set, $retired too, costs $5k", + consumedByCommand: false, + }); + }); + + it("follows omp's prefix rules for inline skill tokens", () => { + expect(prepareOhMyPiPrompt("/tmp/x is broken, $grill-me", catalog).consumedByCommand).toBe( + false, + ); + expect(prepareOhMyPiPrompt("!ls then $grill-me", catalog).consumedByCommand).toBe(false); + expect(prepareOhMyPiPrompt("/skill:unknown args", catalog).consumedByCommand).toBe(false); + }); + + it("recognises advertised commands by their opening name", () => { + expect(prepareOhMyPiPrompt("/computer status", catalog).consumedByCommand).toBe(true); + expect(prepareOhMyPiPrompt(" /compact focus on tests", catalog).consumedByCommand).toBe(true); + expect(prepareOhMyPiPrompt("/compact:soft", catalog).consumedByCommand).toBe(true); + expect(prepareOhMyPiPrompt("/vibe", catalog).consumedByCommand).toBe(false); + expect(prepareOhMyPiPrompt("use /compact later", catalog).consumedByCommand).toBe(false); + }); +}); + +describe("splitOhMyPiAvailableCommands", () => { + it("turns skill entries into skills and keeps the rest as slash commands", () => { + expect( + splitOhMyPiAvailableCommands([ + { name: "compact", description: "Compact the conversation", input: { hint: "[focus]" } }, + { + name: "skill:grill-me", + description: "Interview relentlessly", + input: { hint: "arguments" }, + }, + { name: "skill:grill-me", description: "duplicate" }, + { name: "skill:", description: "nameless" }, + { name: " ", description: "blank" }, + { name: "trace", description: "" }, + ]), + ).toEqual({ + slashCommands: [ + { name: "compact", description: "Compact the conversation", input: { hint: "[focus]" } }, + { name: "trace" }, + ], + skills: [ + { + name: "grill-me", + description: "Interview relentlessly", + path: "skill://grill-me", + enabled: true, + }, + ], + }); + }); +}); diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts new file mode 100644 index 000000000000..35a7cd21147e --- /dev/null +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts @@ -0,0 +1,137 @@ +/** + * OhMyPiSkillDispatch — turns a composer prompt into what omp runs over ACP. + * + * omp invokes a skill as `/skill:`. The token is honored when it opens + * the prompt, or inline when the prompt does not open with another `/`, `!`, + * or `$` prefix; the text on either side becomes the skill's arguments. The + * composer inserts `$name` for every provider, so known mentions are rewritten + * in place, like the Cursor rewrite. + * + * omp joins ACP text blocks with a blank line before parsing. A prompt that a + * builtin command or a skill consumes must therefore travel alone: the runtime + * instructions block would be folded into the command's arguments, and strict + * commands such as `/computer status` then print their usage line instead of + * running. Verified against omp 18.2.7. + * + * @module provider/Drivers/OhMyPiSkillDispatch + */ +import type { + ServerProviderSkill, + ServerProviderSlashCommand, + ServerProviderWorkspaceSnapshot, +} from "@t3tools/contracts"; + +/** + * Same token shape the composer and timeline chips recognise + * (`packages/shared/src/composerInlineTokens.ts`), so a rendered chip and a + * dispatched skill are always the same set. + */ +const SKILL_MENTION_PATTERN = + /(^|\s)\p{Sc}(?![0-9][0-9_]*(?:[kKmMbBtT]|[eE][0-9]+)?(?:\s|$))(?=[a-zA-Z0-9:_-]*[a-zA-Z])([a-zA-Z0-9][a-zA-Z0-9:_-]*)(?=\s|$)/gu; +/** omp's inline skill token: the first match decides, and its name may not contain `/`. */ +const SKILL_TOKEN_PATTERN = /(^|\s)\/skill:([^\s/]+)(?=\s|$)/u; +const SKILL_COMMAND_PREFIX = "skill:"; + +export interface OhMyPiWorkspaceCatalog { + /** Names omp advertised as commands, without the `skill:` entries. */ + readonly commandNames: ReadonlySet; + readonly skillNames: ReadonlySet; +} + +export interface OhMyPiPreparedPrompt { + readonly text: string; + /** omp itself consumes the prompt (a command or a skill), so nothing else may share it. */ + readonly consumedByCommand: boolean; +} + +export function ohMyPiWorkspaceCatalog( + workspace: Pick | undefined, +): OhMyPiWorkspaceCatalog { + return { + commandNames: new Set(workspace?.slashCommands.map((command) => command.name) ?? []), + skillNames: new Set( + workspace?.skills.filter((skill) => skill.enabled).map((skill) => skill.name) ?? [], + ), + }; +} + +export function prepareOhMyPiPrompt( + prompt: string, + catalog: OhMyPiWorkspaceCatalog, +): OhMyPiPreparedPrompt { + const text = prompt.replace(SKILL_MENTION_PATTERN, (match, prefix: string, name: string) => + catalog.skillNames.has(name) ? `${prefix}/${SKILL_COMMAND_PREFIX}${name}` : match, + ); + return { + text, + consumedByCommand: + invokesSkill(text, catalog.skillNames) || opensWithCommand(text, catalog.commandNames), + }; +} + +/** Mirrors omp's `parseSkillInvocation`: an unknown name falls through to the model as text. */ +function invokesSkill(text: string, skillNames: ReadonlySet): boolean { + const trimmed = text.trimStart(); + if (trimmed.startsWith(`/${SKILL_COMMAND_PREFIX}`)) { + const end = trimmed.search(/\s/); + const name = trimmed.slice(1 + SKILL_COMMAND_PREFIX.length, end === -1 ? undefined : end); + return name.length > 0 && skillNames.has(name); + } + if (trimmed.startsWith("/") || trimmed.startsWith("!") || trimmed.startsWith("$")) { + return false; + } + const match = SKILL_TOKEN_PATTERN.exec(text); + return match !== null && skillNames.has(match[2] ?? ""); +} + +/** omp ends a command name at the first whitespace or `:`. */ +function opensWithCommand(text: string, commandNames: ReadonlySet): boolean { + const trimmed = text.trimStart(); + if (!trimmed.startsWith("/")) return false; + const name = trimmed.slice(1).split(/[\s:]/u, 1)[0] ?? ""; + return name.length > 0 && commandNames.has(name); +} + +/** + * omp advertises each skill as a `skill:` command. Those become `$` + * skills, identified only by omp's own `skill://` scheme, and leave the slash + * menu; everything else stays a slash command. + */ +export function splitOhMyPiAvailableCommands( + commands: ReadonlyArray<{ + readonly name: string; + readonly description?: string | null; + readonly input?: { readonly hint: string } | null; + }>, +): { + readonly slashCommands: ReadonlyArray; + readonly skills: ReadonlyArray; +} { + const slashCommands: ServerProviderSlashCommand[] = []; + const skills: ServerProviderSkill[] = []; + const seenSkills = new Set(); + for (const command of commands) { + const name = command.name.trim(); + if (!name) continue; + const description = command.description?.trim(); + if (name.startsWith(SKILL_COMMAND_PREFIX)) { + const skillName = name.slice(SKILL_COMMAND_PREFIX.length); + if (!skillName || seenSkills.has(skillName)) continue; + seenSkills.add(skillName); + skills.push({ + name: skillName, + ...(description ? { description } : {}), + path: `skill://${skillName}`, + enabled: true, + }); + continue; + } + const hint = command.input?.hint.trim(); + slashCommands.push({ + name, + ...(description ? { description } : {}), + ...(hint ? { input: { hint } } : {}), + }); + } + return { slashCommands, skills }; +} diff --git a/apps/server/src/provider/Layers/ClaudeAdapter.ts b/apps/server/src/provider/Layers/ClaudeAdapter.ts index 2bbe0a6b05d3..1588194a9b83 100644 --- a/apps/server/src/provider/Layers/ClaudeAdapter.ts +++ b/apps/server/src/provider/Layers/ClaudeAdapter.ts @@ -5572,6 +5572,9 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( provider: PROVIDER, capabilities: { sessionModelSwitch: "in-session", + // Claude Code applies effort, fast mode, context window, and thinking + // when its query starts, so a change restarts on the session id. + sessionRestartOptionIds: ["effort", "fastMode", "contextWindow", "thinking"], }, compaction: { type: "slash-command", command: "/compact" }, startSession, diff --git a/apps/server/src/provider/Layers/OhMyPiAdapter.ts b/apps/server/src/provider/Layers/OhMyPiAdapter.ts index 1cce77ad3b3d..841f2deab3e7 100644 --- a/apps/server/src/provider/Layers/OhMyPiAdapter.ts +++ b/apps/server/src/provider/Layers/OhMyPiAdapter.ts @@ -18,6 +18,7 @@ import { RuntimeTaskId, RuntimeRequestId, type RuntimeMode, + type ServerProviderWorkspaceSnapshot, type ThreadId, TurnId, } from "@t3tools/contracts"; @@ -41,8 +42,17 @@ import * as EffectAcpErrors from "effect-acp/errors"; import type * as EffectAcpSchema from "effect-acp/schema"; import { resolveAttachmentPath } from "../../attachmentStore.ts"; +import { writeFileStringAtomically } from "../../atomicWrite.ts"; import { ServerConfig } from "../../config.ts"; +import { + OH_MY_PI_SESSION_OPTION_IDS, + ohMyPiConfigOverlay, + ohMyPiInstanceStateDir, + ohMyPiLaunchArgs, + resolveOhMyPiSessionToggles, +} from "../OhMyPiSessionOptions.ts"; import { buildRuntimeInstructions } from "../RuntimeInstructions.ts"; +import { ohMyPiWorkspaceCatalog, prepareOhMyPiPrompt } from "../Drivers/OhMyPiSkillDispatch.ts"; import * as McpProviderSession from "../../mcp/McpProviderSession.ts"; import { ProviderAdapterProcessError, @@ -93,6 +103,10 @@ export interface OhMyPiAdapterLiveOptions { commands: ReadonlyArray, cwd: string, ) => Effect.Effect; + /** The workspace's known commands and skills, from the probe or an earlier session. */ + readonly workspaceCatalog?: ( + cwd: string, + ) => Effect.Effect; } interface PendingApproval { @@ -414,6 +428,10 @@ export function makeOhMyPiAdapter( const path = yield* Path.Path; const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; const serverConfig = yield* Effect.service(ServerConfig); + const launchConfigDir = path.join( + ohMyPiInstanceStateDir(path, serverConfig.stateDir, boundInstanceId), + "config", + ); const crypto = yield* Crypto.Crypto; const nativeEventLogger = options?.nativeEventLogger ?? @@ -778,6 +796,26 @@ export function makeOhMyPiAdapter( }); const mcpSession = McpProviderSession.readMcpProviderSession(input.threadId); + // The toggles are stated at every launch, resume included; see OhMyPiSessionOptions. + const toggles = resolveOhMyPiSessionToggles(ohMyPiModelSelection?.options); + const overlay = ohMyPiConfigOverlay(toggles); + const overlayPath = path.join(launchConfigDir, overlay.fileName); + yield* writeFileStringAtomically({ + filePath: overlayPath, + contents: overlay.contents, + }).pipe( + Effect.provideService(FileSystem.FileSystem, fileSystem), + Effect.provideService(Path.Path, path), + Effect.mapError( + (cause) => + new ProviderAdapterProcessError({ + provider: PROVIDER, + threadId: input.threadId, + detail: `Could not write the OhMyPi launch config at '${overlayPath}'.`, + cause, + }), + ), + ); const acp = yield* makeOhMyPiAcpRuntime({ ohMyPiSettings, environment: { @@ -790,6 +828,7 @@ export function makeOhMyPiAdapter( childProcessSpawner, cwd, runtimeMode: input.runtimeMode, + launchArgs: ohMyPiLaunchArgs({ toggles, overlayPath }), observeToolCallUpdate: (toolCall) => mapExtensionFailure( Effect.suspend(() => { @@ -1190,9 +1229,17 @@ export function makeOhMyPiAdapter( } const promptParts: Array = []; - const rawPrompt = input.input?.trim() ?? ""; - if (rawPrompt) { - promptParts.push({ type: "text", text: rawPrompt }); + const workspaceCwd = ctx.session.cwd; + const prompt = prepareOhMyPiPrompt( + input.input?.trim() ?? "", + ohMyPiWorkspaceCatalog( + workspaceCwd === undefined + ? undefined + : yield* options?.workspaceCatalog?.(workspaceCwd) ?? Effect.succeed(undefined), + ), + ); + if (prompt.text) { + promptParts.push({ type: "text", text: prompt.text }); } if (input.attachments && input.attachments.length > 0) { for (const attachment of input.attachments) { @@ -1248,23 +1295,27 @@ export function makeOhMyPiAdapter( ); } - // ACP has no system-message field; keep runtime context separate from the user's text. + // ACP has no system-message field; keep runtime context separate from the + // user's text. A prompt omp consumes itself travels alone, or the block + // would become the command's arguments; the next ordinary turn carries it. const result = interruptionVersion !== ctx.interruptionVersion ? { stopReason: "cancelled" as const } : yield* ctx.acp .prompt( { - prompt: [ - ...promptParts, - { - type: "text", - text: buildRuntimeInstructions({ - harness: "OhMyPi", - model: resolvedModel, - }), - }, - ], + prompt: prompt.consumedByCommand + ? promptParts + : [ + ...promptParts, + { + type: "text", + text: buildRuntimeInstructions({ + harness: "OhMyPi", + model: resolvedModel, + }), + }, + ], }, { dispatched }, ) @@ -1417,7 +1468,11 @@ export function makeOhMyPiAdapter( return { provider: PROVIDER, - capabilities: { sessionModelSwitch: "in-session", supportsConversationRollback: false }, + capabilities: { + sessionModelSwitch: "in-session", + supportsConversationRollback: false, + sessionRestartOptionIds: OH_MY_PI_SESSION_OPTION_IDS, + }, compaction: { type: "slash-command", command: "/compact" }, startSession, sendTurn, diff --git a/apps/server/src/provider/OhMyPiSessionOptions.test.ts b/apps/server/src/provider/OhMyPiSessionOptions.test.ts new file mode 100644 index 000000000000..c0cc987382c9 --- /dev/null +++ b/apps/server/src/provider/OhMyPiSessionOptions.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "@effect/vitest"; +import { + OH_MY_PI_SESSION_OPTION_DESCRIPTORS, + OH_MY_PI_SESSION_OPTION_IDS, + ohMyPiConfigOverlay, + ohMyPiLaunchArgs, + resolveOhMyPiSessionToggles, +} from "./OhMyPiSessionOptions.ts"; + +describe("OhMyPi session options", () => { + it("treats absent and false selections as off", () => { + expect(resolveOhMyPiSessionToggles(undefined)).toEqual({ + advisor: false, + computerUse: false, + prewalk: false, + }); + expect( + resolveOhMyPiSessionToggles([ + { id: "advisor", value: false }, + { id: "computerUse", value: true }, + { id: "thinking", value: "high" }, + ]), + ).toEqual({ advisor: false, computerUse: true, prewalk: false }); + }); + + it("states every toggle explicitly at launch", () => { + const off = resolveOhMyPiSessionToggles([]); + expect(ohMyPiConfigOverlay(off)).toEqual({ + fileName: "advisor-off.computer-off.yml", + contents: "advisor:\n enabled: false\ncomputer:\n enabled: false\n", + }); + expect(ohMyPiLaunchArgs({ toggles: off, overlayPath: "/tmp/off.yml" })).toEqual([ + "--no-prewalk", + "--config", + "/tmp/off.yml", + ]); + + const on = resolveOhMyPiSessionToggles([ + { id: "advisor", value: true }, + { id: "computerUse", value: true }, + { id: "prewalk", value: true }, + ]); + expect(ohMyPiConfigOverlay(on)).toEqual({ + fileName: "advisor-on.computer-on.yml", + contents: "advisor:\n enabled: true\ncomputer:\n enabled: true\n", + }); + expect(ohMyPiLaunchArgs({ toggles: on, overlayPath: "/tmp/on.yml" })).toEqual([ + "--prewalk", + "--config", + "/tmp/on.yml", + ]); + }); + + it("lists exactly the launch-time descriptors as restart options", () => { + expect(OH_MY_PI_SESSION_OPTION_IDS).toEqual(["advisor", "computerUse", "prewalk"]); + expect(OH_MY_PI_SESSION_OPTION_DESCRIPTORS.every((d) => d.type === "boolean")).toBe(true); + }); +}); diff --git a/apps/server/src/provider/OhMyPiSessionOptions.ts b/apps/server/src/provider/OhMyPiSessionOptions.ts new file mode 100644 index 000000000000..bb5092c05f40 --- /dev/null +++ b/apps/server/src/provider/OhMyPiSessionOptions.ts @@ -0,0 +1,86 @@ +/** + * OhMyPiSessionOptions — the omp session behaviors T3 Code owns as provider + * options: the advisor, computer use, and prewalk. + * + * omp's CLI users switch these on with slash commands before their first + * message. Over ACP those commands only change the running process: the + * session file never records them, and `session/load` in a fresh process comes + * back with all three off. T3 stops idle sessions and resumes them by id, so + * the desired state is stated again at every launch instead, and every launch + * states all three explicitly so omp's global config never decides. + * + * Prewalk travels as `--prewalk` / `--no-prewalk`: omp ignores its config key + * while restoring a session, but honors the flags. Advisor and computer use + * have no off flag, so they ride a `--config` overlay that omp deep-merges over + * its global and project config. Verified against omp 18.2.7; see ADR 0005. + * + * @module provider/OhMyPiSessionOptions + */ +import type { ProviderOptionDescriptor, ProviderOptionSelection } from "@t3tools/contracts"; +import { getProviderOptionBooleanSelectionValue } from "@t3tools/shared/model"; + +export const OH_MY_PI_SESSION_OPTION_DESCRIPTORS: ReadonlyArray = [ + { id: "advisor", label: "Advisor", type: "boolean" }, + { id: "computerUse", label: "Computer use", type: "boolean" }, + { id: "prewalk", label: "Prewalk", type: "boolean" }, +]; + +/** Option ids that only apply when the omp process starts, for the adapter's restart capability. */ +export const OH_MY_PI_SESSION_OPTION_IDS: ReadonlyArray = + OH_MY_PI_SESSION_OPTION_DESCRIPTORS.map((descriptor) => descriptor.id); + +export interface OhMyPiSessionToggles { + readonly advisor: boolean; + readonly computerUse: boolean; + readonly prewalk: boolean; +} + +/** An absent selection is off: composers omit descriptor defaults from dispatch. */ +export function resolveOhMyPiSessionToggles( + selections: ReadonlyArray | null | undefined, +): OhMyPiSessionToggles { + return { + advisor: getProviderOptionBooleanSelectionValue(selections, "advisor") === true, + computerUse: getProviderOptionBooleanSelectionValue(selections, "computerUse") === true, + prewalk: getProviderOptionBooleanSelectionValue(selections, "prewalk") === true, + }; +} + +/** + * The overlay omp reads through `--config`. Keys are nested, not dotted: omp + * splits its own dotted setting paths when it reads the merged tree. The file + * name encodes the content, so concurrent sessions with different toggles + * never share one file. + */ +export function ohMyPiConfigOverlay(toggles: OhMyPiSessionToggles): { + readonly fileName: string; + readonly contents: string; +} { + const state = (enabled: boolean) => (enabled ? "on" : "off"); + return { + fileName: `advisor-${state(toggles.advisor)}.computer-${state(toggles.computerUse)}.yml`, + contents: [ + "advisor:", + ` enabled: ${toggles.advisor}`, + "computer:", + ` enabled: ${toggles.computerUse}`, + "", + ].join("\n"), + }; +} + +export function ohMyPiLaunchArgs(input: { + readonly toggles: OhMyPiSessionToggles; + readonly overlayPath: string; +}): ReadonlyArray { + return [input.toggles.prewalk ? "--prewalk" : "--no-prewalk", "--config", input.overlayPath]; +} + +/** Per-instance runtime state under T3's userdata: launch overlays and probe session files. */ +export function ohMyPiInstanceStateDir( + path: { readonly join: (...segments: ReadonlyArray) => string }, + stateDir: string, + instanceId: string, +): string { + return path.join(stateDir, "ohmypi", instanceId); +} diff --git a/apps/server/src/provider/Services/ProviderAdapter.ts b/apps/server/src/provider/Services/ProviderAdapter.ts index c9b62fd79525..dec0c7c187fb 100644 --- a/apps/server/src/provider/Services/ProviderAdapter.ts +++ b/apps/server/src/provider/Services/ProviderAdapter.ts @@ -52,6 +52,12 @@ export interface ProviderAdapterCapabilities { readonly promptlessTurnContinuation?: boolean; /** False when native conversation history cannot be rewound. */ readonly supportsConversationRollback?: boolean; + /** + * Option ids the provider applies only when its process starts. A turn that + * changes one of them restarts the session on its resume cursor; every other + * option is applied in-session by `sendTurn`. + */ + readonly sessionRestartOptionIds?: ReadonlyArray; } export interface ProviderThreadTurnSnapshot { diff --git a/apps/server/src/provider/acp/OhMyPiAcpSupport.ts b/apps/server/src/provider/acp/OhMyPiAcpSupport.ts index f47f2421a436..e8e3a59b8f57 100644 --- a/apps/server/src/provider/acp/OhMyPiAcpSupport.ts +++ b/apps/server/src/provider/acp/OhMyPiAcpSupport.ts @@ -5,13 +5,20 @@ import { type ProviderOptionSelection, type RuntimeMode, } from "@t3tools/contracts"; +import * as Deferred from "effect/Deferred"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Stream from "effect/Stream"; import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; import type * as AcpErrors from "effect-acp/errors"; import type * as AcpSchema from "effect-acp/schema"; import * as AcpSessionRuntime from "./AcpSessionRuntime.ts"; +const OH_MY_PI_CLIENT_INFO = { name: "t3-code", version: "0.0.0" } as const; +/** omp sends `available_commands_update` about fifty milliseconds after `session/new`. */ +const OH_MY_PI_WORKSPACE_PROBE_TIMEOUT = Duration.seconds(20); + interface OhMyPiAcpRuntimeInput extends Omit< AcpSessionRuntime.AcpSessionRuntimeOptions, "authMethodId" | "clientCapabilities" | "spawn" @@ -20,6 +27,8 @@ interface OhMyPiAcpRuntimeInput extends Omit< readonly ohMyPiSettings: Pick; readonly environment?: NodeJS.ProcessEnv; readonly runtimeMode?: RuntimeMode; + /** Launch flags appended after the approval mode; see `ohMyPiLaunchArgs`. */ + readonly launchArgs?: ReadonlyArray; } export const makeOhMyPiAcpRuntime = Effect.fn("makeOhMyPiAcpRuntime")(function* ( @@ -33,6 +42,7 @@ export const makeOhMyPiAcpRuntime = Effect.fn("makeOhMyPiAcpRuntime")(function* args: [ "acp", ...(input.runtimeMode ? ["--approval-mode", ohMyPiApprovalMode(input.runtimeMode)] : []), + ...(input.launchArgs ?? []), ], cwd: input.cwd, ...(input.environment ? { env: input.environment } : {}), @@ -49,6 +59,57 @@ export const makeOhMyPiAcpRuntime = Effect.fn("makeOhMyPiAcpRuntime")(function* return yield* Effect.service(AcpSessionRuntime.AcpSessionRuntime).pipe(Effect.provide(context)); }); +/** + * Start a throwaway ACP session to read omp's own command list for a cwd: its + * skills, advertised as `skill:`, and every other command. omp has no + * non-interactive listing, and its discovery walks a dozen gated directory + * families, so mirroring it on disk would drift. See ADR 0006. + * + * `--session-dir` keeps the probe out of the user's resume list, since + * `omp acp` ignores `--no-session`. The caller owns that directory. + */ +export const probeOhMyPiWorkspaceCommands = Effect.fn("probeOhMyPiWorkspaceCommands")( + function* (input: { + readonly childProcessSpawner: ChildProcessSpawner.ChildProcessSpawner["Service"]; + readonly ohMyPiSettings: Pick; + readonly environment?: NodeJS.ProcessEnv; + readonly cwd: string; + readonly sessionDir: string; + }) { + const acp = yield* makeOhMyPiAcpRuntime({ + childProcessSpawner: input.childProcessSpawner, + ohMyPiSettings: input.ohMyPiSettings, + ...(input.environment ? { environment: input.environment } : {}), + cwd: input.cwd, + launchArgs: ["--session-dir", input.sessionDir], + clientInfo: OH_MY_PI_CLIENT_INFO, + }); + const commands = yield* Deferred.make< + ReadonlyArray, + AcpErrors.AcpError + >(); + yield* Stream.runForEach(acp.getEvents(), (event) => { + switch (event._tag) { + case "EventStreamBarrier": + return Deferred.succeed(event.acknowledge, undefined); + case "AvailableCommandsUpdated": + return Deferred.succeed(commands, event.availableCommands); + case "ConnectionTerminated": + return Deferred.fail(commands, event.error); + default: + return Effect.void; + } + }).pipe(Effect.forkScoped); + const started = yield* acp.start(); + const available = yield* Deferred.await(commands).pipe( + Effect.timeout(OH_MY_PI_WORKSPACE_PROBE_TIMEOUT), + ); + yield* acp.request("session/close", { sessionId: started.sessionId }).pipe(Effect.ignore); + return available; + }, + Effect.scoped, +); + /** Permission IDs are opaque; select by the ACP kind supplied by the agent. */ export function selectOhMyPiPermissionOption( request: AcpSchema.RequestPermissionRequest, diff --git a/apps/web/src/components/chat/TraitsPicker.test.ts b/apps/web/src/components/chat/TraitsPicker.test.ts index dbe97cdd54e0..b696a8e34b67 100644 --- a/apps/web/src/components/chat/TraitsPicker.test.ts +++ b/apps/web/src/components/chat/TraitsPicker.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; import { ProviderDriverKind, type ProviderOptionDescriptor } from "@t3tools/contracts"; -import { buildTraitsTriggerDisplay, buildUnavailableModelOptionDescriptors } from "./TraitsPicker"; +import { + buildTraitsTriggerDisplay, + buildUnavailableModelOptionDescriptors, + shouldRenderTraitsControls, +} from "./TraitsPicker"; function selectDescriptor( id: string, @@ -156,6 +160,32 @@ describe("buildTraitsTriggerDisplay", () => { }); }); +describe("shouldRenderTraitsControls", () => { + it("renders the control for a model whose only options are toggles", () => { + const models = [ + { + slug: "oh-my-pi-default", + name: "OhMyPi default", + isCustom: false, + isDefault: true, + capabilities: { + optionDescriptors: [{ id: "advisor", label: "Advisor", type: "boolean" as const }], + }, + }, + { slug: "bare", name: "Bare", isCustom: false, capabilities: { optionDescriptors: [] } }, + ]; + const input = { + provider: ProviderDriverKind.make("ohMyPi"), + models, + prompt: "", + modelOptions: undefined, + planModeEnabled: true, + }; + expect(shouldRenderTraitsControls({ ...input, model: "oh-my-pi-default" })).toBe(true); + expect(shouldRenderTraitsControls({ ...input, model: "bare" })).toBe(false); + }); +}); + describe("buildUnavailableModelOptionDescriptors", () => { it("shows only saved values without inventing alternatives", () => { expect( diff --git a/apps/web/src/components/chat/TraitsPicker.tsx b/apps/web/src/components/chat/TraitsPicker.tsx index da293c38ddc9..a1ad8cb6cc0e 100644 --- a/apps/web/src/components/chat/TraitsPicker.tsx +++ b/apps/web/src/components/chat/TraitsPicker.tsx @@ -247,12 +247,15 @@ function getTraitsSectionVisibility(input: { showFastMode, showContextWindow, showAgent, + // Every boolean renders as an On/Off group, so a provider whose only + // options are toggles (OhMyPi's default model) still gets the control. hasAnyControls: showEffort || showThinking || showFastMode || showContextWindow || showAgent || + selected.booleanDescriptors.length > 0 || (selected.modelIsUnavailable && selected.descriptors.length > 0), }; } diff --git a/docs/adr/0005-ohmypi-session-toggles-as-provider-options.md b/docs/adr/0005-ohmypi-session-toggles-as-provider-options.md index a9be587a0a48..bd6958e04ab3 100644 --- a/docs/adr/0005-ohmypi-session-toggles-as-provider-options.md +++ b/docs/adr/0005-ohmypi-session-toggles-as-provider-options.md @@ -19,14 +19,17 @@ traits picker, mobile thread settings, new-thread defaults, and project model defaults carry them with no OhMyPi-specific UI. The adapter applies them at launch rather than by sending commands. `omp acp` -forwards `--advisor` and `--prewalk`, and computer use is set through -`--config` with a small YAML overlay that T3 writes under its own home. The -overlay merges on top of omp's global config instead of replacing it. A process -launched this way reports the toggles on for new and resumed sessions alike, -and the same session resumed without the flags reports them off, so the desired -state is exactly what the process was told. Every launch passes all three -explicitly, on or off, so the control never depends on omp's global config and a -user preference lives in T3's new-thread and project defaults. Changing a toggle +forwards launch flags, so prewalk travels as `--prewalk` or `--no-prewalk`; omp +ignores prewalk's config key while restoring a session but honors the flags. +Advisor and computer use have no off flag, so both ride a small YAML overlay +that T3 writes under its userdata and passes with `--config`. The overlay +deep-merges on top of omp's global and project config instead of replacing it, +and an explicit `false` there overrides a global `true`. A process launched this +way reports the toggles on for new and resumed sessions alike, and the same +session resumed without them reports them off, so the desired state is exactly +what the process was told. Every launch states all three explicitly, on or off, +so the control never depends on omp's global config and a user preference lives +in T3's new-thread and project defaults. Changing a toggle mid-thread reuses the reactor's restart-with-resume path that already handles permission mode and Claude model selection changes. The reactor learns which options need a restart from an adapter capability that lists their ids, so a diff --git a/docs/user/install.md b/docs/user/install.md index de556535608c..27b56a3800c7 100644 --- a/docs/user/install.md +++ b/docs/user/install.md @@ -149,7 +149,15 @@ Refresh provider status after changing your OhMyPi credentials or model configur Choose **OhMyPi default** to use the model configured in OhMyPi, or select a model from the picker. You can switch models, stop turns, and continue a saved session after reconnecting. -OhMyPi's native slash commands appear once a session has started in that workspace. + +**Advisor**, **Computer use**, and **Prewalk** sit with the thinking level in the +composer's model options. T3 Code applies them every time it starts or resumes the +thread's session, so they stay set for the thread; a change takes effect on the next +turn. Your OhMyPi configuration does not switch them on for T3 Code threads, and vibe +mode is not available. + +OhMyPi's slash commands and skills are available in a workspace before its first turn. +Start a skill with a `$` mention, as with other providers. **Auto** uses the same approval policy as **Supervised**. **Auto-accept edits** allows workspace writes, and **Full access** allows all tool tiers. OhMyPi's From f06cffbe0e549c71993639f860e95dd8dee3117b Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 11:41:03 +0000 Subject: [PATCH 03/18] codex: address PR review feedback (#39) - Compare launch-time options strictly so Claude's thinking off and OhMyPi's toggles off both restart the session (Codex P1). - Parenthesize the optional workspace catalog effect so `yield*` never receives undefined (Copilot). - Describe the skill mention pattern accurately against the composer chip pattern (Copilot). - Drop a no-op @effect-diagnostics directive that failed the typecheck. Co-Authored-By: Claude Fable 5.1 --- .../Layers/ProviderCommandReactor.test.ts | 22 ++++++++++++++----- .../Layers/ProviderCommandReactor.ts | 13 +++++------ .../src/provider/Drivers/OhMyPiDriver.test.ts | 1 - .../provider/Drivers/OhMyPiSkillDispatch.ts | 8 ++++--- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts index 7d871ee2a244..b8710271ae87 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts @@ -3287,11 +3287,8 @@ describe("ProviderCommandReactor", () => { await waitFor(() => harness.sendTurn.mock.calls.length === 1); expect(harness.startSession.mock.calls.length).toBe(1); - // Thinking applies in-session, and an explicit off equals the unset default. - await startTurn("2", [ - { id: "thinking", value: "high" }, - { id: "advisor", value: false }, - ]); + // Thinking applies in-session. + await startTurn("2", [{ id: "thinking", value: "high" }]); await waitFor(() => harness.sendTurn.mock.calls.length === 2); expect(harness.startSession.mock.calls.length).toBe(1); @@ -3308,6 +3305,21 @@ describe("ProviderCommandReactor", () => { { id: "advisor", value: true }, ]), }); + + // Switching a launch-time option off is a change too. + await startTurn("4", [ + { id: "thinking", value: "high" }, + { id: "advisor", value: false }, + ]); + await waitFor(() => harness.startSession.mock.calls.length === 3); + await waitFor(() => harness.sendTurn.mock.calls.length === 4); + expect(harness.startSession.mock.calls[2]?.[1]).toMatchObject({ + resumeCursor: { opaque: "resume-1" }, + modelSelection: createModelSelection(instanceId, "oh-my-pi-default", [ + { id: "thinking", value: "high" }, + { id: "advisor", value: false }, + ]), + }); }); it("restarts the provider session when runtime mode is updated on the thread", async () => { diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index b333cb822ef9..78cde114bfe2 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -211,19 +211,18 @@ function buildGeneratedWorktreeBranchName(raw: string): string { /** * Whether an option the provider applies only at launch (its adapter's * `sessionRestartOptionIds`) differs between the selection the session was last - * given and the requested one. An unknown previous selection reads as every - * listed option unset, and `false` equals unset, so a cold memo never restarts - * a session over toggles that are off. + * given and the requested one. Values compare strictly: an explicit `false` + * differs from unset, since Claude's thinking toggle and OhMyPi's session + * toggles both change behavior when switched off. An unknown previous + * selection reads as every listed option unset. */ function haveSessionRestartOptionsChanged( optionIds: ReadonlyArray | undefined, previous: ModelSelection | undefined, requested: ModelSelection, ): boolean { - const valueOf = (selection: ModelSelection | undefined, id: string) => { - const value = selection?.options?.find((option) => option.id === id)?.value; - return value === false ? undefined : value; - }; + const valueOf = (selection: ModelSelection | undefined, id: string) => + selection?.options?.find((option) => option.id === id)?.value; return (optionIds ?? []).some((id) => valueOf(previous, id) !== valueOf(requested, id)); } diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts index feb17ff6f2bc..3121c4e6967c 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts @@ -912,7 +912,6 @@ it.layer(testLayer)("OhMyPi driver", (it) => { .split("\n") .filter((line) => line.includes('"method":"session/prompt"')) .map((line) => - // @effect-diagnostics-next-line preferSchemaOverJson:off ( JSON.parse(line) as { params: { prompt: Array<{ type: string; text?: string }> }; diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts index 35a7cd21147e..3dfc00e9ae87 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts @@ -22,9 +22,11 @@ import type { } from "@t3tools/contracts"; /** - * Same token shape the composer and timeline chips recognise - * (`packages/shared/src/composerInlineTokens.ts`), so a rendered chip and a - * dispatched skill are always the same set. + * Same token shape the Claude and Cursor skill dispatchers use, so a `$name` + * one provider runs, another runs too. It is one character looser than the + * composer chip pattern (`packages/shared/src/composerInlineTokens.ts`), which + * also needs trailing whitespace: a mention that ends the prompt still + * dispatches, matching the CLIs' own parsing. */ const SKILL_MENTION_PATTERN = /(^|\s)\p{Sc}(?![0-9][0-9_]*(?:[kKmMbBtT]|[eE][0-9]+)?(?:\s|$))(?=[a-zA-Z0-9:_-]*[a-zA-Z])([a-zA-Z0-9][a-zA-Z0-9:_-]*)(?=\s|$)/gu; From 966efb3cef01559a5dbeb1390a39b88aedd177a6 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 12:14:36 +0000 Subject: [PATCH 04/18] codex: address PR review feedback (#39) Wait for the workspace catalog before preparing the first OhMyPi prompt. The first turn in a fresh workspace could outrun omp's command update and the probe, sending a $skill mention literally or folding the runtime block into a command's arguments. The driver now subscribes to its snapshot before re-checking, bounded by five seconds, and the mock agent can delay its command update so the race is covered by a test. Co-Authored-By: Claude Fable 5.1 --- apps/server/scripts/acp-mock-agent.ts | 29 +++++---- .../src/provider/Drivers/OhMyPiDriver.test.ts | 59 +++++++++++++++++-- .../src/provider/Drivers/OhMyPiDriver.ts | 26 +++++++- .../src/provider/Layers/OhMyPiAdapter.ts | 6 +- 4 files changed, 104 insertions(+), 16 deletions(-) diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index ac492973ee10..abf917b8419f 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -17,6 +17,8 @@ const exitLogPath = process.env.T3_ACP_EXIT_LOG_PATH; const antigravityProfile = process.env.T3_ACP_ANTIGRAVITY === "1"; /** JSON `AvailableCommand[]` published after session setup, like omp's bootstrap update. */ const availableCommandsJson = process.env.T3_ACP_AVAILABLE_COMMANDS; +/** Delay that publication past the setup response, as omp does. */ +const availableCommandsDelayMs = Number(process.env.T3_ACP_AVAILABLE_COMMANDS_DELAY_MS ?? "0"); const emitToolCalls = process.env.T3_ACP_EMIT_TOOL_CALLS === "1"; const emitInterleavedAssistantToolCalls = process.env.T3_ACP_EMIT_INTERLEAVED_ASSISTANT_TOOL_CALLS === "1"; @@ -438,16 +440,23 @@ const program = Effect.gen(function* () { yield* agent.handleLogout(() => Effect.succeed({})); } - const publishConfiguredCommands = (targetSessionId: string) => - availableCommandsJson === undefined - ? Effect.void - : agent.client.sessionUpdate({ - sessionId: targetSessionId, - update: { - sessionUpdate: "available_commands_update", - availableCommands: JSON.parse(availableCommandsJson), - }, - }); + const publishConfiguredCommands = (targetSessionId: string) => { + if (availableCommandsJson === undefined) return Effect.void; + const publish = agent.client.sessionUpdate({ + sessionId: targetSessionId, + update: { + sessionUpdate: "available_commands_update", + availableCommands: JSON.parse(availableCommandsJson), + }, + }); + return availableCommandsDelayMs > 0 + ? Effect.sleep(`${availableCommandsDelayMs} millis`).pipe( + Effect.andThen(publish), + Effect.forkDetach, + Effect.asVoid, + ) + : publish; + }; yield* agent.handleCreateSession(() => Effect.gen(function* () { diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts index 3121c4e6967c..6ce418685dc2 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts @@ -41,6 +41,8 @@ const testLayer = ServerConfig.layerTest(process.cwd(), { prefix: "t3-omp-driver Layer.provideMerge(Layer.succeed(ProviderEventLoggers, NoOpProviderEventLoggers)), ); const instanceId = ProviderInstanceId.make("omp-test"); +/** omp always follows session setup with a command update; the adapter waits for it. */ +const OMP_BOOTSTRAP_ENV = { T3_ACP_AVAILABLE_COMMANDS: "[]" } as const; const threadId = ThreadId.make("omp-thread"); it.layer(testLayer)("OhMyPi driver", (it) => { @@ -136,6 +138,54 @@ it.layer(testLayer)("OhMyPi driver", (it) => { }).pipe(Effect.scoped), ); + it.effect("waits for the workspace catalog before preparing the first prompt", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped(); + const logPath = path.join(directory, "catalog-race.jsonl"); + const binaryPath = yield* Effect.sync(() => + writeFakeCli({ + directory, + name: "omp-catalog-mock", + env: { + T3_ACP_REQUEST_LOG_PATH: logPath, + // @effect-diagnostics-next-line preferSchemaOverJson:off + T3_ACP_AVAILABLE_COMMANDS: JSON.stringify([ + { name: "skill:grill-me", description: "Interview relentlessly" }, + ]), + T3_ACP_AVAILABLE_COMMANDS_DELAY_MS: "400", + }, + source: execScriptSource({ + scriptPath: NodeURL.fileURLToPath( + new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), + ), + }), + }), + ); + const instance = yield* OhMyPiDriver.create({ + instanceId, + displayName: undefined, + enabled: true, + environment: [], + config: { ...OhMyPiDriver.defaultConfig(), binaryPath }, + }); + yield* instance.adapter.startSession({ + threadId, + cwd: directory, + runtimeMode: "full-access", + }); + yield* instance.adapter.sendTurn({ threadId, input: "$grill-me", attachments: [] }); + const prompts = (yield* fs.readFileString(logPath)) + .split("\n") + .filter((line) => line.includes('"method":"session/prompt"')); + expect(prompts).toHaveLength(1); + expect(prompts[0]).toContain('"text":"/skill:grill-me"'); + expect(prompts[0]).not.toContain("runtime_info"); + yield* instance.adapter.stopAll(); + }).pipe(Effect.scoped), + ); + for (const sendDuringPreparation of [false, true]) { it.effect(`steers within one turn (send during preparation: ${sendDuringPreparation})`, () => Effect.gen(function* () { @@ -148,6 +198,7 @@ it.layer(testLayer)("OhMyPi driver", (it) => { directory, name: "omp-steering-mock", env: { + ...OMP_BOOTSTRAP_ENV, T3_ACP_REQUEST_LOG_PATH: logPath, T3_ACP_COMPLETE_FIRST_PROMPT_ON_CANCEL: "1", }, @@ -243,7 +294,7 @@ it.layer(testLayer)("OhMyPi driver", (it) => { writeFakeCli({ directory, name: "omp-stop-mock", - env: { T3_ACP_REQUEST_LOG_PATH: logPath }, + env: { ...OMP_BOOTSTRAP_ENV, T3_ACP_REQUEST_LOG_PATH: logPath }, source: execScriptSource({ scriptPath: NodeURL.fileURLToPath( new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), @@ -343,7 +394,7 @@ it.layer(testLayer)("OhMyPi driver", (it) => { writeFakeCli({ directory, name: "omp-background-tool-updates-mock", - env: { T3_ACP_EMIT_ASSISTANT_DURING_TOOL_UPDATES: "1" }, + env: { ...OMP_BOOTSTRAP_ENV, T3_ACP_EMIT_ASSISTANT_DURING_TOOL_UPDATES: "1" }, source: execScriptSource({ scriptPath: NodeURL.fileURLToPath( new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), @@ -445,7 +496,7 @@ it.layer(testLayer)("OhMyPi driver", (it) => { writeFakeCli({ directory, name: "omp-task-progress-mock", - env: { T3_ACP_EMIT_OH_MY_PI_TASK_UPDATES: "1" }, + env: { ...OMP_BOOTSTRAP_ENV, T3_ACP_EMIT_OH_MY_PI_TASK_UPDATES: "1" }, source: execScriptSource({ scriptPath: NodeURL.fileURLToPath( new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), @@ -667,7 +718,7 @@ it.layer(testLayer)("OhMyPi driver", (it) => { writeFakeCli({ directory, name: "omp-active-child-stop-mock", - env: { T3_ACP_EMIT_OH_MY_PI_TASK_UPDATES: "1" }, + env: { ...OMP_BOOTSTRAP_ENV, T3_ACP_EMIT_OH_MY_PI_TASK_UPDATES: "1" }, source: execScriptSource({ scriptPath: NodeURL.fileURLToPath( new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.ts index 293bfb16e9e6..33743243c252 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.ts @@ -7,8 +7,10 @@ import { import { createModelCapabilities } from "@t3tools/shared/model"; import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; +import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; @@ -17,6 +19,7 @@ import { ChildProcessSpawner } from "effect/unstable/process"; import * as BackgroundPolicy from "../../background/BackgroundPolicy.ts"; import { ServerConfig } from "../../config.ts"; import { ServerSettingsService } from "../../serverSettings.ts"; +import { subscribeBeforeSnapshotWithoutMutex } from "../../utils/subscribeBeforeSnapshot.ts"; import { ProviderDriverError } from "../Errors.ts"; import { probeOhMyPiWorkspaceCommands } from "../acp/OhMyPiAcpSupport.ts"; import { makeOhMyPiAdapter } from "../Layers/OhMyPiAdapter.ts"; @@ -49,6 +52,11 @@ const capabilities = createModelCapabilities({ }); /** Live sessions and probes both report per workspace; keep a bounded set. */ const MAX_WORKSPACE_SNAPSHOTS = 32; +/** + * omp sends its command list about fifty milliseconds after `session/new`, + * and the probe takes about a second; past this a prompt goes out unprepared. + */ +const WORKSPACE_CATALOG_WAIT = Duration.seconds(5); export type OhMyPiDriverEnv = | BackgroundPolicy.BackgroundPolicy @@ -125,6 +133,22 @@ export const OhMyPiDriver: ProviderDriver = { draft.workspaceSnapshots?.find((workspace) => workspace.cwd === cwd), ), ); + // The first turn in a fresh workspace can outrun both the live session's + // command update and the probe; subscribe before re-checking so an update + // between the two cannot be missed. + const awaitWorkspace = (cwd: string) => + subscribeBeforeSnapshotWithoutMutex(metadata.pubsub, SubscriptionRef.get(metadata)).pipe( + Effect.flatMap(({ latest, changes }) => + Stream.concat(Stream.make(latest), changes).pipe( + Stream.map((draft) => draft.workspaceSnapshots?.find((entry) => entry.cwd === cwd)), + Stream.filter((workspace) => workspace !== undefined), + Stream.runHead, + ), + ), + Effect.scoped, + Effect.timeoutOption(WORKSPACE_CATALOG_WAIT), + Effect.map((found) => Option.getOrUndefined(Option.flatten(found))), + ); // omp's command list is the one source for a workspace's skills and slash // commands, whether a live session or the probe reported it; a live session // always replaces what the probe found. @@ -240,7 +264,7 @@ export const OhMyPiDriver: ProviderDriver = { environment: processEnv, ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), onAvailableCommands: (commands, cwd) => recordWorkspaceCommands(cwd, commands), - workspaceCatalog: findWorkspace, + workspaceCatalog: awaitWorkspace, }); const unsupported = (operation: string) => Effect.fail( diff --git a/apps/server/src/provider/Layers/OhMyPiAdapter.ts b/apps/server/src/provider/Layers/OhMyPiAdapter.ts index 841f2deab3e7..6de537ad21ee 100644 --- a/apps/server/src/provider/Layers/OhMyPiAdapter.ts +++ b/apps/server/src/provider/Layers/OhMyPiAdapter.ts @@ -103,7 +103,11 @@ export interface OhMyPiAdapterLiveOptions { commands: ReadonlyArray, cwd: string, ) => Effect.Effect; - /** The workspace's known commands and skills, from the probe or an earlier session. */ + /** + * The workspace's known commands and skills, from the probe or a session. + * May wait for them: the first turn in a fresh workspace can otherwise + * outrun omp's command update and go out unprepared. + */ readonly workspaceCatalog?: ( cwd: string, ) => Effect.Effect; From a5e79e81a43218a0bffb7072175c43c474790c3e Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 12:28:12 +0000 Subject: [PATCH 05/18] codex: address PR review feedback (#39) Offer "View instructions" only for skills with a filesystem path. OhMyPi names its skills with its own skill:// scheme, which the composer would have tried to open as a file. Web and mobile now share one client-runtime rule for which skill paths are openable. Co-Authored-By: Claude Fable 5.1 --- apps/mobile/src/components/ComposerEditor.tsx | 8 ++++++-- .../src/provider/Drivers/OhMyPiSkillDispatch.ts | 5 +++-- .../src/components/ComposerPromptEditorTiptap.tsx | 14 +++++++++++--- packages/client-runtime/src/providerSkills.test.ts | 14 ++++++++++++++ packages/client-runtime/src/providerSkills.ts | 12 ++++++++++++ 5 files changed, 46 insertions(+), 7 deletions(-) diff --git a/apps/mobile/src/components/ComposerEditor.tsx b/apps/mobile/src/components/ComposerEditor.tsx index a697ad6bc11f..7fa7c23bf4e6 100644 --- a/apps/mobile/src/components/ComposerEditor.tsx +++ b/apps/mobile/src/components/ComposerEditor.tsx @@ -1,3 +1,4 @@ +import { resolveProviderSkillInstructionsPath } from "@t3tools/client-runtime/providerSkills"; import { ComposerContextId } from "@t3tools/contracts"; import { useAtomValue } from "@effect/atom-react"; import { AsyncResult } from "effect/unstable/reactivity"; @@ -159,6 +160,9 @@ export function ComposerEditor({ const selectedSkill = selectedSkillName ? props.skills?.find((skill) => skill.name === selectedSkillName) : undefined; + const selectedSkillInstructionsPath = selectedSkill + ? resolveProviderSkillInstructionsPath(selectedSkill) + : undefined; const record = draft.context?.records.find( (entry) => entry.contextId === selectedReference?.contextId, ); @@ -221,11 +225,11 @@ export function ComposerEditor({ : undefined) } {...(selectedSkill?.description ? { skillDescription: selectedSkill.description } : {})} - {...(selectedSkill?.path && onOpenMention + {...(selectedSkillInstructionsPath && onOpenMention ? { onOpenSkill: () => { setSelected(null); - onOpenMention(selectedSkill.path!); + onOpenMention(selectedSkillInstructionsPath); }, } : {})} diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts index 3dfc00e9ae87..28097436d893 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts @@ -96,8 +96,9 @@ function opensWithCommand(text: string, commandNames: ReadonlySet): bool /** * omp advertises each skill as a `skill:` command. Those become `$` - * skills, identified only by omp's own `skill://` scheme, and leave the slash - * menu; everything else stays a slash command. + * skills and leave the slash menu; everything else stays a slash command. A + * skill's only identifier is omp's own `skill://` scheme: the clients pick a + * source badge from it and do not offer to open it as a file. */ export function splitOhMyPiAvailableCommands( commands: ReadonlyArray<{ diff --git a/apps/web/src/components/ComposerPromptEditorTiptap.tsx b/apps/web/src/components/ComposerPromptEditorTiptap.tsx index c108207304c4..30417a70c454 100644 --- a/apps/web/src/components/ComposerPromptEditorTiptap.tsx +++ b/apps/web/src/components/ComposerPromptEditorTiptap.tsx @@ -75,7 +75,10 @@ import { ComposerContextRecordsContext, } from "./composerContextPresentation"; import type { AssistantCitationSourceAnchor } from "~/lib/assistantTextSelection"; -import { formatProviderSkillDisplayName } from "@t3tools/client-runtime/providerSkills"; +import { + formatProviderSkillDisplayName, + resolveProviderSkillInstructionsPath, +} from "@t3tools/client-runtime/providerSkills"; import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip"; import { importPastedComposerText } from "./composerInlineTokenPaste"; import { didComposerSelectionChangeVisibly } from "./composerSelection"; @@ -278,6 +281,7 @@ function ComposerSkillNodeView({ node }: NodeViewProps) { const skillLabel = (node.attrs.skillLabel as string) || skillName; const skillDescription = (node.attrs.skillDescription as string | null) ?? null; const skill = skills.find((candidate) => candidate.name === skillName); + const skillInstructionsPath = skill ? resolveProviderSkillInstructionsPath(skill) : undefined; return ( - {skill?.path ? ( - ) : null} diff --git a/packages/client-runtime/src/providerSkills.test.ts b/packages/client-runtime/src/providerSkills.test.ts index 62d0319db1bc..e5d202247267 100644 --- a/packages/client-runtime/src/providerSkills.test.ts +++ b/packages/client-runtime/src/providerSkills.test.ts @@ -8,6 +8,7 @@ import { getProviderSkillsForSlashMenu, resolveProviderSkillsForCwd, resolveProviderSlashCommandsForCwd, + resolveProviderSkillInstructionsPath, resolveProviderSkillSourceKind, } from "./providerSkills.ts"; @@ -185,6 +186,19 @@ describe("getProviderSlashCommandsForSlashMenu", () => { }); }); +describe("resolveProviderSkillInstructionsPath", () => { + it("offers filesystem paths and withholds scheme identifiers", () => { + expect( + resolveProviderSkillInstructionsPath({ path: "/home/dev/.claude/skills/x/SKILL.md" }), + ).toBe("/home/dev/.claude/skills/x/SKILL.md"); + expect(resolveProviderSkillInstructionsPath({ path: "C:\\Users\\dev\\SKILL.md" })).toBe( + "C:\\Users\\dev\\SKILL.md", + ); + expect(resolveProviderSkillInstructionsPath({ path: "skill://grill-me" })).toBeUndefined(); + expect(resolveProviderSkillInstructionsPath({})).toBeUndefined(); + }); +}); + describe("resolveProviderSkillSourceKind", () => { it("marks plugin-backed skills as app installs", () => { expect( diff --git a/packages/client-runtime/src/providerSkills.ts b/packages/client-runtime/src/providerSkills.ts index b80cd5803809..b369f96bf5d8 100644 --- a/packages/client-runtime/src/providerSkills.ts +++ b/packages/client-runtime/src/providerSkills.ts @@ -19,6 +19,18 @@ function normalizePathSeparators(pathValue: string): string { return pathValue.replaceAll("\\", "/"); } +/** + * The skill's instruction file, when the provider reported one. OhMyPi names + * its skills with its own `skill://` scheme, which no client can open, so a + * composer must not offer to view those. + */ +export function resolveProviderSkillInstructionsPath( + skill: Partial>, +): string | undefined { + const path = skill.path?.trim() ?? ""; + return path.length > 0 && !/^[a-z][a-z0-9+.-]*:\/\//iu.test(path) ? path : undefined; +} + export function formatProviderSkillDisplayName( skill: Pick, ): string { From f085923304c376bd760b6c8945e777e5f2951c7f Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 12:38:56 +0000 Subject: [PATCH 06/18] codex: address PR review feedback (#39) State which toggles the launch overlay carries; prewalk travels as a flag. Co-Authored-By: Claude Fable 5.1 --- apps/server/src/provider/OhMyPiSessionOptions.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/server/src/provider/OhMyPiSessionOptions.ts b/apps/server/src/provider/OhMyPiSessionOptions.ts index bb5092c05f40..5d5fb70d0a65 100644 --- a/apps/server/src/provider/OhMyPiSessionOptions.ts +++ b/apps/server/src/provider/OhMyPiSessionOptions.ts @@ -47,9 +47,10 @@ export function resolveOhMyPiSessionToggles( } /** - * The overlay omp reads through `--config`. Keys are nested, not dotted: omp + * The overlay omp reads through `--config`, carrying the two toggles that have + * no off flag; prewalk travels as a flag. Keys are nested, not dotted: omp * splits its own dotted setting paths when it reads the merged tree. The file - * name encodes the content, so concurrent sessions with different toggles + * name encodes the two values, so concurrent sessions that differ on either * never share one file. */ export function ohMyPiConfigOverlay(toggles: OhMyPiSessionToggles): { From e4ac9912f60243aef2c5b01b137c1195670f4c27 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 12:50:16 +0000 Subject: [PATCH 07/18] codex: address PR review feedback (#39) Return skill instruction paths with the / separators the clients use for every path, so the View instructions action resolves consistently on Windows. Co-Authored-By: Claude Fable 5.1 --- packages/client-runtime/src/providerSkills.test.ts | 2 +- packages/client-runtime/src/providerSkills.ts | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/client-runtime/src/providerSkills.test.ts b/packages/client-runtime/src/providerSkills.test.ts index e5d202247267..340873701d56 100644 --- a/packages/client-runtime/src/providerSkills.test.ts +++ b/packages/client-runtime/src/providerSkills.test.ts @@ -192,7 +192,7 @@ describe("resolveProviderSkillInstructionsPath", () => { resolveProviderSkillInstructionsPath({ path: "/home/dev/.claude/skills/x/SKILL.md" }), ).toBe("/home/dev/.claude/skills/x/SKILL.md"); expect(resolveProviderSkillInstructionsPath({ path: "C:\\Users\\dev\\SKILL.md" })).toBe( - "C:\\Users\\dev\\SKILL.md", + "C:/Users/dev/SKILL.md", ); expect(resolveProviderSkillInstructionsPath({ path: "skill://grill-me" })).toBeUndefined(); expect(resolveProviderSkillInstructionsPath({})).toBeUndefined(); diff --git a/packages/client-runtime/src/providerSkills.ts b/packages/client-runtime/src/providerSkills.ts index b369f96bf5d8..a5aa0f0774ba 100644 --- a/packages/client-runtime/src/providerSkills.ts +++ b/packages/client-runtime/src/providerSkills.ts @@ -20,15 +20,18 @@ function normalizePathSeparators(pathValue: string): string { } /** - * The skill's instruction file, when the provider reported one. OhMyPi names - * its skills with its own `skill://` scheme, which no client can open, so a - * composer must not offer to view those. + * The skill's instruction file, when the provider reported one, with the `/` + * separators the clients use for every path. OhMyPi names its skills with its + * own `skill://` scheme, which no client can open, so a composer must not + * offer to view those. */ export function resolveProviderSkillInstructionsPath( skill: Partial>, ): string | undefined { const path = skill.path?.trim() ?? ""; - return path.length > 0 && !/^[a-z][a-z0-9+.-]*:\/\//iu.test(path) ? path : undefined; + return path.length > 0 && !/^[a-z][a-z0-9+.-]*:\/\//iu.test(path) + ? normalizePathSeparators(path) + : undefined; } export function formatProviderSkillDisplayName( From 6dc64719ffc851f24d49203d7c4e9e16aecfbd58 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 13:00:10 +0000 Subject: [PATCH 08/18] codex: address PR review feedback (#39) Wait for the session's own command list before its first prompt. A catalog cached from the probe or an earlier session could otherwise go stale across a restart; omp reports afresh after every session setup, so the adapter awaits that report, bounded, and the driver no longer waits on its own. Co-Authored-By: Claude Fable 5.1 --- .../src/provider/Drivers/OhMyPiDriver.ts | 26 +------------------ .../src/provider/Layers/OhMyPiAdapter.ts | 23 ++++++++++++---- 2 files changed, 19 insertions(+), 30 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.ts index 33743243c252..293bfb16e9e6 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.ts @@ -7,10 +7,8 @@ import { import { createModelCapabilities } from "@t3tools/shared/model"; import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; -import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; -import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; @@ -19,7 +17,6 @@ import { ChildProcessSpawner } from "effect/unstable/process"; import * as BackgroundPolicy from "../../background/BackgroundPolicy.ts"; import { ServerConfig } from "../../config.ts"; import { ServerSettingsService } from "../../serverSettings.ts"; -import { subscribeBeforeSnapshotWithoutMutex } from "../../utils/subscribeBeforeSnapshot.ts"; import { ProviderDriverError } from "../Errors.ts"; import { probeOhMyPiWorkspaceCommands } from "../acp/OhMyPiAcpSupport.ts"; import { makeOhMyPiAdapter } from "../Layers/OhMyPiAdapter.ts"; @@ -52,11 +49,6 @@ const capabilities = createModelCapabilities({ }); /** Live sessions and probes both report per workspace; keep a bounded set. */ const MAX_WORKSPACE_SNAPSHOTS = 32; -/** - * omp sends its command list about fifty milliseconds after `session/new`, - * and the probe takes about a second; past this a prompt goes out unprepared. - */ -const WORKSPACE_CATALOG_WAIT = Duration.seconds(5); export type OhMyPiDriverEnv = | BackgroundPolicy.BackgroundPolicy @@ -133,22 +125,6 @@ export const OhMyPiDriver: ProviderDriver = { draft.workspaceSnapshots?.find((workspace) => workspace.cwd === cwd), ), ); - // The first turn in a fresh workspace can outrun both the live session's - // command update and the probe; subscribe before re-checking so an update - // between the two cannot be missed. - const awaitWorkspace = (cwd: string) => - subscribeBeforeSnapshotWithoutMutex(metadata.pubsub, SubscriptionRef.get(metadata)).pipe( - Effect.flatMap(({ latest, changes }) => - Stream.concat(Stream.make(latest), changes).pipe( - Stream.map((draft) => draft.workspaceSnapshots?.find((entry) => entry.cwd === cwd)), - Stream.filter((workspace) => workspace !== undefined), - Stream.runHead, - ), - ), - Effect.scoped, - Effect.timeoutOption(WORKSPACE_CATALOG_WAIT), - Effect.map((found) => Option.getOrUndefined(Option.flatten(found))), - ); // omp's command list is the one source for a workspace's skills and slash // commands, whether a live session or the probe reported it; a live session // always replaces what the probe found. @@ -264,7 +240,7 @@ export const OhMyPiDriver: ProviderDriver = { environment: processEnv, ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), onAvailableCommands: (commands, cwd) => recordWorkspaceCommands(cwd, commands), - workspaceCatalog: awaitWorkspace, + workspaceCatalog: findWorkspace, }); const unsupported = (operation: string) => Effect.fail( diff --git a/apps/server/src/provider/Layers/OhMyPiAdapter.ts b/apps/server/src/provider/Layers/OhMyPiAdapter.ts index 6de537ad21ee..4d2fd1cb02d8 100644 --- a/apps/server/src/provider/Layers/OhMyPiAdapter.ts +++ b/apps/server/src/provider/Layers/OhMyPiAdapter.ts @@ -25,6 +25,7 @@ import { import * as DateTime from "effect/DateTime"; import * as Crypto from "effect/Crypto"; import * as Deferred from "effect/Deferred"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; import * as Fiber from "effect/Fiber"; @@ -85,6 +86,12 @@ const encodeUnknownJsonStringExit = Schema.encodeUnknownExit(Schema.fromJsonStri const PROVIDER = ProviderDriverKind.make("ohMyPi"); const OH_MY_PI_RESUME_VERSION = 1 as const; +/** + * omp reports its command list about fifty milliseconds after session setup, + * on new and resumed sessions alike; past this bound a prompt goes out with + * whatever the workspace already knew. + */ +const OH_MY_PI_COMMANDS_WAIT = Duration.seconds(5); function encodeJsonStringForDiagnostics(input: unknown): string | undefined { const result = encodeUnknownJsonStringExit(input); return Exit.isSuccess(result) ? result.value : undefined; @@ -103,11 +110,7 @@ export interface OhMyPiAdapterLiveOptions { commands: ReadonlyArray, cwd: string, ) => Effect.Effect; - /** - * The workspace's known commands and skills, from the probe or a session. - * May wait for them: the first turn in a fresh workspace can otherwise - * outrun omp's command update and go out unprepared. - */ + /** The workspace's known commands and skills, from the probe or a session. */ readonly workspaceCatalog?: ( cwd: string, ) => Effect.Effect; @@ -336,6 +339,8 @@ interface OhMyPiSessionContext { promptsInFlight: number; interruptionVersion: number; stopped: boolean; + /** Settled once this session reported its own command list. */ + readonly commandsReported: Deferred.Deferred; } function settlePendingApprovalsAsCancelled( @@ -1003,6 +1008,7 @@ export function makeOhMyPiAdapter( promptsInFlight: 0, interruptionVersion: 0, stopped: false, + commandsReported: yield* Deferred.make(), }; for (const observed of pendingObservedToolCalls.splice(0)) { yield* emitOhMyPiChildTaskEvents(ctx, observed); @@ -1021,6 +1027,7 @@ export function makeOhMyPiAdapter( yield* ( options?.onAvailableCommands?.(event.availableCommands, cwd) ?? Effect.void ); + yield* Deferred.succeed(ctx.commandsReported, undefined); return; case "ConnectionTerminated": ctx.session = { ...ctx.session, status: "error", updatedAt: yield* nowIso }; @@ -1233,6 +1240,12 @@ export function makeOhMyPiAdapter( } const promptParts: Array = []; + // The first prompt waits for this session's own command list, so a + // catalog cached from a probe or an earlier session cannot go stale + // across a restart. + yield* Deferred.await(ctx.commandsReported).pipe( + Effect.timeoutOption(OH_MY_PI_COMMANDS_WAIT), + ); const workspaceCwd = ctx.session.cwd; const prompt = prepareOhMyPiPrompt( input.input?.trim() ?? "", From b41aaff68c76e554db65a7033d480bb4bd94b49a Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 13:10:53 +0000 Subject: [PATCH 09/18] codex: address PR review feedback (#39) Rewrite skill mentions only in the user's own text. Attached terminal, file, and selection context travels in the projected envelope, which is data, so a $name inside it must neither become an invocation nor drop the runtime block. A shared splitter separates the envelope; the OhMyPi dispatcher uses it. Also reject every URI scheme, not only ://, when offering a skill's instruction file, while keeping Windows drive letters openable. Co-Authored-By: Claude Fable 5.1 --- .../Drivers/OhMyPiSkillDispatch.test.ts | 13 +++++++++++ .../provider/Drivers/OhMyPiSkillDispatch.ts | 7 ++++-- .../client-runtime/src/providerSkills.test.ts | 2 ++ packages/client-runtime/src/providerSkills.ts | 5 +++-- .../src/composerContextReferences.test.ts | 16 ++++++++++++++ .../shared/src/composerContextReferences.ts | 22 ++++++++++++++++++- 6 files changed, 60 insertions(+), 5 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts index c81b3f186427..d8f6b7fba36d 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts @@ -32,6 +32,19 @@ describe("prepareOhMyPiPrompt", () => { }); }); + it("leaves attached context verbatim and never dispatches from it", () => { + const envelope = + '\n\n\n$grill-me\n'; + expect(prepareOhMyPiPrompt(`fix this${envelope}`, catalog)).toEqual({ + text: `fix this${envelope}`, + consumedByCommand: false, + }); + expect(prepareOhMyPiPrompt(`$grill-me${envelope}`, catalog)).toEqual({ + text: `/skill:grill-me${envelope}`, + consumedByCommand: true, + }); + }); + it("follows omp's prefix rules for inline skill tokens", () => { expect(prepareOhMyPiPrompt("/tmp/x is broken, $grill-me", catalog).consumedByCommand).toBe( false, diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts index 28097436d893..bcdcb33a3d70 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts @@ -20,6 +20,7 @@ import type { ServerProviderSlashCommand, ServerProviderWorkspaceSnapshot, } from "@t3tools/contracts"; +import { splitComposerContextEnvelope } from "@t3tools/shared/composerContextReferences"; /** * Same token shape the Claude and Cursor skill dispatchers use, so a `$name` @@ -61,11 +62,13 @@ export function prepareOhMyPiPrompt( prompt: string, catalog: OhMyPiWorkspaceCatalog, ): OhMyPiPreparedPrompt { - const text = prompt.replace(SKILL_MENTION_PATTERN, (match, prefix: string, name: string) => + // Only the user's own text carries mentions; attached context is data. + const { body, envelope } = splitComposerContextEnvelope(prompt); + const text = body.replace(SKILL_MENTION_PATTERN, (match, prefix: string, name: string) => catalog.skillNames.has(name) ? `${prefix}/${SKILL_COMMAND_PREFIX}${name}` : match, ); return { - text, + text: `${text}${envelope}`, consumedByCommand: invokesSkill(text, catalog.skillNames) || opensWithCommand(text, catalog.commandNames), }; diff --git a/packages/client-runtime/src/providerSkills.test.ts b/packages/client-runtime/src/providerSkills.test.ts index 340873701d56..7cd6063911ca 100644 --- a/packages/client-runtime/src/providerSkills.test.ts +++ b/packages/client-runtime/src/providerSkills.test.ts @@ -195,6 +195,8 @@ describe("resolveProviderSkillInstructionsPath", () => { "C:/Users/dev/SKILL.md", ); expect(resolveProviderSkillInstructionsPath({ path: "skill://grill-me" })).toBeUndefined(); + expect(resolveProviderSkillInstructionsPath({ path: "file:///tmp/SKILL.md" })).toBeUndefined(); + expect(resolveProviderSkillInstructionsPath({ path: "mailto:dev" })).toBeUndefined(); expect(resolveProviderSkillInstructionsPath({})).toBeUndefined(); }); }); diff --git a/packages/client-runtime/src/providerSkills.ts b/packages/client-runtime/src/providerSkills.ts index a5aa0f0774ba..1758e7f80229 100644 --- a/packages/client-runtime/src/providerSkills.ts +++ b/packages/client-runtime/src/providerSkills.ts @@ -23,13 +23,14 @@ function normalizePathSeparators(pathValue: string): string { * The skill's instruction file, when the provider reported one, with the `/` * separators the clients use for every path. OhMyPi names its skills with its * own `skill://` scheme, which no client can open, so a composer must not - * offer to view those. + * offer to view those. A URI scheme has two or more characters before its + * colon, which keeps Windows drive letters openable. */ export function resolveProviderSkillInstructionsPath( skill: Partial>, ): string | undefined { const path = skill.path?.trim() ?? ""; - return path.length > 0 && !/^[a-z][a-z0-9+.-]*:\/\//iu.test(path) + return path.length > 0 && !/^[a-z][a-z0-9+.-]+:/iu.test(path) ? normalizePathSeparators(path) : undefined; } diff --git a/packages/shared/src/composerContextReferences.test.ts b/packages/shared/src/composerContextReferences.test.ts index 99d16b6c8569..435ec7ef1074 100644 --- a/packages/shared/src/composerContextReferences.test.ts +++ b/packages/shared/src/composerContextReferences.test.ts @@ -10,6 +10,7 @@ import { projectComposerContextForProvider, replaceComposerContextReferences, sanitizeComposerContextLabel, + splitComposerContextEnvelope, } from "./composerContextReferences.ts"; const ctx = (value: string) => value as ComposerContextId; @@ -263,3 +264,18 @@ describe("provider projection", () => { expect(projected).not.toContain("boom"); }); }); + +describe("splitComposerContextEnvelope", () => { + it("separates the user's text from the projected envelope", () => { + const projected = + 'run it\n\n\n$grill-me\n'; + expect(splitComposerContextEnvelope(projected)).toEqual({ + body: "run it", + envelope: projected.slice("run it".length), + }); + expect(splitComposerContextEnvelope("plain prose")).toEqual({ + body: "plain prose", + envelope: "", + }); + }); +}); diff --git a/packages/shared/src/composerContextReferences.ts b/packages/shared/src/composerContextReferences.ts index 23993cb9d407..2dbb20ccc59c 100644 --- a/packages/shared/src/composerContextReferences.ts +++ b/packages/shared/src/composerContextReferences.ts @@ -287,5 +287,25 @@ export function projectComposerContextForProvider(input: { entries.push(entry); } if (entries.length === 0) return body; - return `${body}\n\n<${CONTEXT_ENVELOPE_TAG} version="1">\n${entries.join("\n")}\n`; + return `${body}${CONTEXT_ENVELOPE_OPENING}${entries.join("\n")}\n`; +} + +const CONTEXT_ENVELOPE_OPENING = `\n\n<${CONTEXT_ENVELOPE_TAG} version="1">\n`; + +/** + * Split a projected prompt into the user-authored text and the context + * envelope, empty when there is none. Payload escaping keeps the opening tag + * out of captured data, so its first occurrence is the envelope. Adapters that + * rewrite the user's text, such as skill dispatch, must leave the envelope + * verbatim: a `$name` inside an attached terminal line is data, not a mention. + */ +export function splitComposerContextEnvelope(text: string): { + readonly body: string; + readonly envelope: string; +} { + const start = text.indexOf(CONTEXT_ENVELOPE_OPENING); + if (start === -1 || !text.endsWith(``)) { + return { body: text, envelope: "" }; + } + return { body: text.slice(0, start), envelope: text.slice(start) }; } From c1334d5df3c61a3c1a9170a7102b71ef46eda644 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 13:20:27 +0000 Subject: [PATCH 10/18] codex: address PR review feedback (#39) Send OhMyPi builtin commands without attached context. A builtin never reaches the model, so the projected envelope would only become arguments that strict commands reject; skills keep it, since it reaches the model as the skill's arguments. The envelope splitter now takes the last opening tag, so prose that quotes the tag cannot be mistaken for the envelope. Co-Authored-By: Claude Fable 5.1 --- .../src/provider/Drivers/OhMyPiSkillDispatch.test.ts | 5 +++++ .../src/provider/Drivers/OhMyPiSkillDispatch.ts | 12 +++++++++--- .../shared/src/composerContextReferences.test.ts | 5 +++++ packages/shared/src/composerContextReferences.ts | 10 ++++++---- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts index d8f6b7fba36d..bbad8012e013 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts @@ -43,6 +43,11 @@ describe("prepareOhMyPiPrompt", () => { text: `/skill:grill-me${envelope}`, consumedByCommand: true, }); + // A builtin never reaches the model, so attached context would only break its arguments. + expect(prepareOhMyPiPrompt(`/computer status${envelope}`, catalog)).toEqual({ + text: "/computer status", + consumedByCommand: true, + }); }); it("follows omp's prefix rules for inline skill tokens", () => { diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts index bcdcb33a3d70..1e42dd3600e5 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts @@ -42,6 +42,11 @@ export interface OhMyPiWorkspaceCatalog { } export interface OhMyPiPreparedPrompt { + /** + * What omp receives. A skill keeps the attached context, which reaches the + * model as the skill's arguments; a builtin command never reaches the model, + * so it travels bare or its arguments would not parse. + */ readonly text: string; /** omp itself consumes the prompt (a command or a skill), so nothing else may share it. */ readonly consumedByCommand: boolean; @@ -67,10 +72,11 @@ export function prepareOhMyPiPrompt( const text = body.replace(SKILL_MENTION_PATTERN, (match, prefix: string, name: string) => catalog.skillNames.has(name) ? `${prefix}/${SKILL_COMMAND_PREFIX}${name}` : match, ); + const skill = invokesSkill(text, catalog.skillNames); + const command = !skill && opensWithCommand(text, catalog.commandNames); return { - text: `${text}${envelope}`, - consumedByCommand: - invokesSkill(text, catalog.skillNames) || opensWithCommand(text, catalog.commandNames), + text: command ? text : `${text}${envelope}`, + consumedByCommand: skill || command, }; } diff --git a/packages/shared/src/composerContextReferences.test.ts b/packages/shared/src/composerContextReferences.test.ts index 435ec7ef1074..0ef5e363b9db 100644 --- a/packages/shared/src/composerContextReferences.test.ts +++ b/packages/shared/src/composerContextReferences.test.ts @@ -277,5 +277,10 @@ describe("splitComposerContextEnvelope", () => { body: "plain prose", envelope: "", }); + const prose = 'quote this:\n\n\nnot an envelope'; + expect(splitComposerContextEnvelope(`${prose}${projected.slice("run it".length)}`)).toEqual({ + body: prose, + envelope: projected.slice("run it".length), + }); }); }); diff --git a/packages/shared/src/composerContextReferences.ts b/packages/shared/src/composerContextReferences.ts index 2dbb20ccc59c..f288705ce824 100644 --- a/packages/shared/src/composerContextReferences.ts +++ b/packages/shared/src/composerContextReferences.ts @@ -295,15 +295,17 @@ const CONTEXT_ENVELOPE_OPENING = `\n\n<${CONTEXT_ENVELOPE_TAG} version="1">\n`; /** * Split a projected prompt into the user-authored text and the context * envelope, empty when there is none. Payload escaping keeps the opening tag - * out of captured data, so its first occurrence is the envelope. Adapters that - * rewrite the user's text, such as skill dispatch, must leave the envelope - * verbatim: a `$name` inside an attached terminal line is data, not a mention. + * out of captured data, and the envelope is appended last, so its final + * occurrence is the envelope even when the user's own prose contains the tag. + * Adapters that rewrite the user's text, such as skill dispatch, must leave + * the envelope verbatim: a `$name` inside an attached terminal line is data, + * not a mention. */ export function splitComposerContextEnvelope(text: string): { readonly body: string; readonly envelope: string; } { - const start = text.indexOf(CONTEXT_ENVELOPE_OPENING); + const start = text.lastIndexOf(CONTEXT_ENVELOPE_OPENING); if (start === -1 || !text.endsWith(``)) { return { body: text, envelope: "" }; } From c0d33624bf88ab3a90892cc5c1830fe7bd2c786a Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 13:29:07 +0000 Subject: [PATCH 11/18] codex: address PR review feedback (#39) Strip in-place context markers from OhMyPi builtin commands as well as the envelope, so a command with an attached reference travels bare and strict commands still parse. Co-Authored-By: Claude Fable 5.1 --- .../provider/Drivers/OhMyPiSkillDispatch.test.ts | 4 +++- .../src/provider/Drivers/OhMyPiSkillDispatch.ts | 7 +++++-- .../shared/src/composerContextReferences.test.ts | 14 ++++++++++++++ packages/shared/src/composerContextReferences.ts | 14 ++++++++++++++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts index bbad8012e013..c8ab1987a9f3 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.test.ts @@ -44,7 +44,9 @@ describe("prepareOhMyPiPrompt", () => { consumedByCommand: true, }); // A builtin never reaches the model, so attached context would only break its arguments. - expect(prepareOhMyPiPrompt(`/computer status${envelope}`, catalog)).toEqual({ + expect( + prepareOhMyPiPrompt(`/computer status [Terminal: log; ref=ctx_1]${envelope}`, catalog), + ).toEqual({ text: "/computer status", consumedByCommand: true, }); diff --git a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts index 1e42dd3600e5..9dbd8965ae69 100644 --- a/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts +++ b/apps/server/src/provider/Drivers/OhMyPiSkillDispatch.ts @@ -20,7 +20,10 @@ import type { ServerProviderSlashCommand, ServerProviderWorkspaceSnapshot, } from "@t3tools/contracts"; -import { splitComposerContextEnvelope } from "@t3tools/shared/composerContextReferences"; +import { + splitComposerContextEnvelope, + stripComposerContextMarkers, +} from "@t3tools/shared/composerContextReferences"; /** * Same token shape the Claude and Cursor skill dispatchers use, so a `$name` @@ -75,7 +78,7 @@ export function prepareOhMyPiPrompt( const skill = invokesSkill(text, catalog.skillNames); const command = !skill && opensWithCommand(text, catalog.commandNames); return { - text: command ? text : `${text}${envelope}`, + text: command ? stripComposerContextMarkers(text) : `${text}${envelope}`, consumedByCommand: skill || command, }; } diff --git a/packages/shared/src/composerContextReferences.test.ts b/packages/shared/src/composerContextReferences.test.ts index 0ef5e363b9db..755b64938d07 100644 --- a/packages/shared/src/composerContextReferences.test.ts +++ b/packages/shared/src/composerContextReferences.test.ts @@ -11,6 +11,7 @@ import { replaceComposerContextReferences, sanitizeComposerContextLabel, splitComposerContextEnvelope, + stripComposerContextMarkers, } from "./composerContextReferences.ts"; const ctx = (value: string) => value as ComposerContextId; @@ -284,3 +285,16 @@ describe("splitComposerContextEnvelope", () => { }); }); }); + +describe("stripComposerContextMarkers", () => { + it("removes projected markers and tidies the spacing", () => { + expect( + stripComposerContextMarkers( + "/computer [Terminal: build log; ref=ctx_1] status [Pull request: #39; ref=ctx_2]", + ), + ).toBe("/computer status"); + expect(stripComposerContextMarkers("plain [not a marker] text")).toBe( + "plain [not a marker] text", + ); + }); +}); diff --git a/packages/shared/src/composerContextReferences.ts b/packages/shared/src/composerContextReferences.ts index f288705ce824..968e828547b8 100644 --- a/packages/shared/src/composerContextReferences.ts +++ b/packages/shared/src/composerContextReferences.ts @@ -291,6 +291,20 @@ export function projectComposerContextForProvider(input: { } const CONTEXT_ENVELOPE_OPENING = `\n\n<${CONTEXT_ENVELOPE_TAG} version="1">\n`; +/** The in-place marker `formatComposerContextProviderMarker` writes; labels never contain `]`. */ +const CONTEXT_MARKER_PATTERN = /\[[A-Z][A-Za-z ]*: [^\]]*; ref=[^\]\s]+\]/gu; + +/** + * Remove the in-place markers a projection left in the user's text, for a + * provider command that never reaches the model and would otherwise receive + * them as arguments. + */ +export function stripComposerContextMarkers(text: string): string { + return text + .replace(CONTEXT_MARKER_PATTERN, "") + .replace(/[ \t]{2,}/gu, " ") + .trim(); +} /** * Split a projected prompt into the user-authored text and the context From 96e584ebc3398936548145ba28afaf4b3ce501e3 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 13:39:09 +0000 Subject: [PATCH 12/18] codex: address PR review feedback (#39) Settle the OhMyPi command-catalog wait on its first timeout so a process that never reports commands costs one wait, not one per turn. Match every formatted context kind, digits included, when stripping markers from a builtin command. Co-Authored-By: Claude Fable 5.1 --- apps/server/src/provider/Layers/OhMyPiAdapter.ts | 8 +++++++- packages/shared/src/composerContextReferences.test.ts | 3 +++ packages/shared/src/composerContextReferences.ts | 8 ++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/apps/server/src/provider/Layers/OhMyPiAdapter.ts b/apps/server/src/provider/Layers/OhMyPiAdapter.ts index 4d2fd1cb02d8..a3f304b6509a 100644 --- a/apps/server/src/provider/Layers/OhMyPiAdapter.ts +++ b/apps/server/src/provider/Layers/OhMyPiAdapter.ts @@ -1242,9 +1242,15 @@ export function makeOhMyPiAdapter( const promptParts: Array = []; // The first prompt waits for this session's own command list, so a // catalog cached from a probe or an earlier session cannot go stale - // across a restart. + // across a restart. A process that never reports settles the wait on + // its first timeout, so later prompts do not pay it again. yield* Deferred.await(ctx.commandsReported).pipe( Effect.timeoutOption(OH_MY_PI_COMMANDS_WAIT), + Effect.flatMap((reported) => + Option.isSome(reported) + ? Effect.void + : Deferred.succeed(ctx.commandsReported, undefined), + ), ); const workspaceCwd = ctx.session.cwd; const prompt = prepareOhMyPiPrompt( diff --git a/packages/shared/src/composerContextReferences.test.ts b/packages/shared/src/composerContextReferences.test.ts index 755b64938d07..54d5c90b2e28 100644 --- a/packages/shared/src/composerContextReferences.test.ts +++ b/packages/shared/src/composerContextReferences.test.ts @@ -293,6 +293,9 @@ describe("stripComposerContextMarkers", () => { "/computer [Terminal: build log; ref=ctx_1] status [Pull request: #39; ref=ctx_2]", ), ).toBe("/computer status"); + expect(stripComposerContextMarkers("/advisor on [Foo 2: later kind; ref=ctx_3]")).toBe( + "/advisor on", + ); expect(stripComposerContextMarkers("plain [not a marker] text")).toBe( "plain [not a marker] text", ); diff --git a/packages/shared/src/composerContextReferences.ts b/packages/shared/src/composerContextReferences.ts index 968e828547b8..694eeb718463 100644 --- a/packages/shared/src/composerContextReferences.ts +++ b/packages/shared/src/composerContextReferences.ts @@ -291,8 +291,12 @@ export function projectComposerContextForProvider(input: { } const CONTEXT_ENVELOPE_OPENING = `\n\n<${CONTEXT_ENVELOPE_TAG} version="1">\n`; -/** The in-place marker `formatComposerContextProviderMarker` writes; labels never contain `]`. */ -const CONTEXT_MARKER_PATTERN = /\[[A-Z][A-Za-z ]*: [^\]]*; ref=[^\]\s]+\]/gu; +/** + * The in-place marker `formatComposerContextProviderMarker` writes. Kinds are + * `[a-z0-9-]` and display with spaces for dashes, so a label like `Foo 2` + * carries digits; captured labels never contain `]`. + */ +const CONTEXT_MARKER_PATTERN = /\[[A-Z][A-Za-z0-9 ]*: [^\]]*; ref=[^\]\s]+\]/gu; /** * Remove the in-place markers a projection left in the user's text, for a From cc748b19c1e4d9e9c4f99539a7c13b401dea15e8 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 16:10:58 +0000 Subject: [PATCH 13/18] fix(web): keep the traits menu open when it shows several sections Picking an option closed the menu, so setting thinking, advisor, and computer use for one thread meant opening it three times. A lone section still closes on pick, like the model picker. Co-Authored-By: Claude Fable 5.1 --- apps/web/src/components/chat/TraitsPicker.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/chat/TraitsPicker.tsx b/apps/web/src/components/chat/TraitsPicker.tsx index a1ad8cb6cc0e..0b191f0f299f 100644 --- a/apps/web/src/components/chat/TraitsPicker.tsx +++ b/apps/web/src/components/chat/TraitsPicker.tsx @@ -339,6 +339,10 @@ export const TraitsMenuContent = memo(function TraitsMenuContentImpl({ const updateDescriptors = (nextDescriptors: ReadonlyArray) => { updateModelOptions(buildProviderOptionSelectionsFromDescriptors(nextDescriptors)); }; + // Base UI keeps radio menus open by default. A lone section closes on pick so + // the menu behaves like the model picker; several sections stay open so one + // visit can set them all, instead of reopening the menu per choice. + const closeOnPick = selectDescriptors.length + booleanDescriptors.length <= 1; const handleSelectChange = ( descriptor: Extract, @@ -417,9 +421,7 @@ export const TraitsMenuContent = memo(function TraitsMenuContentImpl({ key={option.id} value={option.id} hideIndicator - // Base UI keeps radio menus open by default. Close on pick so - // the traits menu behaves like the model picker. - closeOnClick + closeOnClick={closeOnPick} disabled={ultrathinkInBodyText && descriptor.id === primarySelectDescriptor?.id} > @@ -466,7 +468,7 @@ export const TraitsMenuContent = memo(function TraitsMenuContentImpl({ }} > {(["on", "off"] as const).map((value) => ( - + {value === "on" ? "On" : "Off"} From 7d48f4807df441b3b7b7f423f39791e5dbc928cf Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 16:14:27 +0000 Subject: [PATCH 14/18] Revert "fix(web): keep the traits menu open when it shows several sections" This reverts commit cc748b19c1e4d9e9c4f99539a7c13b401dea15e8. --- apps/web/src/components/chat/TraitsPicker.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/web/src/components/chat/TraitsPicker.tsx b/apps/web/src/components/chat/TraitsPicker.tsx index 0b191f0f299f..a1ad8cb6cc0e 100644 --- a/apps/web/src/components/chat/TraitsPicker.tsx +++ b/apps/web/src/components/chat/TraitsPicker.tsx @@ -339,10 +339,6 @@ export const TraitsMenuContent = memo(function TraitsMenuContentImpl({ const updateDescriptors = (nextDescriptors: ReadonlyArray) => { updateModelOptions(buildProviderOptionSelectionsFromDescriptors(nextDescriptors)); }; - // Base UI keeps radio menus open by default. A lone section closes on pick so - // the menu behaves like the model picker; several sections stay open so one - // visit can set them all, instead of reopening the menu per choice. - const closeOnPick = selectDescriptors.length + booleanDescriptors.length <= 1; const handleSelectChange = ( descriptor: Extract, @@ -421,7 +417,9 @@ export const TraitsMenuContent = memo(function TraitsMenuContentImpl({ key={option.id} value={option.id} hideIndicator - closeOnClick={closeOnPick} + // Base UI keeps radio menus open by default. Close on pick so + // the traits menu behaves like the model picker. + closeOnClick disabled={ultrathinkInBodyText && descriptor.id === primarySelectDescriptor?.id} > @@ -468,7 +466,7 @@ export const TraitsMenuContent = memo(function TraitsMenuContentImpl({ }} > {(["on", "off"] as const).map((value) => ( - + {value === "on" ? "On" : "Off"} From fae0fa1877ee6d7ea8fa65c4011f2e63f413eab8 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 17:18:06 +0000 Subject: [PATCH 15/18] fix(server): bound the OhMyPi workspace probe and let live commands win The probe only timed out while waiting for the command list, so an omp that hung during the ACP handshake or on session close kept both the probe process and the caller waiting forever. Every wait is now bounded on its own. A probe also launches without a thread's provider options, so it sees only the commands omp's own configuration enables. One that finished after a live session had already reported could therefore narrow the menu it found. A live report now claims the workspace for good and a late probe write is dropped. Co-Authored-By: Claude Opus 5 --- .../src/provider/Drivers/OhMyPiDriver.test.ts | 60 +++++++++++++++++++ .../src/provider/Drivers/OhMyPiDriver.ts | 40 +++++++++---- .../src/provider/acp/OhMyPiAcpSupport.ts | 12 +++- 3 files changed, 99 insertions(+), 13 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts index 6ce418685dc2..10a606cdf795 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts @@ -186,6 +186,66 @@ it.layer(testLayer)("OhMyPi driver", (it) => { }).pipe(Effect.scoped), ); + it.effect("keeps a live command list when a slower probe finishes after it", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const directory = yield* fs.makeTempDirectoryScoped(); + const binaryPath = yield* Effect.sync(() => + writeFakeCli({ + directory, + name: "omp-late-probe-mock", + env: { + // What a thread's own session advertises, reported at once. + // @effect-diagnostics-next-line preferSchemaOverJson:off + T3_ACP_AVAILABLE_COMMANDS: JSON.stringify([ + { name: "computer", description: "Drive the screen" }, + ]), + }, + // The probe is the launch carrying `--session-dir`. It reports a + // narrower list, late, standing in for the commands omp gates behind + // options the probe cannot know. + source: + ` + if (process.argv.includes("--session-dir")) { + process.env.T3_ACP_AVAILABLE_COMMANDS = JSON.stringify([ + { name: "compact", description: "Compact the conversation" }, + ]); + process.env.T3_ACP_AVAILABLE_COMMANDS_DELAY_MS = "400"; + } + ` + + execScriptSource({ + scriptPath: NodeURL.fileURLToPath( + new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), + ), + }), + }), + ); + const instance = yield* OhMyPiDriver.create({ + instanceId, + displayName: undefined, + enabled: true, + environment: [], + config: { ...OhMyPiDriver.defaultConfig(), binaryPath }, + }); + const probe = yield* instance.snapshotForCwd!(directory).pipe(Effect.forkChild); + yield* instance.adapter.startSession({ + threadId, + cwd: directory, + runtimeMode: "full-access", + }); + // The adapter settles its command wait only after the driver has recorded + // the session's list, so the live write has landed once this returns. + yield* instance.adapter.sendTurn({ threadId, input: "hello", attachments: [] }); + yield* Fiber.join(probe); + expect( + (yield* instance.snapshot.getSnapshot).workspaceSnapshots?.map((workspace) => + workspace.slashCommands.map((command) => command.name), + ), + ).toEqual([["computer"]]); + yield* instance.adapter.stopAll(); + }).pipe(Effect.scoped), + ); + for (const sendDuringPreparation of [false, true]) { it.effect(`steers within one turn (send during preparation: ${sendDuringPreparation})`, () => Effect.gen(function* () { diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.ts index 293bfb16e9e6..06ef92fa13c9 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.ts @@ -10,6 +10,7 @@ import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; +import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import * as SubscriptionRef from "effect/SubscriptionRef"; @@ -126,9 +127,14 @@ export const OhMyPiDriver: ProviderDriver = { ), ); // omp's command list is the one source for a workspace's skills and slash - // commands, whether a live session or the probe reported it; a live session - // always replaces what the probe found. + // commands, whether a live session or the probe reported it. omp advertises + // what its own configuration enables, and the probe launches without a + // thread's options, so it can report fewer commands than the session + // running them: once a live session has named a cwd, a probe that lands + // afterwards is dropped rather than allowed to narrow the menu. + const liveWorkspaces = yield* Ref.make>(new Set()); const recordWorkspaceCommands = ( + source: "live" | "probe", cwd: string, commands: ReadonlyArray<{ readonly name: string; @@ -136,13 +142,25 @@ export const OhMyPiDriver: ProviderDriver = { readonly input?: { readonly hint: string } | null; }>, ) => - SubscriptionRef.update(metadata, (draft) => ({ - ...draft, - workspaceSnapshots: [ - ...(draft.workspaceSnapshots ?? []).filter((workspace) => workspace.cwd !== cwd), - { cwd, checkedAt: draft.checkedAt, ...splitOhMyPiAvailableCommands(commands) }, - ].slice(-MAX_WORKSPACE_SNAPSHOTS), - })); + Ref.modify(liveWorkspaces, (live) => + source === "live" + ? ([true, new Set(live).add(cwd)] as const) + : ([!live.has(cwd), live] as const), + ).pipe( + Effect.flatMap((accepted) => + accepted + ? SubscriptionRef.update(metadata, (draft) => ({ + ...draft, + workspaceSnapshots: [ + ...(draft.workspaceSnapshots ?? []).filter( + (workspace) => workspace.cwd !== cwd, + ), + { cwd, checkedAt: draft.checkedAt, ...splitOhMyPiAvailableCommands(commands) }, + ].slice(-MAX_WORKSPACE_SNAPSHOTS), + })) + : Effect.void, + ), + ); // A throwaway ACP session in a T3-owned session directory; see ADR 0006. const probeWorkspace = (cwd: string) => Effect.gen(function* () { @@ -158,7 +176,7 @@ export const OhMyPiDriver: ProviderDriver = { cwd, sessionDir, }); - yield* recordWorkspaceCommands(cwd, commands); + yield* recordWorkspaceCommands("probe", cwd, commands); }).pipe( Effect.scoped, Effect.provideService(Crypto.Crypto, crypto), @@ -239,7 +257,7 @@ export const OhMyPiDriver: ProviderDriver = { instanceId, environment: processEnv, ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), - onAvailableCommands: (commands, cwd) => recordWorkspaceCommands(cwd, commands), + onAvailableCommands: (commands, cwd) => recordWorkspaceCommands("live", cwd, commands), workspaceCatalog: findWorkspace, }); const unsupported = (operation: string) => diff --git a/apps/server/src/provider/acp/OhMyPiAcpSupport.ts b/apps/server/src/provider/acp/OhMyPiAcpSupport.ts index e8e3a59b8f57..8aff787ac559 100644 --- a/apps/server/src/provider/acp/OhMyPiAcpSupport.ts +++ b/apps/server/src/provider/acp/OhMyPiAcpSupport.ts @@ -18,6 +18,8 @@ import * as AcpSessionRuntime from "./AcpSessionRuntime.ts"; const OH_MY_PI_CLIENT_INFO = { name: "t3-code", version: "0.0.0" } as const; /** omp sends `available_commands_update` about fifty milliseconds after `session/new`. */ const OH_MY_PI_WORKSPACE_PROBE_TIMEOUT = Duration.seconds(20); +/** The probe session is disposable, so closing it politely is worth only a moment. */ +const OH_MY_PI_WORKSPACE_PROBE_CLOSE_TIMEOUT = Duration.seconds(2); interface OhMyPiAcpRuntimeInput extends Omit< AcpSessionRuntime.AcpSessionRuntimeOptions, @@ -100,11 +102,17 @@ export const probeOhMyPiWorkspaceCommands = Effect.fn("probeOhMyPiWorkspaceComma return Effect.void; } }).pipe(Effect.forkScoped); - const started = yield* acp.start(); + // Every wait is bounded on its own: an omp that hangs during handshake, + // before reporting, or on close would otherwise keep both the probe process + // and the caller alive forever. Leaving the scope kills the process, so a + // close that does not answer promptly is abandoned rather than waited on. + const started = yield* acp.start().pipe(Effect.timeout(OH_MY_PI_WORKSPACE_PROBE_TIMEOUT)); const available = yield* Deferred.await(commands).pipe( Effect.timeout(OH_MY_PI_WORKSPACE_PROBE_TIMEOUT), ); - yield* acp.request("session/close", { sessionId: started.sessionId }).pipe(Effect.ignore); + yield* acp + .request("session/close", { sessionId: started.sessionId }) + .pipe(Effect.timeout(OH_MY_PI_WORKSPACE_PROBE_CLOSE_TIMEOUT), Effect.ignore); return available; }, Effect.scoped, From e138f2c36ee202cc8ebb04f27f0df788e5e8fa9c Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 17:27:47 +0000 Subject: [PATCH 16/18] fix(server): drop a late OhMyPi probe by reading the workspace entry Remembering which workspaces a live session had claimed kept a second set alongside the snapshots, which are capped at 32. Once a workspace aged out, its marker stayed behind and rejected the probe that would have refilled it, leaving that workspace with empty menus for good. A probe only runs for a workspace with no entry, so an entry that appeared since can only have come from a live session. Reading it is the whole signal, and it ages out with the snapshot it belongs to. Co-Authored-By: Claude Opus 5 --- .../src/provider/Drivers/OhMyPiDriver.ts | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.ts index 06ef92fa13c9..84b32d8ad6ef 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.ts @@ -10,7 +10,6 @@ import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; -import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import * as SubscriptionRef from "effect/SubscriptionRef"; @@ -127,12 +126,7 @@ export const OhMyPiDriver: ProviderDriver = { ), ); // omp's command list is the one source for a workspace's skills and slash - // commands, whether a live session or the probe reported it. omp advertises - // what its own configuration enables, and the probe launches without a - // thread's options, so it can report fewer commands than the session - // running them: once a live session has named a cwd, a probe that lands - // afterwards is dropped rather than allowed to narrow the menu. - const liveWorkspaces = yield* Ref.make>(new Set()); + // commands, whether a live session or the probe reported it. const recordWorkspaceCommands = ( source: "live" | "probe", cwd: string, @@ -142,25 +136,25 @@ export const OhMyPiDriver: ProviderDriver = { readonly input?: { readonly hint: string } | null; }>, ) => - Ref.modify(liveWorkspaces, (live) => - source === "live" - ? ([true, new Set(live).add(cwd)] as const) - : ([!live.has(cwd), live] as const), - ).pipe( - Effect.flatMap((accepted) => - accepted - ? SubscriptionRef.update(metadata, (draft) => ({ - ...draft, - workspaceSnapshots: [ - ...(draft.workspaceSnapshots ?? []).filter( - (workspace) => workspace.cwd !== cwd, - ), - { cwd, checkedAt: draft.checkedAt, ...splitOhMyPiAvailableCommands(commands) }, - ].slice(-MAX_WORKSPACE_SNAPSHOTS), - })) - : Effect.void, - ), - ); + SubscriptionRef.update(metadata, (draft) => { + const recorded = draft.workspaceSnapshots ?? []; + // A probe only runs for a workspace with no entry, so an entry that + // appeared since came from a live session. omp advertises what its own + // configuration enables and the probe launches without a thread's + // options, so letting the slower probe land would narrow the menu. + // Reading the entry rather than remembering the cwd keeps this in step + // with eviction: a workspace that ages out can be probed again. + if (source === "probe" && recorded.some((workspace) => workspace.cwd === cwd)) { + return draft; + } + return { + ...draft, + workspaceSnapshots: [ + ...recorded.filter((workspace) => workspace.cwd !== cwd), + { cwd, checkedAt: draft.checkedAt, ...splitOhMyPiAvailableCommands(commands) }, + ].slice(-MAX_WORKSPACE_SNAPSHOTS), + }; + }); // A throwaway ACP session in a T3-owned session directory; see ADR 0006. const probeWorkspace = (cwd: string) => Effect.gen(function* () { From a3161d136750623e58180d49c0e7a43939447d64 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 17:46:27 +0000 Subject: [PATCH 17/18] fix(server): dispatch OhMyPi prompts from the session's own command list Launch options gate omp's commands, so two threads in one workspace can advertise different sets. Prompt preparation read the shared per-cwd snapshot, so whichever session reported last decided how the other's `$skill` mentions and slash commands were dispatched. Each session now keeps the catalog its own omp reported and prepares from that; the workspace snapshot stays what it was for, the pre-first-turn menu. Co-Authored-By: Claude Opus 5 --- .../src/provider/Drivers/OhMyPiDriver.test.ts | 75 +++++++++++++++++++ .../src/provider/Drivers/OhMyPiDriver.ts | 1 - .../src/provider/Layers/OhMyPiAdapter.ts | 39 ++++++---- 3 files changed, 98 insertions(+), 17 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts index 10a606cdf795..425ae36a8681 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts @@ -44,6 +44,7 @@ const instanceId = ProviderInstanceId.make("omp-test"); /** omp always follows session setup with a command update; the adapter waits for it. */ const OMP_BOOTSTRAP_ENV = { T3_ACP_AVAILABLE_COMMANDS: "[]" } as const; const threadId = ThreadId.make("omp-thread"); +const otherThreadId = ThreadId.make("omp-thread-2"); it.layer(testLayer)("OhMyPi driver", (it) => { it.effect("does not start a disabled CLI", () => @@ -246,6 +247,80 @@ it.layer(testLayer)("OhMyPi driver", (it) => { }).pipe(Effect.scoped), ); + it.effect("dispatches from each session's own commands, not the workspace's", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped(); + const logPath = path.join(directory, "own-catalog.jsonl"); + const otherLogPath = path.join(directory, "other-catalog.jsonl"); + const binaryPath = yield* Effect.sync(() => + writeFakeCli({ + directory, + name: "omp-two-thread-mock", + env: { + T3_ACP_REQUEST_LOG_PATH: logPath, + T3_OTHER_REQUEST_LOG_PATH: otherLogPath, + // @effect-diagnostics-next-line preferSchemaOverJson:off + T3_ACP_AVAILABLE_COMMANDS: JSON.stringify([ + { name: "computer", description: "Drive the screen" }, + ]), + }, + // Approval mode is the launch difference standing in for the options + // that gate omp's commands: the second thread's omp advertises a + // different set for the very same workspace. + source: + ` + if (process.argv.includes("always-ask")) { + process.env.T3_ACP_REQUEST_LOG_PATH = process.env.T3_OTHER_REQUEST_LOG_PATH; + process.env.T3_ACP_AVAILABLE_COMMANDS = JSON.stringify([ + { name: "compact", description: "Compact the conversation" }, + ]); + } + ` + + execScriptSource({ + scriptPath: NodeURL.fileURLToPath( + new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), + ), + }), + }), + ); + const instance = yield* OhMyPiDriver.create({ + instanceId, + displayName: undefined, + enabled: true, + environment: [], + config: { ...OhMyPiDriver.defaultConfig(), binaryPath }, + }); + yield* instance.adapter.startSession({ + threadId, + cwd: directory, + runtimeMode: "full-access", + }); + yield* instance.adapter.startSession({ + threadId: otherThreadId, + cwd: directory, + runtimeMode: "approval-required", + }); + // Returns only once the second thread's list has reached the workspace + // snapshot, so the shared entry now disagrees with the first thread. + yield* instance.adapter.sendTurn({ + threadId: otherThreadId, + input: "hello", + attachments: [], + }); + yield* instance.adapter.sendTurn({ threadId, input: "/computer status", attachments: [] }); + const prompts = (yield* fs.readFileString(logPath)) + .split("\n") + .filter((line) => line.includes('"method":"session/prompt"')); + expect(prompts).toHaveLength(1); + expect(prompts[0]).toContain('"text":"/computer status"'); + // A builtin travels alone; omp folds a second block into its arguments. + expect(prompts[0]).not.toContain("runtime_info"); + yield* instance.adapter.stopAll(); + }).pipe(Effect.scoped), + ); + for (const sendDuringPreparation of [false, true]) { it.effect(`steers within one turn (send during preparation: ${sendDuringPreparation})`, () => Effect.gen(function* () { diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.ts index 84b32d8ad6ef..06095a6ffca0 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.ts @@ -252,7 +252,6 @@ export const OhMyPiDriver: ProviderDriver = { environment: processEnv, ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), onAvailableCommands: (commands, cwd) => recordWorkspaceCommands("live", cwd, commands), - workspaceCatalog: findWorkspace, }); const unsupported = (operation: string) => Effect.fail( diff --git a/apps/server/src/provider/Layers/OhMyPiAdapter.ts b/apps/server/src/provider/Layers/OhMyPiAdapter.ts index a3f304b6509a..fde529632d36 100644 --- a/apps/server/src/provider/Layers/OhMyPiAdapter.ts +++ b/apps/server/src/provider/Layers/OhMyPiAdapter.ts @@ -18,7 +18,6 @@ import { RuntimeTaskId, RuntimeRequestId, type RuntimeMode, - type ServerProviderWorkspaceSnapshot, type ThreadId, TurnId, } from "@t3tools/contracts"; @@ -53,7 +52,12 @@ import { resolveOhMyPiSessionToggles, } from "../OhMyPiSessionOptions.ts"; import { buildRuntimeInstructions } from "../RuntimeInstructions.ts"; -import { ohMyPiWorkspaceCatalog, prepareOhMyPiPrompt } from "../Drivers/OhMyPiSkillDispatch.ts"; +import { + type OhMyPiWorkspaceCatalog, + ohMyPiWorkspaceCatalog, + prepareOhMyPiPrompt, + splitOhMyPiAvailableCommands, +} from "../Drivers/OhMyPiSkillDispatch.ts"; import * as McpProviderSession from "../../mcp/McpProviderSession.ts"; import { ProviderAdapterProcessError, @@ -92,6 +96,8 @@ const OH_MY_PI_RESUME_VERSION = 1 as const; * whatever the workspace already knew. */ const OH_MY_PI_COMMANDS_WAIT = Duration.seconds(5); +/** What a session that never reported dispatches from: nothing. */ +const OH_MY_PI_EMPTY_CATALOG = ohMyPiWorkspaceCatalog(undefined); function encodeJsonStringForDiagnostics(input: unknown): string | undefined { const result = encodeUnknownJsonStringExit(input); return Exit.isSuccess(result) ? result.value : undefined; @@ -110,10 +116,6 @@ export interface OhMyPiAdapterLiveOptions { commands: ReadonlyArray, cwd: string, ) => Effect.Effect; - /** The workspace's known commands and skills, from the probe or a session. */ - readonly workspaceCatalog?: ( - cwd: string, - ) => Effect.Effect; } interface PendingApproval { @@ -341,6 +343,12 @@ interface OhMyPiSessionContext { stopped: boolean; /** Settled once this session reported its own command list. */ readonly commandsReported: Deferred.Deferred; + /** + * What this session's own omp advertised. Launch options gate commands, so + * two threads in one workspace can differ; dispatch from the session that + * will run the prompt, never from the workspace's shared snapshot. + */ + catalog: OhMyPiWorkspaceCatalog | undefined; } function settlePendingApprovalsAsCancelled( @@ -1009,6 +1017,7 @@ export function makeOhMyPiAdapter( interruptionVersion: 0, stopped: false, commandsReported: yield* Deferred.make(), + catalog: undefined, }; for (const observed of pendingObservedToolCalls.splice(0)) { yield* emitOhMyPiChildTaskEvents(ctx, observed); @@ -1024,6 +1033,9 @@ export function makeOhMyPiAdapter( case "ConfigOptionsUpdated": return; case "AvailableCommandsUpdated": + ctx.catalog = ohMyPiWorkspaceCatalog( + splitOhMyPiAvailableCommands(event.availableCommands), + ); yield* ( options?.onAvailableCommands?.(event.availableCommands, cwd) ?? Effect.void ); @@ -1240,10 +1252,10 @@ export function makeOhMyPiAdapter( } const promptParts: Array = []; - // The first prompt waits for this session's own command list, so a - // catalog cached from a probe or an earlier session cannot go stale - // across a restart. A process that never reports settles the wait on - // its first timeout, so later prompts do not pay it again. + // The first prompt waits for this session's own command list, since + // dispatch reads it and omp reports it shortly after setup. A + // process that never reports settles the wait on its first timeout, + // so later prompts do not pay it again and dispatch nothing. yield* Deferred.await(ctx.commandsReported).pipe( Effect.timeoutOption(OH_MY_PI_COMMANDS_WAIT), Effect.flatMap((reported) => @@ -1252,14 +1264,9 @@ export function makeOhMyPiAdapter( : Deferred.succeed(ctx.commandsReported, undefined), ), ); - const workspaceCwd = ctx.session.cwd; const prompt = prepareOhMyPiPrompt( input.input?.trim() ?? "", - ohMyPiWorkspaceCatalog( - workspaceCwd === undefined - ? undefined - : yield* options?.workspaceCatalog?.(workspaceCwd) ?? Effect.succeed(undefined), - ), + ctx.catalog ?? OH_MY_PI_EMPTY_CATALOG, ); if (prompt.text) { promptParts.push({ type: "text", text: prompt.text }); From 9f851c1c7e1a2c520b08936cdd55a8653390106b Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Tue, 22 Sep 2026 18:00:53 +0000 Subject: [PATCH 18/18] docs(server): correct why OhMyPi catalogs are session-scoped Measured against omp 18.2.7: the advertised command list is identical with computer use on and off, so the comments claiming T3's launch options gate commands were wrong. The reason the dispatch catalog must follow the session and not the workspace cache is that the cache is keyed by cwd and written by whichever probe or session reported last, which a skill or plugin change on disk can leave disagreeing with the process about to receive the prompt. Co-Authored-By: Claude Opus 5 --- .../server/src/provider/Drivers/OhMyPiDriver.test.ts | 12 ++++++------ apps/server/src/provider/Drivers/OhMyPiDriver.ts | 9 ++++----- apps/server/src/provider/Layers/OhMyPiAdapter.ts | 11 ++++++----- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts index 425ae36a8681..d59b1fd17d9f 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.test.ts @@ -202,9 +202,9 @@ it.layer(testLayer)("OhMyPi driver", (it) => { { name: "computer", description: "Drive the screen" }, ]), }, - // The probe is the launch carrying `--session-dir`. It reports a - // narrower list, late, standing in for the commands omp gates behind - // options the probe cannot know. + // The probe is the launch carrying `--session-dir`. It reports late + // and disagrees, standing in for a read that the live session's own, + // later one should win over. source: ` if (process.argv.includes("--session-dir")) { @@ -266,9 +266,9 @@ it.layer(testLayer)("OhMyPi driver", (it) => { { name: "computer", description: "Drive the screen" }, ]), }, - // Approval mode is the launch difference standing in for the options - // that gate omp's commands: the second thread's omp advertises a - // different set for the very same workspace. + // Approval mode stands in for anything that makes two omp processes + // in one workspace disagree — a skill added or a plugin installed + // between the two session starts: the second advertises another set. source: ` if (process.argv.includes("always-ask")) { diff --git a/apps/server/src/provider/Drivers/OhMyPiDriver.ts b/apps/server/src/provider/Drivers/OhMyPiDriver.ts index 06095a6ffca0..83e07d1c3c4d 100644 --- a/apps/server/src/provider/Drivers/OhMyPiDriver.ts +++ b/apps/server/src/provider/Drivers/OhMyPiDriver.ts @@ -139,11 +139,10 @@ export const OhMyPiDriver: ProviderDriver = { SubscriptionRef.update(metadata, (draft) => { const recorded = draft.workspaceSnapshots ?? []; // A probe only runs for a workspace with no entry, so an entry that - // appeared since came from a live session. omp advertises what its own - // configuration enables and the probe launches without a thread's - // options, so letting the slower probe land would narrow the menu. - // Reading the entry rather than remembering the cwd keeps this in step - // with eviction: a workspace that ages out can be probed again. + // appeared since came from a live session: the same list, read later, + // by the process the user is talking to. Reading the entry rather than + // remembering the cwd keeps this in step with eviction, so a workspace + // that ages out can be probed again. if (source === "probe" && recorded.some((workspace) => workspace.cwd === cwd)) { return draft; } diff --git a/apps/server/src/provider/Layers/OhMyPiAdapter.ts b/apps/server/src/provider/Layers/OhMyPiAdapter.ts index fde529632d36..976fb3079e3d 100644 --- a/apps/server/src/provider/Layers/OhMyPiAdapter.ts +++ b/apps/server/src/provider/Layers/OhMyPiAdapter.ts @@ -92,8 +92,8 @@ const PROVIDER = ProviderDriverKind.make("ohMyPi"); const OH_MY_PI_RESUME_VERSION = 1 as const; /** * omp reports its command list about fifty milliseconds after session setup, - * on new and resumed sessions alike; past this bound a prompt goes out with - * whatever the workspace already knew. + * on new and resumed sessions alike; past this bound a prompt goes out + * dispatching nothing, reaching the model as the text the user typed. */ const OH_MY_PI_COMMANDS_WAIT = Duration.seconds(5); /** What a session that never reported dispatches from: nothing. */ @@ -344,9 +344,10 @@ interface OhMyPiSessionContext { /** Settled once this session reported its own command list. */ readonly commandsReported: Deferred.Deferred; /** - * What this session's own omp advertised. Launch options gate commands, so - * two threads in one workspace can differ; dispatch from the session that - * will run the prompt, never from the workspace's shared snapshot. + * What this session's own omp advertised. The workspace snapshot is a menu + * cache keyed by cwd, written by whichever probe or session reported last; a + * skill added or a plugin installed since then makes it disagree. Dispatch + * from the process that will actually receive the prompt. */ catalog: OhMyPiWorkspaceCatalog | undefined; }