Skip to content

fix(board): the working-state snapshot is current or says it is stale (#401) - #433

Merged
mabry1985 merged 1 commit into
mainfrom
fix/work-snapshot-staleness-401
Sep 11, 2026
Merged

mabry1985 merged 1 commit into
mainfrom
fix/work-snapshot-staleness-401

Conversation

@mabry1985

@mabry1985 mabry1985 commented Sep 10, 2026

Copy link
Copy Markdown
Member

What was wrong

The PM's injected working state showed bd-ezs7 and bd-p8ft in states the store had already moved them out of, and the two were stale in opposite directions. From the audit and agent logs (UTC):

time event
07:15:08 the health sweep publishes the snapshot, at the start of the sweep: ezs7 in_progress, p8ft blocked
07:16:40 the PM itself calls board_unblock_feature(bd-p8ft), which returns ready
07:17:02 the PM itself calls board_block_feature(bd-ezs7), which returns blocked
07:17:20 the PM writes "bd-p8ft is confirmed board_state=blocked / blocked_class=terminal" and plans around it

The host doesn't cache the snapshot: protoAgent graph/work_providers.collect_work_sections calls the provider every turn. The staleness came from the plugin. The snapshot was republished only by the 300s sweep, before the sweep's own transitions, and it carried nothing that said how old it was.

Fix: how stale the snapshot can be, stated exactly

  • Every write marks it stale. Every br write that can move a card (create, update, close, reopen, delete) bumps a process-wide revision, and so does a live config reload of a knob the hints read. The bump happens in a finally around the call, so a write that timed out but may still have landed counts. With fix(loop): a stalled br call costs one read, not the whole tick (#404) #431, a stall reaches that finally only after _run_br_process has stopped the br tree (SIGTERM, then SIGKILL) and reaped it, so nothing the write does can land after the mark.
  • A stale snapshot says so. A snapshot records the revision it was read at (the revision is read before the board). When it is behind, the first item is a STALE line naming when it was taken, placed first so a host item cap can't trim it.
  • A refresher of its own. It runs separately from the claim tick, so a loop paused at its setup gate still refreshes. It re-reads within 5s of a change (coalesced, so a burst of writes costs one read), and at least every 60s in any case, which bounds writers this process can't see. It backs off exponentially while reads fail. A failed read, fix(loop): a stalled br call costs one read, not the whole tick (#404) #431's BoardTimeout included, is logged as one line with no traceback. Only one read runs at a time: a second one while a read is in flight is skipped. A snapshot older than 120s is marked STALE.
    • The result: the snapshot is either marked STALE or at most about a minute old.
  • The sweep publishes last. It publishes after its own transitions. A stall in that read is the sweep's stall, and it goes up to _tick_phase, which ends the tick (fix(loop): a stalled br call costs one read, not the whole tick (#404) #431's rule that the first stall ends the tick). If it were swallowed there, the preflight and the claim scan would each stall on the same wedged store.
  • A light read. store.live_cards() is one br list of the open statuses. It adds a br show, in _SHOW_BATCH batches, only for the rows whose hint needs more: blocked cards (their reason and edges) and backlog cards that report dependencies. When a card could be stranded, it also fetches that card's closed dependencies. It replaces the full-board read, which showed every id and scanned br ready (the ~350-id call fix(loop): prevent a stalled br query from aborting the entire reconcile tick #404 caught stalling).
  • feat(loop): surface and promote stranded cards whose recorded blockers have cleared #406 compatibility. feat(loop): surface and promote stranded cards whose recorded blockers have cleared #406 landed while this PR was open, and the light read has to keep its "dependencies closed" hints. br list omits dependency edges and closed cards, so, before this change, the light read dropped a stranded backlog card from the view. It also told a cancelled dependency apart from a merged one only by accident. Now both survive the light read, and _report_stranded logs from this read, once per card.
  • Blocked hints. A blocked card, ranked first, reads needs a human (<class>): <reason> or retries on its own (<class>): <reason>. (feat(loop): surface and promote stranded cards whose recorded blockers have cleared #406 also annotates the rest with the board's next action.)
  • The block-class leftover (the issue's secondary finding).
  • The snapshot state lives in a process-stable sys.modules slot, so a plugin reload can't split the counter between the store and the provider. Two stale comments were corrected: failures.py, and a comment in drive.py.

Tests

  • tests/test_work_snapshot_staleness_401.py:
  • tests/test_stale_blocked_class_401.py (real br):
    • an unblock drops the class and keeps the tiers, for the terminal, dispatch-infra and transient classes;
    • a class left on an unblocked card isn't reported.
  • tests/test_work_snapshot_review_401.py, one test per review finding:
  • Updated existing tests: the test_loop sweep fake serves live_cards, and test_store's unblock test now expects the class to be dropped.
  • Dropped: the card-hint test. feat(loop): surface and promote stranded cards whose recorded blockers have cleared #406 fixed the same blank hint on main and has its own test, so this one passed on main and could no longer red-check.

Red-check:

#431's test_a_stalled_reconcile_read_does_not_cost_the_claim_scan: it is not on main. #431's review round removed it, in 23c014c, when the first stall came to end the tick. Its successor is test_after_a_stall_the_next_tick_claims. In a scratch copy that starts this PR's refresher for every loop #431's tick tests build, all 19 of tests/test_br_timeout_404.py pass, three runs each for the two tick tests. In the next-tick test, the tick hits the stall and the refresher's read succeeds. In the wedged-store test, the tick still makes exactly one stalled call. The refresher then stalls on its own and backs off.

Seam ratchet, recomputed on main with the file's own _external_seams() scanner:

  • MAX_UNCOVERED_STORE falls from 18 to 16, because clear_blocked and block_from_review are now REAL.
  • live_cards and the new _show_by_id are REAL.
  • Store seams are now 33 REAL and 16 UNCOVERED. The worktree seams are unchanged: 24 REAL, 3 EXEMPT, 0 UNCOVERED.

Gates (rebased onto 700885d, which includes #431, #438 and #436): ruff format --check ., ruff check . and pytest -q all pass, with 2139 passed and 16 skipped.

Fixes #401

🤖 Generated with Claude Code

https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

mabry1985 added a commit that referenced this pull request Sep 10, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
mabry1985 added a commit that referenced this pull request Sep 10, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
mabry1985 added a commit that referenced this pull request Sep 10, 2026
Adversarial review of #433 found five issues and four smaller ones. Each fix has a test that
fails on the previous head (6ec8b31), or kills the mutation review showed surviving.

1. A failed refresh logged a traceback per attempt and retried with no backoff (26 reads in
   0.3s). A BoardError is now one line (like #431's _tick_phase), and the refresher backs
   off exponentially (5s doubling, capped at 60s).
2. The refresh was the full-board read -- br list of every status + a br show of EVERY id +
   br ready, the ~350-id call #404 caught stalling -- run after every write. The snapshot
   now reads store.live_cards(): one br list of the open statuses, plus a batched br show of
   only the blocked cards (for their reasons). Refreshes are coalesced (at most one per 5s).
3. Wiring pinned against the mutations that survived: removing the refresher, reading the
   revision after the board, cutting the write set to {"update"} -- each now fails a test.
4. Staleness is bounded whatever this process writes: the refresher re-reads a quiet board
   at least every 60s (a hand-run br, health_sweep_interval_s: 0), and the provider marks a
   snapshot older than 120s STALE (the refresher failing or stopped). The docstring and
   changelog state that bound instead of "current or says it is not".
5. The refresh is its own task, started with the loop, not a step of the claim tick -- a
   loop paused at its setup gate kept a STALE snapshot for the whole pause. A live reload
   that changes a knob the hints read marks the snapshot stale. Blocked cards (ranked
   first) now carry their reason and whether they retry on their own or need a human.

Small: block_from_review stamps its own class (terminal) over any stale one and records a
`blocked:` reason, so an old `transient` can't make the sweep auto-heal an exhausted
escalation, and boot's preflight-hold release reads the CURRENT block (it released an
escalation block on a card that had once been preflight-held); the revision/snapshot state
moves into a process-stable sys.modules slot so a plugin reload can't split the counter;
drive.py's stale "operator unblock resets the tier posture" comment is corrected.
live_cards and block_from_review are exercised through real br, so the store's
uncovered-seam ratchet falls to 19.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
…#401)

The PM's injected working state showed two cards in states it had itself just moved them out
of (protoEngineer, 2026-09-07: it unblocked bd-p8ft at 07:16:40Z and blocked bd-ezs7 at
07:17:02Z, then planned around "bd-p8ft is confirmed blocked" at 07:17:20Z). The host reads the
provider fresh every turn; the plugin's snapshot was refreshed only by the 300s health sweep,
at the START of it, and carried nothing that said how old it was.

- Every br write that can move a card (create/update/close/reopen/delete) bumps a process-wide
  revision, in a `finally` around the call so a write that timed out but landed still counts.
  #431's stall reaches that `finally` only after the br tree is stopped and reaped, so nothing
  can land after the mark. A live config reload of a knob the hints read bumps it too.
- A snapshot records the revision it was read at (read BEFORE the board); while the board has
  moved past it, or it is older than 120s, the provider leads with a STALE line naming when it
  was taken.
- A refresher of its own (not the claim tick -- a paused loop still refreshes) re-reads within
  5s of a change (coalesced), at least every 60s regardless (bounding writers this process
  can't see), backs off exponentially on failure, and logs a failed or stalled read
  (BoardError, BoardTimeout included) as one line. One read at a time: a second while one is
  in flight is skipped. The sweep publishes after its own transitions, and a stall there is
  the sweep's stall: it goes up to _tick_phase and ends the tick (#431's "first stall ends the
  tick"), where swallowing it let the preflight and claim scan stall on the same store.
- The read is store.live_cards(): one `br list` of the open statuses, plus a `br show` (in
  _SHOW_BATCH batches) of only the rows whose hint needs more -- blocked cards (reason,
  edges) and backlog cards that have dependencies -- and, when a card could be stranded, its
  closed dependencies, so #406's "dependencies closed" hints survive the light read and a
  CANCELLED dependency is still told from a merged one. Cards newly stranded are logged from
  this read (once per card).
- Hints: a blocked card reads "needs a human (<class>): <reason>" / "retries on its own
  (<class>): <reason>".
- Block classes: clear_blocked drops the class whatever it is (noted on its audit comment),
  still resetting a too-wide park's timeout count (#378); the projection reports blocked_class
  only while blocked; block_from_review stamps its own class and records a `blocked:` reason
  -- so an old `transient` can't make the sweep auto-heal an exhausted escalation, and boot's
  preflight-hold release reads the current block. Tier labels (#339) are never touched.
- The snapshot state lives in a process-stable sys.modules slot; stale comments in failures.py
  and drive.py corrected.
- Seam ratchet recomputed on main: MAX_UNCOVERED_STORE 18 -> 16 (clear_blocked,
  block_from_review now REAL); live_cards and the new _show_by_id are REAL.

Includes the adversarial-review round (tests pin the wiring against the mutations review found
surviving: no refresher, revision read after the board, write set cut to {"update"}).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
@mabry1985
mabry1985 force-pushed the fix/work-snapshot-staleness-401 branch from d595621 to e1ace21 Compare September 11, 2026 00:47
@mabry1985
mabry1985 merged commit e74c6ad into main Sep 11, 2026
4 checks passed
@mabry1985
mabry1985 deleted the fix/work-snapshot-staleness-401 branch September 11, 2026 00:51
mabry1985 added a commit that referenced this pull request Sep 11, 2026
No conflicts. #433 moves the working-state snapshot onto its own refresher task, which
`stop()` now cancels alongside the tick. Shutdown's save-then-remove of in-flight trees
still runs after both. `clear_blocked` now drops any blocked-class label, and that
includes `stranded-work`, so unblocking a card after recovering its tree works as the
docs say.

The seam ratchet, recomputed from the scanner: worktree.py has 31 seams (28 REAL,
3 EXEMPT, 0 UNCOVERED) and store.py has 49 (33 REAL, 16 UNCOVERED). The constants are
exact: MAX_UNCOVERED_WORKTREE 0, MAX_EXEMPT_WORKTREE 3, MAX_UNCOVERED_STORE 16.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
mabry1985 added a commit that referenced this pull request Sep 11, 2026
The one conflict was module constants in loop/reconcile.py: #433's refresher poll interval
and the salvage's gate-output constants. Both are kept.

#431 bounds a stalled `br` call by raising. A salvage whose first board read raised used
to escape as an exception, which the route turned into a 500. It now returns an `error`
record and still releases the card's reservation. The failures after the push already
land in the record. The new test fails without this.

The seam ratchet, recomputed from the scanner: worktree.py has 33 seams (30 REAL,
3 EXEMPT, 0 UNCOVERED) and store.py has 49 (33 REAL, 16 UNCOVERED, which is exactly
MAX_UNCOVERED_STORE).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review-structural · head e1ace21e994f · formal

This review produced no findings because all five lanes failed on infrastructure (four exceeded their 1200 s budget; the structural lane hit a gateway auth/config error). No code was actually reviewed. The verifier confirmed zero claims to verify and recommends re-running the panel before merging. Do not treat this as a clean pass — it is a complete coverage gap. Re-run the panel (or at minimum the structural and correctness lanes) and gate the merge on a successful pass.

No findings — the review came back clean.

findings JSON (machine-readable)
[]

4 panel step(s) hit their time budget and were skipped this round: find_correctness, find_removed_behavior, find_crossfile, find_conventions. The verdict stands on the remaining angles; a finding only that step would have caught could be missed — the next push re-runs the full panel.

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.

The injected working-state projection disagrees with the board store, so the PM plans against stale card states

1 participant