refactor(native-chat): keep the provider resume handle opaque to shared code - #24991
Draft
brennanb2025 wants to merge 8 commits into
Draft
brennanb2025 wants to merge 8 commits into
brennanb2025 wants to merge 8 commits into
Conversation
…ed code
Shared structured-chat code parsed each provider's resume handle: Claude's
session id and branch leaf, Codex's thread id, through a 'claude' | 'codex'
union every new agent had to widen. The in-memory handle is now
{ transport, agent, nativeId, providerData? }: shared readers use nativeId,
lease and handle-chain checks compare transport and agent, and only the
Claude adapter reads its leaf (providerData).
Stored and wire forms are unchanged for Claude and Codex. One encoding
module writes their typed shapes and decodes both those and the neutral
shape a new transport uses, which an older build refuses as unreadable
rather than reading as Codex. Key and root strings, which fork seeds,
superseded creations and resume offers persist, stay byte-identical.
The journal's own handle type becomes the journal-row and attach-wire
encoding of the same handle, and the journal identity carries the
neutral handle (null before the provider proves one).
No user-visible change.
…l identity The journal row converter now takes the identity every caller already holds, so a row's handle has one obvious constructor. Tests that wrote the in-memory handle straight into journal rows now build it through that converter, and the processless Claude fixture names a not-yet-proved handle as null.
A typed Claude or Codex handle that also carries the neutral form's transport, agent, native id or provider data named two identities; it was read as Claude or Codex and the next write dropped the other one. Such a row now stays unreadable and is set aside untouched.
This was referenced Oct 3, 2026
brennanb2025
added a commit
that referenced
this pull request
Oct 4, 2026
Journal-identity fixtures from the timeline branches named the pre-#24991 handle shape; they now use the opaque handle. The ACP acquire keeps its lane in a slot the closures write, and a renderer route test builds its partial store the way its siblings do.
This was referenced Oct 4, 2026
Open
…n suite Main grew the suite to the 800-line limit; the opaque-handle import pushed it over. The two tests built the same identity inline.
Rename the neutral provider handle's providerData to resumeCursor before any row persists the neutral form: it is an adapter-owned resume position (Claude's transcript leaf), never identity. Claude/Codex stored and wire bytes are unchanged; their typed shapes never carried the field. State the stored-form contract (a handle's field set is closed; later per-link data goes on the chain link, which every build preserves) and pin it with a record round-trip test. Document that transport records the id space the native id was minted in, which can differ from the agent's current transport.
Contributor
Author
Review summary (head
|
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ELI5
Every structured native chat keeps a bookmark that says which conversation inside the agent it continues (Claude calls it a session, Codex calls it a thread). Until now, Orca's shared code knew the exact layout of that bookmark for Claude and for Codex, so adding any new agent meant teaching a dozen shared files a third layout. This PR turns the bookmark into a generic id plus a sealed note that only the agent's own code opens. Nothing changes for anyone using the app today; it is groundwork so the next agents (starting with Grok over the Agent Client Protocol) plug in without touching shared code.
What Changed
The problem. The "provider handle" (the durable pointer from an Orca chat to the provider's own conversation) was a typed union in shared code:
{ provider: 'claude', sessionId, leafUuid } | { provider: 'codex', threadId }. Shared readers picked fields by provider: ownership lists, orchestration mail addressing, history results, the record validator, the lease and handle-chain checks, and the journal identity. Claude'sleafUuid(which transcript entry a resume continues from, a branch cursor only Claude has) was interpreted in shared runtime code. A new transport would have had to add a union arm and a branch at every one of those sites.User-facing change: none. Claude and Codex chats create, resume, fork, rewind and recover exactly as before, and everything stored on disk or sent over the wire for them is byte-for-byte the same.
The mechanism.
src/shared/agent-session-provider-handle.ts):{ transport, agent, nativeId, resumeCursor? }.transportis an open vocabulary for the protocol whose id spacenativeIdlives in ('claude-sdk','codex-app-server', later'acp').agentis the Orca agent whose binary resumes the conversation. One transport serves many agents, so the transport alone does not scope an id.nativeIdis the provider's own conversation id (Claude's session id, Codex's thread id). Shared code reads only this.resumeCursoris resume state only that provider's adapter reads (where a resume continues from), never identity. For Claude it is the leaf uuid; Codex has none.transportrecords the id space the native id was created in. If an agent later changes transport, its old chats must become not resumable on that build, never unreadable records.nativeIdonly: ownership listing, mail-address lookup, the history result's provider session id, the rewind target check, journal recovery, and adopted-transcript import.agentSessionProviderHandleBelongsTo) and never read the handle's data. The chain rules (created / adopted / resumed / forked, unsaved-creation supersession, fence ordering) stay on the link and are unchanged.reviseAgentSessionClaudeResumePointbecame the genericreviseAgentSessionProviderResumePoint, which takes a whole replacement handle and only refuses one that names another conversation. The Claude runtime adapter builds that handle from its leaf. Claude launch resolution and the Claude history window read the leaf throughclaudeProviderHandleLeafUuid.src/shared/agent-session-provider-handle-encoding.ts):transport,agent,nativeIdorresumeCursorfields names two identities, so it is refused rather than read as Claude/Codex (which would silently drop the other identity on the next write). The whole record is then set aside as unreadable and kept byte for byte. Unrelated extra fields on a typed handle are still tolerated on read (and dropped on the next write).provider === 'claude'/'codex'and refuse it, so it is set aside as unreadable (never rewritten, every mutation refused) instead of being read as Codex. The "unknown providers must not impersonate Codex" guard keeps its intent.claude:["sid","leaf"],claude:"sid",codex:"tid").src/shared/agent-session-record-stored-form.ts) wraps the existing lease normalization. Rows are decoded on load and encoded on every write: the store's row diff and the one-time records-file import.normalizeLegacyHandoffRecordis replaced bydecodePersistedAgentSessionRecord.AgentSessionProviderHandle({ kind: 'claude' | 'codex' | 'opaque', … }) is renamedAgentSessionJournalProviderHandleand documented as what it always was in practice: the encoding journal rows record and the attach wire carries. Every journal row builds it the same way, from the journal identity, through one function (agentSessionJournalProviderHandle(identity)): the identity's handle in row form, orpendingbefore one is proved. Tests build row handles through that same function instead of writing the in-memory handle into a row. The journal identity handed to adapters now carries the neutral handle, ornullbefore the provider has proved one (today this is the{ kind: 'opaque', value: 'pending' }placeholder, which journal rows still record byte-for-byte). The reservedopaquearm gets its intended use: a non-Claude/Codex handle is recorded in journal rows as{ kind: 'opaque', agent, value: nativeId }.Census of every place a handle is persisted or sent (checked before changing anything):
agent_session_records.record_json(chat journal database)providerHandleChain[].handleagent-sessions.jsonrecords file (read once by the version-4 import)forkedFromKey,supersedesKeyproviderHandleRoot(persisted across quit/relaunch)epochandsubmissionrows (providerHandle){ kind }handle; write-only, readers only check it is an objectopaquearmagentSession.attach/ensureparams (providerHandle, strict zodclaude/codex), host-internaladopt.providerHandle{ kind }handle; the attach fingerprint covers it as sentproviderSession.id, orchestration mail lookup by provider idnativeIdisAgentSessionHandleProvider/AgentSessionHandleProviderNothing an older client or host decodes changes shape, so no capability gate is needed.
Overlap with #24862. Both PRs touch
src/shared/agent-session-record.ts,structured-claude-runtime-adapter.ts,codex-structured-session-acquire.tsand seven test files or fixtures. A trial merge (git merge-tree) of this branch with #24862's head shows no conflict in any of them; the only conflicts in that trial come from main changes #24862 has not merged yet (i18n catalogs, and a test file it deletes that main has since edited). Its only new handle literal is an attach-wire one, which this PR keeps valid. Whichever lands second should re-run its tests.Why
The common pattern keeps the provider's resume handle generic in shared code: a native id that shared code reads, plus provider-owned data it never parses. Orca's typed union was the outlier, and it is why every new agent touched shared types.
Alternatives considered:
provider === 'codex'again. With the neutral type, the provider fields do not exist outside the encoding module.Differences from the common pattern
{ kind }shape for Claude and Codex (temporary). The wire follow-up (feat(native-chat): open structured chat's wire and stored records to registered agents, behind a negotiated capability #25159) widens what clients and hosts exchange behind a new advertised capability. Journal rows are write-only and need no change.record.provideris still'claude' | 'codex', and the account-home variable is still a closed pair (temporary). refactor(native-chat): structured agents declare their capabilities instead of shared code naming Claude and Codex #25076 gives each of them one owner; feat(native-chat): open structured chat's wire and stored records to registered agents, behind a negotiated capability #25159 opens the stored agent list behind a negotiated capability and replaces the one function that derives a record's handle namespace fromrecord.provider(agentSessionProviderHandleNamespace). The account-home variable opens with the first protocol agent's launch description.Linked Issue
None (maintainer foundation PR for structured native chat beyond Claude/Codex).
Visual Proof
N/A: no UI or behavior change. The handle is internal state, and stored and wire bytes are unchanged for Claude and Codex.
CI status (October 5)
Merged current main (clean). On head
4178e5a0385every CI job passes, including typecheck, cross-version wire compatibility and packaging, except unit shards 1/5 and 4/5. Those two fail on main's own locale tests, which #25418 broke by adding translations the tests expect to be missing (NativeChatSupportedAgents.test.tsx,source-control-discard-localization.test.ts). This PR touches neither those tests nor any locale catalog. The aggregateverifyjob fails only because of those shards.Testing
New tests (
src/shared/agent-session-record-stored-form.test.ts,src/shared/agent-session-provider-handle.test.ts):forkedFromKeyand a superseding creation'ssupersedesKey) validate, decode to the neutral handle, and encode back byte for byte.acp/grokwith provider data) round-trips, is refused by older builds' rules (it does not read as Codex), and has no attach-wire form.{ kind }objects as before (includingpending).nativeIdalone. A same-fence resume that only moved the resume cursor is recorded, not elided. A chain never accepts a link from another transport or agent with the same id.Run:
vitest run --config config/vitest.config.tson every test file that changed or imports a changed module (480 files, 4780 tests): all pass exceptclaude-agent-sdk-contract-pins.test.ts. That test reads the installed SDK'spackage.jsonfrom the sharednode_modulesof my checkout (0.3.251 against the pinned 0.3.284). This PR touches neither, so it is a stale install on my machine, not this change. Also run: the changed-code quality gate (check-changed-code-quality.mjs, passes), oxlint and oxfmt on every changed file (clean).What I verified / didn't
providerHandleChain), and that older builds set an unreadable record aside without rewriting it (by readingagent-session-record-rows.ts; I did not run an older build).git merge-tree).tests/e2e/cross-version-wire/agent-session-stop-event-downgrade.unit.test.tspasses. It opens the same journal with an older build's code, which now gets its identity in the typed form it expects. The other changedtests/e2eunit tests pass too.Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
{ provider: 'codex', threadId }→codexProviderHandle(...)). The substantive change is in the two new shared modules and about 20 non-test files.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)