Skip to content

Native chat: answer 'can this agent do X' from each agent's registration - #27078

Closed
brennanb2025 wants to merge 3 commits into
mainfrom
brennanb2025/native-chat-capability-record
Closed

brennanb2025 wants to merge 3 commits into
mainfrom
brennanb2025/native-chat-capability-record

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 12 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​261 $\color{#cf222e}{\Huge{\mathbf{−}}}$​29 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​232
Prod 25 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​417 $\color{#cf222e}{\Huge{\mathbf{−}}}$​174 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​243

ELI5

Orca's native chat runs several agents: Claude, Codex, OpenCode, Grok, OMP and Pi. A few features ask "can this agent do X?":

  • bring an existing terminal conversation into a chat,
  • show a chat's conversation as that chat in Session History, and refuse a terminal resume that would write into it at the same time,
  • offer a chat tab's Session History actions.

Today the answer is a hard-coded "only if it's Claude or Codex", repeated in about ten places. Every time a newer agent was added, one of those places was missed. This PR makes each agent's own registration answer those questions, in one place. Nothing changes for users yet.

What Changed

User-facing: nothing. Only Claude and Codex answer "yes", exactly as before. This is a refactor so that a follow-up can turn these features on for OpenCode, Grok, OMP and Pi by filling in their registrations, without touching shared code.

Mechanism:

  • Two new required fields on each agent's registration. Like modelCatalog, every agent has to state an answer, so a new agent can't silently be left out:
    • transcriptImport: how to bring an outside CLI conversation into a chat. Null when the agent can't.
    • sessionHistory: which Session History row types the agent's chats own, how a stored conversation maps to a row id, and how to read that agent's terminal resume command. Null when the agent's chats don't own rows.
  • Claude and Codex logic moved, not rewritten. Their existing code moved into claude-cli-conversations.ts and codex-cli-conversations.ts. OpenCode, Grok, OMP and Pi write null.
  • One ownership index. The duplicate-import check and the Session History ownership check now read the same index. Rows are keyed by the registration's row-id mapping, which is required, not defaulted. That matters later: Pi stores a file path where the history row uses an id, and two separate indexes would disagree.
  • Agent binary names come from the existing terminal agent config. The resume-command guard looks them up there instead of a hard-coded codex|claude pattern.
  • Clients learn the answers from the host's agent list. The host publishes two derived flags, transcriptAdoption and sessionHistory, in its existing agent list. They are computed from the registration fields, not stored a second time. The screen code reads them through the same helper image support already uses.
  • "Resume in New CLI" on a chat-owned row is now offered when the agent has a fork command (getAgentForkArgv). Today that is Claude and Codex.

Hard-coded Claude/Codex checks that remain, and why:

  • Old-host fallbacks (a host without the agent list runs only those two).
  • The seed model catalog (only those two ship seed data).
  • Saved Arguments.
  • Protocol-specific code.
  • Features that aren't native chat (CLI titles, accounts, usage, automations).

Why

Three bugs of the same kind came from these scattered checks:

  • Orchestration workers were Claude/Codex-only.
  • Floating-workspace chats of the newer agents lost their folder.
  • Session History still shows OpenCode chats as plain terminal rows, and "resume" opens a second writer to the same conversation.

Moving each answer onto the agent's registration removes that class of bug: shared code no longer branches on agent names, and a new agent must state each answer.

Alternatives considered:

  • Adding the newer agents to each check one by one. That repeats the pattern that caused the bugs.
  • Declaring the flags on each agent definition. That makes a second copy that can disagree with the registration, so the flags are derived instead.

Mixed versions:

  • A newer client talking to an older host treats a missing flag as "Claude or Codex", which is the old build's answer, never "no".
  • An older client ignores the new fields; its decoder is not strict.

Linked Issue

N/A (maintainer refactor; follow-up to the floating-workspace links fix, #26981, and the workers fix, #26776).

Visual Proof

N/A: no user-visible change; this is a strict-parity refactor.

Testing

  • Parity: every moved check gives the same answer as main.
    • A reviewer fuzzed the old and new resume-command parsers with 400,000 random commands (paths, .exe, capitals, quoting, --resume=, -c, fork flags, --, several binaries in one command): 0 differences.
    • Each version mix was traced: old host, new host, no agent list, an agent missing from the list, SSH.
  • New tests:
    • The decoder fills missing flags for old hosts.
    • The RPC publishes the flags for Claude and Codex only.
    • OpenCode rows and resume commands are left alone.
    • A create that resumes a conversation for an agent without an importer is refused before any import work.
    • Resume-in-chat and the tab history menu follow the host flag.
    • An owned OpenCode, Grok or Pi row offers no CLI fork.
  • Checks: targeted vitest (22 files), pnpm tc:node, pnpm tc:web, the code-quality gate, oxlint, and an import-cycle check with no depth limit on all touched files.
  • I manually tested these changes locally (refactor with no visible change; covered by tests)
  • Automated tests added/updated

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Cross-platform: binary matching keeps main's rules: last path segment, any case, optional .exe.
  • Remote/SSH: an SSH host publishes no agent list, so the Claude/Codex fallback applies, as before.
  • Mobile: doesn't read these fields.
  • Performance: each row does one lookup over six registrations.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…he agent registration

Claude and Codex's transcript importers and Session History ownership (row id,
resume-command grammar) move onto their runtime registrations as required-nullable
fields; every other agent writes null. agentSession.agents publishes the derived
transcriptAdoption/sessionHistory flags, and a client reads a host that omits them
(or lists nothing) as the old build's answer. Strict parity: only Claude and Codex
answer yes anywhere.
…w agents, fork offer from fork argv

Review round 1: the duplicate-adoption check and Session History ownership read one
index keyed by the registration's row id (an importer now requires sessionHistory);
resume-command binaries derive from rowAgents through the TUI agent config; the
Resume in New CLI offer asks getAgentForkArgv instead of naming Claude and Codex.
Strict parity: only Claude and Codex answer yes.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: ready for your review (draft, not merged)

The problem. Features like "bring a terminal conversation into a chat", "show a chat's conversation as that chat in Session History, and refuse a terminal resume that would write into it at the same time", and "chat tab history actions" decided which agents qualify with a hard-coded "Claude or Codex" check, repeated in about ten places. Newer agents (OpenCode, Grok, OMP, Pi) kept missing features because one of those places wasn't updated. The visible case today: an OpenCode chat's conversation shows in Session History as a plain terminal row, and "resume" starts a second writer on it.

What this PR changes for users. Nothing. Only Claude and Codex answer "yes", exactly as before. Each agent's registration now answers these questions (two new required fields), and clients read the answer from the host's existing agent list. The follow-up #27183 turns the features on for the other four agents by filling those fields in.

Fixed during review:

  • Round 1 (architecture review):
    • The duplicate-import check and Session History ownership each had their own ID mapping. That would silently disagree for Pi, whose chat stores a file path while Session History uses an ID. Both now read one index, and the mapping is required.
    • Agent program names now come from the existing terminal-agent config instead of one field per agent, so OpenCode's two programs (opencode, opencode2) fit.
    • The "Resume in New CLI" offer on a chat-owned row now comes from whether the agent has a fork command, not a name check.
  • Round 2 found nothing.

Deferred (nits):

  • No test pins that an importer requires Session History ownership. The type enforces it.
  • A Codex-only setting sits in the shared importer settings type.
  • One test loads every registration.

Verified:

  • Parity checks:
    • Two reviewers compared the old and new resume-command parser on 400,000 and 500,000 random commands: 0 differences.
    • All four version combinations were traced: old host with new client, new host with old client, a host with no agent list, and SSH.
    • Old clients ignore the new fields, which was checked with the shipped decoder.
  • Readiness checklist: no P0, P1 or P2.
  • Local checks: pnpm tc:node, pnpm tc:web, all 24 orca-ci-checks, and the targeted tests pass on the tree with current main merged (1a6de78).
  • CI at 1a6de78 (with main merged): all tests and static analysis pass. Only the Windows packaging job (and the summary verify job that depends on it) is red. Its failing test differed between the two runs: an NSIS PowerShell probe timeout at 567aded, then a ConPTY job-ownership test at 1a6de78. Neither touches anything this PR changes.
  • Live QA: run on Session History shows OpenCode, Grok, OMP and Pi chats as chats and refuses a second terminal writer #27183's branch, which contains this PR, on a hidden macOS build with real OpenCode. The Session History rows and refusals behaved as intended.

Not verified:

  • Live Claude/Codex runs (covered by unit tests and the fuzz comparison).
  • Windows and Linux live.
  • Mobile live. Mobile doesn't read these fields.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 12, 2026 01:12
@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e75daefd-003b-4bd5-8de1-41d0033ee312

📥 Commits

Reviewing files that changed from the base of the PR and between 3c71318 and 1a6de78.


📒 Files selected for processing (37)
  • src/main/ai-vault/structured-session-ownership.test.ts
  • src/main/ai-vault/structured-session-ownership.ts
  • src/main/claude/claude-cli-conversations.ts
  • src/main/codex/codex-cli-conversations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-definition.ts
  • src/main/native-chat/agent-session-wire/structured-agent-registry.ts
  • src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts
  • src/main/native-chat/agent-session-wire/structured-provider-session-ownership.ts
  • src/main/native-chat/structured-agent-cli-conversations.ts
  • src/main/native-chat/structured-agent-session-history-adoption.test.ts
  • src/main/native-chat/structured-agent-session-history-adoption.ts
  • src/main/pi/rpc-runtime-registration.ts
  • src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts
  • src/main/runtime/orca-runtime-structured-agent-session-create-intent.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-agents.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-agents.ts
  • src/main/runtime/structured-agent-runtime-registrations.ts
  • src/main/runtime/structured-agent-session-create-adoption.ts
  • src/main/runtime/structured-agent-session-history-ownership.ts
  • src/renderer/src/components/native-chat/structured-agent-registered-agent-surface.test.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-cli-fork.test.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-cli-fork.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-launch-actions.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat-launch.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat-workspace.test.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat-workspace.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat.test.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-surface-switch.test.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-surface-switch.ts
  • src/renderer/src/components/tab-bar/tab-session-history-switch.test.ts
  • src/renderer/src/components/tab-bar/tab-session-history-switch.ts
  • src/renderer/src/runtime/use-host-structured-agent.ts
  • src/shared/agent-session-capabilities.ts
  • src/shared/agent-session-registered-agents.test.ts
  • src/shared/agent-session-registered-agents.ts
  • src/shared/structured-native-chat-launch-route.ts

💤 Files with no reviewable changes (1)
  • src/main/native-chat/structured-agent-session-history-adoption.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.



📝 Walkthrough

Walkthrough

Claude and Codex CLI registrations now define transcript import and session history behavior. Runtime ownership and transcript adoption use registered agent configurations. The runtime reports transcript-adoption and session-history capabilities, which renderer actions use to determine eligibility. Claude resume commands are parsed by executable name, and CLI fork actions check session ownership and agent fork support.


Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 1a6de

The reviewed capability changes preserve the investigated resume and host-list behavior. No issue identified here prevents merging after normal checks.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 47.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 36 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: capability answers now come from each agent's registration.
Description check Passed The description follows the repository template and explains the user impact, mechanism, rationale, testing, compatibility considerations, and checklist status. It also documents that visual proof is …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Folded into #27183 in slimmer form and closing this one. The registration fields that answer "which history rows does this agent's chat own" and "can this agent's CLI transcripts be adopted" now live there. The two capability flags this PR sent from host to client, and the client code that read them, are dropped: each flag's only reader could already answer from what it had, and a new field would need reconciling between old and new clients and hosts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant