Fix two ways a company-mode session becomes permanently unusable (#52) - #53
Open
evgemar wants to merge 2 commits into
Open
Fix two ways a company-mode session becomes permanently unusable (#52)#53evgemar wants to merge 2 commits into
evgemar wants to merge 2 commits into
Conversation
added 2 commits
August 27, 2026 21:39
The delivery-review Task closure and the WorkItem approval are two writes on two different SQLite connections, so an interruption between them can leave an AWAITING_HUMAN Task next to an already approved WorkItem. _ensure_open_final_delivery_review_checkpoints() then republishes the card and re-parks the Task for good: the pair no longer satisfies the attempt envelope check in settle_stale_delegation_run_claims_for_controller(), so every later controller takeover raises and the session can never accept another message. Skip republishing when the linked WorkItem has already reached a done phase. The helper is deliberately conservative — a missing link, a store without the accessor, or any lookup failure reports False and preserves the previous behaviour. Refs HKUDS#52
settle_stale_delegation_run_claims_for_controller() selects work items by claim, not by phase, so a card that was claimed but never dispatched is inspected by controller takeover. Both sides then carry an empty attempt envelope, which failed every branch: the raise condition rejects attempt_seq <= 0, and both escape hatches require either attempt_seq > 0 or a terminal phase. An empty envelope on both sides is symmetric, not mixed — there is nothing to reconcile. The remaining conjuncts already pin project, run, link kind, projection id and claim identity, and linked_task_status == expected_linked_task_status proves the pair agrees under task_status_for_phase. Requiring DONE_PHASES on top of that made a consistent waiting_dependencies/blocked pair raise, and since the raise happens on every later takeover the session could never be resumed. Observed twice on real runs, in two different projects. Recovery required hand-editing SQLite to clear the claim columns. Refs HKUDS#52
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.
Fixes both variants of the permanent session wedge reported in #52.
Two real company-mode runs, in two different projects, ended with a session that
could never accept another message:
Recovery required hand-editing SQLite, so the runs were effectively lost.
1. Do not republish a delivery review card over a settled WorkItem
The delivery-review Task closure and the WorkItem approval are two writes on two
different SQLite connections. An interruption between them leaves an
AWAITING_HUMANTask next to an alreadyapprovedWorkItem._ensure_open_final_delivery_review_checkpoints()then republishes the card andre-parks the Task for good, and the pair fails the envelope check on every later
takeover.
Skip republishing when the linked WorkItem already reached a done phase. The new
helper is deliberately conservative: a missing link, a store without the
accessor, or any lookup failure reports
Falseand preserves the previousbehaviour.
2. Treat a never-dispatched linked pair as symmetric, not mixed
settle_stale_delegation_run_claims_for_controller()selects work items byclaim, not by phase, so a card that was claimed but never dispatched still gets
inspected. Both sides then carry an entirely empty attempt envelope, which failed
every branch: the raise rejects
attempt_seq <= 0, and both escape hatchesrequire either
attempt_seq > 0or a terminal phase.An empty envelope on both sides is symmetric — there is nothing to reconcile.
The remaining conjuncts already pin project, run, link kind, projection id and
claim identity, and
linked_task_status == expected_linked_task_statusprovesthe pair agrees under
task_status_for_phase. RequiringDONE_PHASESon top ofthat made a consistent
waiting_dependencies/blockedpair raise.The guard itself is untouched for genuinely mixed envelopes — the existing tests
that assert the raise still pass.
Tests
tests/test_claim_release_invariant.pygainstest_controller_takeover_skips_never_dispatched_consistent_pair, which fails onmainwith the exact production error and passes with this change.Full suite: 2786 passed. Nine failures are pre-existing on
mainin thisenvironment (macOS permission errors on
chflagsin temp dirs); the failure setis byte-identical with and without this branch.
Not addressed here
The underlying non-atomicity remains: the Task closure, the WorkItem approval and
the checkpoint terminal status are three separate writes, and the work-item
writer uses its own isolated connection, so they cannot share a transaction as
currently structured. That looks like an architectural decision rather than a
drive-by fix, and it is described in #52 for whoever picks it up.