Repository navigation
feat(mcp): retire legacy server surfaces - #2820
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75b353a7e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isError: board.exit !== 0, | ||
| payload: parseStandaloneJson(board), | ||
| commands: [task, board], |
There was a problem hiding this comment.
When task list --json exits non-zero, emits malformed JSON, or returns the wrong tasks while board --json remains healthy, this evidence still sets isError solely from the board result and observedRepo validates only the board payload. The validator merely checks that both argv arrays are present and that some command has exit 0, so the release dogfood gate can approve a candidate with a broken standalone task surface; require the task observation to exit successfully, parse correctly, and contain the seeded sentinel too.
AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.
| const content = removeOwnedFallback(raw, owned); | ||
| assertNonGenieTomlSemantics(raw, content, configPath); | ||
| if (content === raw) { | ||
| return { path: configPath, action: 'skipped', ok: true, route: 'fallback', detail: 'project fallback current' }; | ||
| } | ||
| return { | ||
| path: configPath, | ||
| action: exists ? 'updated' : 'created', | ||
| ok: true, | ||
| route: 'fallback', | ||
| detail: 'project fallback active', | ||
| content, | ||
| }; | ||
| } | ||
|
|
||
| /** Merge only the marker-owned Codex fallback. Kept exported for migration callers/tests. */ | ||
| export function mergeCodexMcpFallback(configPath: string, entry: McpServerEntry): ArtifactAction { | ||
| const prepared = prepareCodexFallback(configPath, entry, true); | ||
| if ('content' in prepared && prepared.content !== undefined) applyPreparedWrite(prepared); | ||
| return prepared.action; | ||
| } | ||
|
|
||
| /** Remove only the marker-owned Codex fallback. */ | ||
| export function removeCodexMcpFallback(configPath: string): ArtifactAction { | ||
| const prepared = prepareCodexFallback(configPath, genieMcpEntry(), false); | ||
| if (prepared.route === 'conflict') return 'skipped'; | ||
| if ('content' in prepared && prepared.content !== undefined) applyPreparedWrite(prepared); | ||
| return prepared.action; | ||
| applyPreparedWrite({ path: configPath, action: 'updated', content }); |
There was a problem hiding this comment.
On the first genie init or successful genie setup --codex after upgrading, an existing marker-owned route reaches this write and replaces .codex/config.toml without creating any backup. This is the new migration path for mixed user/Genie config, so users have no preserved pre-migration bytes for rollback or recovery; create the backup before applying the retirement write and cover it in the migration fixture.
AGENTS.md reference: AGENTS.md:L31-L32
Useful? React with 👍 / 👎.
| const retired = route.route !== 'none' && route.route !== 'plugin'; | ||
| const status: CheckStatus = retired || hard.length > 0 || trust.length > 0 ? 'warn' : 'pass'; | ||
| const findingText = findings.map((finding) => `${finding.kind}: ${finding.detail}`).join('; '); | ||
| return { | ||
| name: 'Codex Genie MCP registration', | ||
| status, | ||
| detail: `${route.route}: ${route.detail ?? 'no detail'}${findingText.length > 0 ? `; ${findingText}` : ''}`, | ||
| detail: `${retired ? 'retired route remains preserved' : 'retired routes absent'}${findingText.length > 0 ? `; ${findingText}` : ''}`, |
There was a problem hiding this comment.
When .codex/config.toml is malformed, unsafe, or has incomplete/duplicate Genie markers, inspectCodexProjectMcp returns ok: false with route: 'none'; this condition now ignores route.ok and discards its diagnostic. In a trusted project with no other layer findings, genie doctor therefore reports pass and “retired routes absent” even though genie init cannot inspect or retire the config; keep the inspection failure as a failing or warning status and surface route.detail.
Useful? React with 👍 / 👎.
There is no separate Genie UI any more — the Orca integration is the only UI surface — so the bridge built for it is retired on exactly the terms `genie mcp` was (#2820): `genie ui-bridge` is a stub that writes a stable diagnostic to stderr and exits 1, and everything that existed only behind it is deleted. Removed (4,088 LOC): - src/lib/v5/mcp-server.ts + test (shared newline-JSON-RPC transport loop) - src/lib/v5/mcp-tools.ts + test (tool registry + readonly/degraded/write opens) - src/lib/v5/bridge-watcher.ts + test (change watcher + ppid backstop) - src/lib/v5/UI-BRIDGE.md (the bridge protocol contract) - task-state.ts `listWishSlugs` / `WishGroupRow` — consumed only by the retired board tool; the hook bundle keeps its own `listKnownSlugs` Kept: sqlite-open.ts's `openWithWalIndexRecovery` and the hire-roster accessors. Both now have no shipped caller, but the recovery helper is the opt-in seam for any future single-owner writer (and is tested directly), and hire rows remain part of exported/imported board state. Their comments now say so instead of pointing at deleted modules. `ui-bridge` stays in the interactivity workspace-gate bypass set for the same reason `mcp` does: the legacy gate must not exit 2 and mask the retirement diagnostic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gGxGgKskzyzr1HRUB6cV3
* feat(ui-bridge): retire the UI-owned stdio bridge There is no separate Genie UI any more — the Orca integration is the only UI surface — so the bridge built for it is retired on exactly the terms `genie mcp` was (#2820): `genie ui-bridge` is a stub that writes a stable diagnostic to stderr and exits 1, and everything that existed only behind it is deleted. Removed (4,088 LOC): - src/lib/v5/mcp-server.ts + test (shared newline-JSON-RPC transport loop) - src/lib/v5/mcp-tools.ts + test (tool registry + readonly/degraded/write opens) - src/lib/v5/bridge-watcher.ts + test (change watcher + ppid backstop) - src/lib/v5/UI-BRIDGE.md (the bridge protocol contract) - task-state.ts `listWishSlugs` / `WishGroupRow` — consumed only by the retired board tool; the hook bundle keeps its own `listKnownSlugs` Kept: sqlite-open.ts's `openWithWalIndexRecovery` and the hire-roster accessors. Both now have no shipped caller, but the recovery helper is the opt-in seam for any future single-owner writer (and is tested directly), and hire rows remain part of exported/imported board state. Their comments now say so instead of pointing at deleted modules. `ui-bridge` stays in the interactivity workspace-gate bypass set for the same reason `mcp` does: the legacy gate must not exit 2 and mask the retirement diagnostic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gGxGgKskzyzr1HRUB6cV3 * docs: keep the bounded .mcp.json retirement wording alongside the ui-bridge retirement Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gGxGgKskzyzr1HRUB6cV3 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Evidence