Skip to content

fix: work item deadlock - centralized claim mirror clearing + dispatc… - #21

Open
myfastcat wants to merge 1 commit into
HKUDS:mainfrom
myfastcat:main
Open

fix: work item deadlock - centralized claim mirror clearing + dispatc…#21
myfastcat wants to merge 1 commit into
HKUDS:mainfrom
myfastcat:main

Conversation

@myfastcat

Copy link
Copy Markdown

fix: work item deadlock - centralized claim mirror clearing + dispatch fixes

Summary

Fixes a deadlock where work items get stuck in READY or READY_FOR_REWORK phase but cannot be claimed by any session, causing the entire workflow to stall.

Root Cause

When a work item's claim column (claimed_by_role_runtime_session_id) was cleared, the metadata mirror (metadata.claimed_by_role_session_id / metadata.claimed_task_id) was not always cleared consistently.

The dispatch CAS (claim_delegation_work_item_if_dispatchable) predicates on both the column and metadata mirror being empty. When only the column was cleared but the metadata mirror retained a stale value, the work item became READY but permanently unclaimable — a deadlock.

This manifested in:

  • 3-task scenario: Parent task stuck after child approval
  • 4-task scenario: QA/delivery tasks stuck in READY

Changes

File Change Purpose
opc/database/store.py Centralized fix in update_delegation_work_item Whenever claimed_by_role_runtime_session_id is cleared to empty, automatically clear metadata.claimed_by_role_session_id and metadata.claimed_task_id. This closes the entire bug class at the single write chokepoint.
opc/layer2_organization/company_mode.py _work_item_is_runnable early returns Items with synthesis_turn_started=True or frontier in ("resumed", "synthesis_ready") or READY_FOR_REWORK phase bypass the dependency check — they were already validated when woken.
opc/layer2_organization/company_mode.py _rehydrate_parked_member_sessions enhancement Unpark blocked sessions when their focused work item is in DONE_PHASES or is_runnable(), allowing them to pick up

@LZH-YS1998

Copy link
Copy Markdown
Contributor

Thanks for this PR — the core fix targets the real root cause, which we have confirmed.

What works (verified)

The centralized mirror-clearing in store.py is a genuine root-cause fix. The claim CAS (claim_delegation_work_item_if_dispatchable) predicates on both the claim column and the metadata mirror (claimed_by_role_session_id / claimed_task_id) being empty, but two release paths on main clear only the column:

  1. transition_work_item(release_claim=True) into a non-terminal phase — the mirror is only dropped for DONE_PHASES (work_item_transition.py:177-179); the review-reject → READY_FOR_REWORK release goes through here;
  2. clear_claim_on_wake in refresh_dependents_for_run when a parent wakes after all children are approved (work_item_transition.py:1163).

We reproduced both scenarios against a real SQLite store: on main both are permanent deadlocks (column empty, mirror stale); with this PR both become claimable again. Enforcing "clearing the column also clears the mirror" at the single write chokepoint (update_delegation_work_item) is the right architectural call. The DONE_PHASES branch in _rehydrate_parked_member_sessions is also a real fix — the terminal hook only updates the durable role session, not the in-memory member session, so sessions genuinely could stay parked on an approved card forever.

Regression: ~500 related tests, zero new failures (all observed failures pre-exist on main).

What is still open

"Closes the whole bug class at the single write chokepoint" is not quite complete. reopen_approved_delegation_work_item_for_rework (store.py:5700) clears the claim column via its own SQL, bypassing update_delegation_work_item, and the engine's follow-up reopen call site (engine.py:8188) does not unset the mirror. We verified empirically: an APPROVED card (which retains its claim as an audit record) reopened through this path still deadlocks on this PR's branch — the same bug class survives there.

What this PR would introduce

The four new early-returns in _work_item_is_runnable are not needed for the deadlock fix — in both scenarios from the PR description, the dependency loop already passes (all children APPROVED, or the settlement stamp covers failed deps). We confirmed the store.py change alone resolves both deadlocks. The bypasses add new risk:

  • The rework_feedback bypass has no phase gate, and the park paths never clear that field. A parent under rework that re-delegates children and parks in WAITING_FOR_CHILDREN is normally protected because the parked card still holds its durable claim — but after a process restart, the startup sweeper clears that claim, and this bypass then lets the parent skip the dependency check and get claimed prematurely, running concurrently with its still-active children (premature synthesis).
  • The READY_FOR_REWORK bypass dispatches a rework card before its dependencies complete when those dependencies were rebuilt/rewired to fresh cards (failure-triage flows).
  • or is_runnable(work_item.phase) in _rehydrate_parked_member_sessions unparks sessions while their focused card is under dispatch_hold / queued_behind_session, for marginal benefit.

Suggested changes

  1. Keep the centralized mirror-clearing in store.py and the DONE_PHASES branch in _rehydrate — that is where all the net benefit of this PR lives.
  2. Drop (or substantially narrow) the four early-returns in _work_item_is_runnable. If a concrete scenario requires one, gate the rework_feedback bypass on phase == Phase.READY_FOR_REWORK at minimum, and add a test demonstrating the stall it prevents.
  3. Close the remaining gap: make the release_claim branch of reopen_approved_delegation_work_item_for_rework clear the metadata mirror as well, consistent with the centralized fix — that is what actually closes the bug class.
  4. Add regression tests for the two release scenarios (reject-release, children-approved wake) asserting that after release both the column and the mirror are empty and the CAS can re-claim.

- Enforce the runtime-claim/metadata-mirror invariant in update_delegation_work_item so any path clearing the column also clears the mirror the dispatch CAS predicates on.

- Clear the metadata mirror in reopen_approved_delegation_work_item_for_rework release_claim branch, closing the reopen-for-rework deadlock gap.

- Add DONE_PHASES check in _rehydrate_parked_member_sessions to clear focus when work item is in terminal phase.

- Add regression tests asserting that reject-release, children-approved wake, and reopen-for-rework leave both column and mirror empty and re-claimable.
@myfastcat

Copy link
Copy Markdown
Author

Thank you for your feedback, this PR is updated accordingly. Please review.

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.

2 participants