Add an offline repair for stranded automation quarantines - #257
Conversation
Automation sessions stranded by the missing run-state settlement cannot be recovered online. The wedge detector fenced them with run_state:<state>, and quarantineRepairEvidence refuses to release while the run state is unsettled, so the fence reports repairable:false and an explicit release is a no-op. The actor also fences every session mutation except quarantineSession and releaseQuarantine, so nothing online can settle the state either. Repair the cause offline instead of weakening the fence: settle the run state, after which the unmodified releaseQuarantine passes on its own merits with every other evidence check still running. Release requires durable proof that the run is terminal and unowned: the wedge reason and a run_state command kind, a terminal automation ledger verdict for that exact session, no run-journal record naming it under any alias, and a legal run-state edge. Anything else stays fenced. The job walks the quarantine catalog only, never the placement table and never every actor database, and it asserts the services are stopped before touching state.
🤖 OS review · request changes · confidence 4/5Safe once the P2 below is fixed. The catalog-driven repair is fail-closed and preserves the existing quarantine checks, but its new inspection helper reports incorrect results for actor-isolated sessions.
🔁 Not merge-ready and no live session owns this branch — add the |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![open-session-os-tella-dev[bot]](https://avatars.githubusercontent.com/u/65127121?s=60&v=4){kind=small}
| sessionId: string, | ||
| inputs: AutomationQuarantineRepairInputs, | ||
| ): AutomationQuarantineVerdict & { quarantined: boolean } { | ||
| const store = new SessionKernelStore(centralPath); |
There was a problem hiding this comment.
🟠 P2 — Inspect the routed actor store instead of only the central database
inspectAutomationQuarantine opens SessionKernelStore directly, bypassing the SessionKernelStoreHost routing used by the repair itself. For an isolated session, the wedge quarantine and run state live in its actor database while the central database only holds catalog projections, so store.quarantinedSession(sessionId) returns undefined and this helper incorrectly reports quarantined: false. Use the host's routed quarantinedSession and runState calls, accepting or deriving isolatedRoot, or remove this unused helper until it can inspect both storage placements. Add an isolated-session test because the current test fixture exercises only central storage.
There was a problem hiding this comment.
Fixed in 5ee5be6b6 (pushed as 284b10d).
Confirmed and reproduced. inspectAutomationQuarantine constructed new SessionKernelStore(centralPath) and read both quarantinedSession and runState off it. For an actor-isolated session those live in that session's own database, and the catalog holds only a sparse projection in a different table (session_kernel_sparse_projections), not session_kernel_quarantine. So the direct read returned undefined and the helper reported a live fence as session is not quarantined — an operator checking one of the stranded sessions would have concluded there was nothing to repair.
There is a second, subtler half. Even when the catalog does carry the row, SessionKernelStoreHost.quarantinedSession recomputes repairable from the isolated store's own quarantineRepairEvidence. A central-only read keeps the central verdict, so repairable can be wrong for an isolated session even when the fence is visible. Routing fixes both.
Both reads now go through SessionKernelStoreHost, exactly as repairSettledAutomationQuarantines does, with an optional isolatedRoot (defaulting the same way).
Scope check, because it determines whether the job itself was affected: the scan was not blind. It calls quarantinedSessions through the host, which routes to allQuarantinedSessions and merges central.quarantinedSessions() with central.isolatedQuarantineProjectionEntries(). And host.quarantineSession publishes that projection as it writes the isolated row, which is the path the wedge detector takes via the actor. So only the read-only helper was wrong.
I want to flag one thing I got wrong mid-investigation, since it shaped the tests. My first isolated fixture seeded the fence into the central store and then migrated, and on that fixture the repair job returned [] — which briefly looked like the scan was blind too. That shape is a migration artifact the live instance never has: quarantining through the host after isolation (the production path) publishes the projection and the job works end to end. The fixture now does that, and there is an end-to-end test asserting the isolated session is settled and released.
Tests, all in automation-quarantine-repair.test.ts:
inspection sees an actor-isolated quarantine— asserts the direct central read returnsundefinedfirst, so the test pins the bug rather than just the fixinspection reads an isolated session's own run stateinspection accepts an explicit isolated rootinspection still refuses an unknown session— fail-closed guard: routing must not turn "no fence" into a repairable verdictthe repair job settles and releases an isolated session— end to end on the production shape
Mutation-checked: reverting only the two reads inside inspect back to the central store fails exactly the three isolated-inspection tests and nothing else.
Release checks are untouched — the helper remains read-only, and releaseQuarantine still performs its own authoritative evidence verification. bun run check passes; 18 tests in this file.
|
CI note: the two red jobs here are a pre-existing
Proof it predates this work: The assertion depends on the identity roster: Out of scope for this PR — flagging rather than fixing so it is not silently attributed here. |
tella-butler
added
the
os-auto-fix
inspectAutomationQuarantine opened the central database directly. For an actor-isolated session — which is how live sessions are placed — the quarantine row and run state live in that session's own database and the catalog holds only a sparse projection, so the direct read reported a live fence as "session is not quarantined". Even when the catalog does carry the row, a central read cannot recompute repairable from the isolated store's own evidence the way the host does, so an operator could conclude there was nothing to repair. Route both reads through SessionKernelStoreHost, as the repair already does. The scan itself was never blind: it reads the merged catalog through the host, and host.quarantineSession publishes the projection as it writes the isolated row. Release checks are unchanged; the helper stays read-only and still fails closed for a session with no fence.
…tomation-sessions
|
Post-merge independent review of exact head |
3 participants
Follow-up to #256. That PR stops new automation sessions from being stranded; this one recovers the ones already stranded. Split out because it touches no production code path — it is an offline operator job plus a pure evidence reducer.
Why an explicit release does not work
I originally reported that already-quarantined sessions could be recovered with an explicit
{type: "quarantine", action: "release", sessionId}. That was wrong, and the tests here prove it:commandKind: "run_state:running".SessionKernelStore.quarantineRepairEvidencereturnsfalsewhenever the run state is one ofpreparing/starting/running/ask_blocked/interrupted/reattachingand there is no recoverable gateway/delivery settlement. Arun_state:*fence has neither, so it short-circuits tofalse.quarantinedSession().repairableisfalse— which is whatGET /api/system/session-kernel/dead-lettersreports — andreleaseQuarantinereturns without deleting the row.actor-worker.tsfences every session mutation exceptquarantineSessionandreleaseQuarantine, so nothing online can settle the run state either.That is a genuine deadlock, and it is correct behaviour: the fence exists precisely because an unsettled run state with no owner is unverified.
the stranded state is genuinely unrecoverable onlinein the new test file pins both halves.Approach: repair the cause, not the fence
Rather than adding a bypass to
quarantineRepairEvidence, the job settles the run state offline. After that the unmodifiedreleaseQuarantinepasses on its own merits, with every other evidence check still running — ambiguous commands, claimed timers, pending effects all still fail closed. Genericrun_statequarantine keeps its full strength for every session that cannot produce the proof below.No production code path changes.
The durable proof required
Every clause is a fail-closed conjunct (
settledAutomationQuarantineEvidence):commandKindstarts withrun_state:.ok/error) for that exact session id. This is the durable receipt thatrunAutomationdrained its engine stream and reached its completion tail — the precise state the missing settlement stranded. A ledger still readingrunningproves nothing and is refused.osSessionId,claudeSessionId,runKey). A journal record is a live owner or a recovery claim; either way the run may still execute.nextRunStateedge.turn_endforok,run_failedforerror. This never invents a state the machine would refuse — it applies the settlement the completed run should have applied itself.Ownership invariants
The job walks the quarantine catalog only. It never enumerates placements and never opens every actor database. It asserts
opensession,opensession-executorandopensession-session-kernelall report explicitinactivebefore touching state, reusingassertServicesStoppedfromscripts/migrate-actor-transcripts.ts.Usage
Tests
18 across two files. Beyond the two deadlock-proof tests above:
failedrunningand absent)idle+turn_endis refused as a double teardown)currentRunIdis clearedbun run checkpasses.Started by Johnny Lin in this OS session