fix(runtime-host): lazily adopt legacy Session Connection identity - #4627
fix(runtime-host): lazily adopt legacy Session Connection identity#4627bug-superman wants to merge 1 commit into
Conversation
b494457 to
410468d
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
The direction is right and the identity store stays single-owner: llmConnectionId is still only written by HostSessionCatalogCoordinator and the revision inheritance path. I verified the Host has one SessionAdmissionGate instance shared by the catalog, root and message coordinators, that moving the sessionCatalog construction earlier does not use anything defined after it, and that the six readSessionHeader call sites in message-coordinator.ts that do not pass a lease only read isArchived, so they are not a gap. Three things to fix before merge, plus one scope question.
P2: the wiring has no test. All five new tests call coordinator.adoptLegacySessionConnectionIdentity(...) directly; nothing exercises RootTurnCoordinator or HostMessageCoordinator, so the user-visible outcome the PR claims (a legacy Session can send again) is not covered. Two things make that riskier than usual. The adopter is an optional trailing constructor parameter, so every existing RootTurnCoordinator construction silently keeps the old behavior and a wiring mistake cannot turn a test red. And a lease that crosses task contexts makes SessionAdmissionGate throw (Cannot reuse a Session admission lease from another task, Session admission lease was not issued by this gate), which would surface as a hard error on the normal send path. I could not verify by running whether the lease passed at root-turn-coordinator.ts:2213 is always in the issuing context, so that is an evidence gap on my side. Please add one end to end test: an unlocked legacy Session that is refused on main and sends here, asserting llmConnectionId is persisted.
P3: readSessionHeader now writes to the store (root-turn-coordinator.ts:510, contract at message-coordinator.ts:197), while its name and interface still say read. Consider renaming it or lifting the adoption out into its own call.
P3: prepareSessionHeader is called at nine sites, each one a readHeaderSnapshot immediately followed by a prepareSessionHeader. Folding both into a single private readAdmittedHeader(sessionId, lease) puts each call site back to one line and makes it impossible for a future entry point to miss the step.
Scope question, not a finding: connectionLocked is set as soon as a user message lands (packages/storage/src/sqlite-session-metadata-store.ts:1494), so this only unblocks legacy Sessions that never sent anything. Every pre-upgrade Session with history stays blocked and needs the user to explicitly reselect a Connection. The Safety section explains why locked Sessions cannot be guessed, and I agree with that, but the title and summary read as if all legacy Sessions are covered. Please state the actual scope, and confirm the reselect path is reachable in the UI for a Session that is currently reporting the legacy unavailable reason.
On CI: gh pr checks reports no checks on this branch, and the only workflow run at this head is CI sitting at action_required. Nothing has been verified by CI at 410468db.
Generated-by: OpenAI Codex
410468d to
d133857
Compare
Closes #3860.
Summary
Safety
Bound execution continues to resolve by immutable
connectionIdand verifies the stored slug. Missing IDs and ID/slug mismatches never fall back to a same-slug catalog entry, so deleted/reused Connections cannot cross credentials. OAuth reconciliation remains ID-bound through the existing resolver.A pre-upgrade legacy Session has no deletion tombstone or historical identity evidence. If its original Connection was deleted and the slug reused before this upgrade, the original identity cannot be reconstructed; locked legacy Sessions therefore remain blocked instead of being silently rebound.
Verification
npm --workspace @maka/runtime-host run typecheck(Node 24)npm --workspace @maka/runtime-host run build(Node 24)@maka/uitype errors after core/storage/runtime/runtime-host compile successfully.