You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds two ADRs and a CONTEXT.md glossary covering the OhMyPi integration design:
docs/adr/0005-ohmypi-session-toggles-as-provider-options.md: OhMyPi's session toggles (computer use, advisor, prewalk) are modeled as provider options, applied at process launch via CLI flags and a small config overlay rather than slash commands.
docs/adr/0006-ohmypi-skills-and-commands-from-acp-probe.md: OhMyPi skills and commands are discovered through an ephemeral ACP probe session instead of replicating omp's on-disk skill scan.
CONTEXT.md: glossary entries for provider option, provider setting, and skill to keep the vocabulary consistent across the codebase.
Why
OhMyPi's session toggles never persist to its session file, so a toggle set once by slash commands silently decays whenever T3 Code stops and resumes the session. Persisting them as provider options and re-applying them at every launch makes the desired state exactly what the process was told, while reusing existing traits picker, defaults, and restart-with-resume machinery.
OhMyPi has no CLI to list skills and its own discovery logic is too complex to mirror safely. A throwaway ACP probe reads omp's own available_commands_update view, which stays accurate for both the $ and / menus without drift, and the session directory override keeps probes out of the user's omp resume list.
- 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
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟢 Approval recommended
The changes are documentation-only, consistent with existing ADR formatting, and introduce no behavioral or interface changes.
Review effort: Lite Findings: None
What changed in this PR
Adds architectural documentation for the OhMyPi provider integration, capturing two key design decisions (session toggles as provider options and ACP-based skill discovery) and introducing a small glossary to keep terminology consistent.
Changes:
Added ADR 0005 documenting modeling OhMyPi session toggles as persisted provider options applied at process launch.
Added ADR 0006 documenting discovering OhMyPi skills/commands via an ephemeral ACP probe session.
Added CONTEXT.md glossary entries to standardize “provider option”, “provider setting”, and “skill” terminology.
- Probe workspace commands without a session so skills/slash commands appear before the first turn
- Split omp's skill:<name> 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
Fryuni
changed the title
docs: record OhMyPi provider option and skill discovery decisions
feat: add support for OhMyPi skills and extra modes
Sep 22, 2026
- 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 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The mock ACP agent publishes T3_ACP_AVAILABLE_COMMANDS via a raw JSON.parse without error handling, which can crash the script outside Effect control flow if the env var is malformed.
Handle malformed T3_ACP_AVAILABLE_COMMANDS JSON without crashing
apps/server/scripts/acp-mock-agent.ts:450
T3_ACP_AVAILABLE_COMMANDS is parsed with JSON.parse without validation; if the env var is malformed, this throws outside the Effect error channel and crashes the mock agent in a hard-to-diagnose way. Wrap the parse in Effect.try (or pre-parse with a try/catch) so failures surface with a clear error message and stay in Effect control flow.
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
The comment claims the overlay filename prevents sharing for “different toggles”, but the overlay only varies with advisor and computerUse (prewalk is not represented in the overlay). This is a small documentation mismatch that could mislead future changes (e.g. assuming prewalk state is encoded in the overlay filename).
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The new client helper for opening skill-instruction paths should normalize Windows separators to match the existing “paths use '/' even on Windows” convention, and the current test locks in the inconsistent behavior.
The Windows-path expectation should match the normalized /-separator form returned by resolveProviderSkillInstructionsPath (and used elsewhere in the client).
Normalize Windows instruction paths for consistent client handling
resolveProviderSkillInstructionsPath returns Windows paths with backslashes unchanged, but the rest of the client treats workspace/file paths as /-separated even on Windows (e.g. RightPanel openFile comment and path handling). This can cause the “View instructions” action to open a path the UI/preview layer won’t resolve consistently.
Probe and live-session updates share this write path without recording their source. refreshWorkspaceSnapshot starts the probe immediately after startSession returns, so a delayed probe can finish after the session's available_commands_update and overwrite the live catalog with an older snapshot, despite the comment that live data replaces probe data. Track the source/generation or suppress a probe commit once a live update has arrived.
Both the live-session callback and the probe call this same updater. snapshotForCwd can launch a probe while a newly started live session is still waiting to report its commands, so the probe may finish afterward and overwrite the live catalog for this cwd; whichever process finishes last wins. Keep probe and live sources separate or make the update source-aware so a live notification always wins.
URI schemes are not limited to two characters. This regex lets a one-character scheme such as x://skill through, so openMention later treats it as a workspace file path; packages/shared/src/composerInlineTokens.ts:46-69 already handles this by matching one-character schemes and exempting Windows drive paths. Reuse that rule here so all scheme identifiers are withheld.
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 <noreply@anthropic.com>
refreshWorkspaceSnapshot starts probeWorkspace detached before startSession, and both the probe and live adapter call this same recordWorkspaceCommands. If the live session reports its commands first, a slower probe can complete afterward and overwrite the live catalog, despite the comment and ADR that live updates replace probed data. Track the source/generation or reject probe writes once a live update for the cwd has landed.
Blocked dispatch still rewrites known skill mentions
This rewrites every known $mention before applying omp's opening-prefix rules. For example, /tmp/x is broken, $grill-me is not dispatchable because the prompt starts with /, but it is still sent as /tmp/x is broken, /skill:grill-me with consumedByCommand: false, changing the user's prose. Guard the rewrite with the same prefix check (while allowing a known leading skill), or leave the body unchanged whenever dispatch is blocked.
One-character URI schemes are misclassified as filesystem paths
URI schemes may legally be one character long (x:...), but this + quantifier requires at least two characters. That makes x://... look like a filesystem path and exposes it via View instructions, contrary to the shared composer parser's one-character scheme rule. Use a * scheme regex plus an explicit Windows drive-path exception.
This URI check requires at least two characters before :, but URI schemes may be one character. As a result, a provider-reported path such as x:resource is treated as a filesystem path and offered to openMention, unlike the shared parser's any-scheme rule (packages/shared/src/composerInlineTokens.ts:46-69). Use the shared scheme quantifier and explicitly exempt Windows drive paths.
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
The reactor compares raw selections, but resolveOhMyPiSessionToggles deliberately normalizes both an absent value and false to the same launch state. Consequently, selecting Off for a previously unset OhMyPi toggle changes undefined to false here and restarts/resumes the session even though the generated flags and overlay are identical. Compare effective provider values or carry per-option defaults into the restart capability; raw undefined/false comparison is only correct for providers where omission has different semantics.
CWD-only catalogs overwrite commands across concurrent sessions
Live catalogs are keyed only by cwd, so the last OhMyPi session to report wins even when multiple threads share a workspace with different advisor/computer/prewalk launch options. A session with fewer enabled commands can therefore overwrite a richer catalog, causing the other thread's menu and $skill dispatch to treat valid commands as unknown. Track catalogs per session/thread or merge/select the catalog for the active session instead of replacing the cwd entry globally.
Scheme validation rejects valid one-character URI schemes
This scheme check requires at least two characters before :, so a valid one-character URI such as a://skills/x is treated as a filesystem path and can be passed to openMention. The shared file-link parser uses the RFC scheme form with * plus an explicit Windows-drive exception (packages/shared/src/composerInlineTokens.ts:46-69); apply the same rule here so external skill identifiers are never offered as files.
Glossary incorrectly implies skills are discovered on disk
CONTEXT.md:21
This glossary defines every skill as being discovered on disk, but the OhMyPi design added here deliberately discovers skills through ACP and represents them with skill:// identifiers. Keeping this wording makes the new integration contradict its own ADR and can lead future contributors to reintroduce an on-disk scan; describe skills as provider-exposed instruction bundles and leave discovery mechanism/provider invocation unspecified.
The scheme check requires at least two characters before :, but URI schemes may legally be one character (for example, x://...). A provider-reported one-letter URI would therefore be returned as a filesystem path and offered to open, contrary to this helper's purpose. Detect schemes with * and explicitly allow Windows drive paths (^[a-z]:([\\/\\]|$)) instead.
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Fryuni
changed the title
What Changed
Adds two ADRs and a
CONTEXT.mdglossary covering the OhMyPi integration design:docs/adr/0005-ohmypi-session-toggles-as-provider-options.md: OhMyPi's session toggles (computer use, advisor, prewalk) are modeled as provider options, applied at process launch via CLI flags and a small config overlay rather than slash commands.docs/adr/0006-ohmypi-skills-and-commands-from-acp-probe.md: OhMyPi skills and commands are discovered through an ephemeral ACP probe session instead of replicating omp's on-disk skill scan.CONTEXT.md: glossary entries for provider option, provider setting, and skill to keep the vocabulary consistent across the codebase.Why
OhMyPi's session toggles never persist to its session file, so a toggle set once by slash commands silently decays whenever T3 Code stops and resumes the session. Persisting them as provider options and re-applying them at every launch makes the desired state exactly what the process was told, while reusing existing traits picker, defaults, and restart-with-resume machinery.
OhMyPi has no CLI to list skills and its own discovery logic is too complex to mirror safely. A throwaway ACP probe reads omp's own
available_commands_updateview, which stays accurate for both the$and/menus without drift, and the session directory override keeps probes out of the user's omp resume list.Checklist