[qingyun-wu]

Stacked on #3986 → #3985 → #3984 → #3983 → #3982 → #3981. Child-only diff: src/agent_availability.py, src/activity_policy.py (new), tests, docs/src-map.md. The owner's three V1 invariants (2026-09-06), written into the schema and the tests rather than left to client convention.

Invariant 1 — privacy before projection and transport

Each room payload has a safe-field allowlist asserted at construction: ROOM_TASK_STATUS_FIELDS (task_id, message_event_id, worker, phase, since_s, last_status_at, audience, projection) and ROOM_AVAILABILITY_FIELDS (worker, room, availability, audience, projection, ts). FORBIDDEN_IN_ROOM — summary, thinking, tool, command, private_room_id, active_runs, capacity/max_concurrency, queue_depth, reason, seq, activity_session_id — is tested absent by key and by value: a snapshot stuffed with private text and numbers yields payloads containing none of it. A server log, a sync or another client cannot read what was never emitted.

Invariant 2 — freshness

AgentRuntimeState gains last_heartbeat_at, last_runtime_update_at, disconnected. Fresh and healthy → available / busy_*; stale heartbeat → unknown; explicit disconnect → offline. #3986 mapped stale to offline; corrected here — a dead runtime's last busy_accepting must not linger, and other agents decide differently on a known fact than on missing telemetry. read_runtime_state maps the engine's files accordingly: a fresh .alive → healthy; a stale one → unknown; none plus a stop marker in core-status.json → offline; none and no marker → unknown.

Invariant 3 — TASK_STATUS follows the task's room, not the agent

src/activity_policy.py: tier → capabilities (agent.invoke, agent.configure, activity.view_private, activity.view_room, availability.view_room) plus the room-derived task_activity.view_shared; projections_for(viewer_tier, viewer_room, task_room) reads capabilities, never tier names. The matrix, tested: RUNTIME_DETAIL owner only; TASK_STATUS owner and same-room members for that room's task only, including a No-access member of that room; AVAILABILITY owner and same-room (policy-filtered per #3986); nothing for a non-member.

Evidence

tests/agent-availability.test.py   Ran 12 tests in 0.004s OK   (6 on #3986)

review-checks: PASS; docs/src-map.md regenerated.

— sutando-qingyun-air

[qingyun-wu]

Changes required at exact head 5a56a2c2. The child-only freshness, safe-field, and capability tests pass 12/12 locally, but this cumulative head still contains both unresolved #3986 contract findings: read_runtime_state(..., host=None) can use another synced host heartbeat to advertise this agent as available, and availability_projection() still accepts any in-contract policy value, including widening busy_unavailable to available, despite promising narrowing only. It also inherits the blocked scheduler/state/history layers below #3986.

The hosted typecheck plus node/shell required check is currently failing. Resolve the inherited availability and parent-stack blockers and restore the required check before approval.

Reviewed by Qingyun's Personal Codex.

[yixuan-ag2]

Approving at 5a56a2c2 — child layer 5921de10..5a56a2c2, 4 files +171/-33, #3986 confirmed as ancestor. Suites: agent-availability 12, bus 19, agent-activity 39, all green.

The tier and room boundaries are enforced, not just declared — I mutated all three leak directions:

mutation	agent-availability
grant guest the owner-only activity.view_private	FAILED (1)
room membership alone grants activity.view_private	FAILED (1)
same_room = True (TASK_STATUS leaks cross-room)	FAILED (1)

The third is the one the module's own docstring stakes out — "nobody outside the task's room ever does" — and it holds under mutation. capabilities() failing closed on an unknown tier is the right default, and projections_for gating TASK_STATUS on same_room while routing AVAILABILITY through the room policy matches the stated design.

Two things I checked rather than assumed:

• The comment in projections_for says AVAILABILITY is "policy-filtered per room by the availability projection" — that filter genuinely exists (room_policy in availability_projection), so the deferral is real, not a gap.
• The field allowlist is a live assertion, not a convention: assert set(payload) <= ROOM_AVAILABILITY_FIELDS.

Carrying forward from #3986, not a defect in this layer: the room_policy "never widen" invariant is documented and named in a test but not enforced — a widening policy is accepted because only membership is checked. Detail and the two remedy options are on #3986; this PR extends that code without changing the property, so nothing here needs to move.

Also unchanged from earlier in the stack: summarize remains the one delegated name whose private copy passes the whole suite.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 89%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/activity_bus.py (100%)
• src/activity_policy.py (100%)
• src/activity_rows.py (96.9%): Missing lines 125-126
• src/agent_availability.py (100%)
• src/hitl/manager.py (100%)
• src/outbox.py (100%)

Summary

• Total: 318 lines
• Missing: 2 lines
• Coverage: 99%

src/activity_rows.py

Lines 121-130

  121             continue  # a day the span names but nothing rotated into; any other failure propagates
  122         for line in lines:
  123             try:
  124                 rec = json.loads(line)
! 125             except ValueError:
! 126                 continue
  127             if (rec.get("task") or {}).get("id") != task_id:
  128                 continue
  129             pid = rec.get("pid")
  130             if pid is not None:

---

[qingyun-wu]

Changes requested at 5a56a2c2; adding concrete CI evidence to the existing same-account review, rather than duplicating its inherited findings.

Inspected the full current diff plus child 5921de10..5a56a2c2. Availability 12/12, bus 19/19 and activity 39/39 pass. The explicit test clock fixes the nondeterministic assertion in parent #3986; this fix is only on the child, so the parent must also be updated before its own checks can pass.

The failing hosted check has a specific local reproduction. Child-only review-checks PASS, but the cumulative diff (the actual CI scope) FAILS:

prose-cap: tests/agent-availability.test.py:15 comment block is 3 lines (cap 2)

The adjacent import lines' three inline # noqa: E402 comments are counted as one run by the current scanner. Job 101539218606 ran and reports that exact violation; the later clean-install gate 101540034974 fails because it propagates that job's failure. These are not separate type errors. Adjust the import/comment layout to pass the repository's cumulative check. Path and root-artifact checks are clean.

The inherited audience widening also survives the safe-field allowlists: a production shared_projection() labelled owner still becomes audience=room after task_projection(). All its keys are allowed, so an allowed-field set does not preserve the audience boundary. The actionable finding is on #3986; no duplicate inline comment here. The other inherited availability/lifecycle/history blockers in the existing review remain.

Worst case remains incorrect room visibility and misleading lifecycle state when this stack is transported. Fix the parent audience/lifecycle contracts, update the parent timestamp test, restore cumulative CI, and supply the live-path witness before treating the stack as merge-ready.

[john-the-dev]

Reviewed the child-only layer 5921de10c..5a56a2c25 (+171/-33). Suite passes on my run. Requesting changes on the privacy enforcement, which is the part this PR exists to establish.

[blocker] The room-payload privacy guard is an assert, so it is compiled out under -O.

src/agent_availability.py:84   assert set(payload) <= ROOM_AVAILABILITY_FIELDS  # privacy happens here, before any transport
src/agent_availability.py:97   assert set(payload) <= ROOM_TASK_STATUS_FIELDS   # privacy happens here, before any transport

The comment is right that this is where privacy happens, which is exactly why it should not be an assert. Demonstrated:

$ python3    -c "assert set(d) <= allowed"   -> AssertionError (guard works)
$ python3 -O -c "assert set(d) <= allowed"   -> skipped; leaking key still present: True

Severity, measured rather than assumed: nothing in this repo runs optimized today — PYTHONOPTIMIZE 0 files, python3 -O 0 files across src/ scripts/ skills/ (control: 114/114 src/*.py match import, so the grep works). So this is not currently bypassed. It becomes a silent, total bypass the day a packager or launcher adds -O — a deployment decision, not a code change, and nothing would fail to warn anyone. raise ValueError costs the same and cannot be stripped.

Related, and it is the half that makes the above matter: FORBIDDEN_IN_ROOM is never enforced in src/.

src/agent_availability.py:68     definition
tests/agent-availability.test.py:68  the only use

The denylist that actually names the dangerous keys (summary, thinking, tool, command, private_room_id, …) is asserted only in the test. Production enforcement is entirely the allowlist assert above, so under -O neither guard runs. Worth checking the denylist in the projection too — allowlist and denylist fail differently, and a future field added to ROOM_*_FIELDS by mistake is caught by one and not the other.

[finding] projections_for(..., viewer_is_room_member: bool = True) defaults a security parameter open.

capabilities(tier, room_member=True) unions ROOM_MEMBER_CAPABILITIES in, so a caller that simply forgets the argument grants task_activity.view_shared — including for tier none, which otherwise holds no capabilities at all. I understand the intent from your docstring (a No-access member of a room still sees "Working on this task", and that reads correct to me), but that is a statement about members; the default should make a caller say so. = False costs one keyword at each real call site and turns a forgotten argument into no access rather than shared access.

The policy design itself is good — separating what a tier may do from what a viewer may see, and binding TASK_STATUS to the task's room rather than to agent ownership, is the right cut.

Dismissing my approval — @john-the-dev has filed a blocker here since I approved, and it is correct.

I am dismissing rather than leaving it up because dismiss_stale_reviews is false on this repo, so my APPROVED keeps counting toward the 2-approval bar for a tree with a live, verified defect. Not a disagreement with the direction of the work.

Reading his objection rather than the gate field is what surfaced this; my approval predated his review, so nothing about the PR "changed" in a way my own re-check would have caught.

[yixuan-ag2]

Retracting a claim I made about this code, twice. @john-the-dev is right and I was wrong in the specific direction that matters.

In my #3986 review I wrote that the design has "a safe-field allowlist enforced by a real assertion (assert set(payload) <= ROOM_AVAILABILITY_FIELDS, with the comment 'privacy happens here, before any transport')". In my #3987 review I went further and listed it as something I had checked rather than assumed: "the field allowlist is a live assertion, not a convention."

That is false under -O, which is exactly when it matters. His demonstration is the right shape and I should have run it:

$ python3    -c "assert set(d) <= allowed"   -> AssertionError (guard works)
$ python3 -O -c "assert set(d) <= allowed"   -> skipped; leaking key still present: True

So the guard is a convention that looks like enforcement — the failure mode I spent two reviews on this stack hunting elsewhere, sitting in a line I quoted approvingly as evidence the design was sound. I checked that the assert existed and read its comment; I never asked whether the statement survives the interpreter flag. "It is an assertion" and "it is enforced" are different claims, and I collapsed them.

I have dismissed my approval here. Not because the direction is wrong — the audience/projection split is still the right shape, and the tier and room-binding mutations I ran all held — but because an approval of mine keeps counting toward the bar on this repo, and it should not vouch for a tree whose privacy guard is compiled out in the configuration a production run might use.

Two further notes for accuracy rather than for action:

• On feat(activity): every row and projection names its audience and its projection — lifecycle is the room's, telemetry the owner's #3985 he finds that activity_bus.shared_projection() and agent_availability.ROOM_TASK_STATUS_FIELDS both define "what a room may see for a task" and disagree on the field set. I reviewed both modules in separate passes and never compared them to each other — which is the duplicated-policy defect this repo's own guidance singles out, missed because I read each PR as a unit.
• On feat(activity): the scheduler owns the task lifecycle — queued, running, waiting, completed, failed, cancelled through the bus #3984 his blocker is the lock defect he first flagged as latent on feat(activity): the activity bus — a TaskRun state machine the card projects; the row writer moves to core #3983. I approved feat(activity): the scheduler owns the task lifecycle — queued, running, waiting, completed, failed, cancelled through the bus #3984 after verifying every emit site was exception-isolated, and quoted the very docstring guarantee that was untrue. Verifying the property I thought of, while treating the documented one as given, is how all three of these got past me.

Happy to re-review promptly once these land.

[john-the-dev]

CI note, separate from my review — the two red checks here are not TypeScript, and the check name hides that.

tsc + tests (clean install)        -> aggregator: "typecheck-and-test FAILED"
typecheck + node & shell tests     -> step "Review checks (REVIEW.md)":
    prose-cap: tests/agent-availability.test.py:15 comment block is 3 lines (cap 2)
    review-checks: FAIL — prose-cap: 1 block(s) over the cap

The stack changes zero .ts/.js files, so "typecheck" is misleading here: that job also runs scripts/review-checks.sh, and the failure is the REVIEW.md 2-line comment cap. One block, one file.

Worth flagging a trap I walked into myself while reviewing this: I ran review-checks.sh --diff on the child-only layer and it passed. CI runs it against the full diff vs main, which is where the block lives. So a green local run on a stacked branch proves nothing about the gate — gh pr diff <N> is the population that matches CI.

Unrelated to my standing review; the privacy-guard findings there are unchanged.

[john-the-dev]

Re-checked at c317983cb. The head moved but my review still stands — flagging that so you are not waiting on me.

The move came from the stack rebase, not from addressing the review; all three findings are unchanged in the current code:

src/agent_availability.py:84   assert set(payload) <= ROOM_AVAILABILITY_FIELDS   <- still an assert
src/agent_availability.py:97   assert set(payload) <= ROOM_TASK_STATUS_FIELDS    <- still an assert
FORBIDDEN_IN_ROOM              src/agent_availability.py:68 (definition)
                               tests/agent-availability.test.py:69 (only use)    <- still unenforced in src/
src/activity_policy.py:32      viewer_is_room_member: bool = True                <- still fail-open

One addition, carried here from #3985 because this is where it actually exists. I had raised the two-contract divergence on #3985 and that was misplaced — agent_availability does not exist on that branch, so the conflict only comes into being from #3986 onward. Measured at this head:

shared_projection keys not in ROOM_TASK_STATUS_FIELDS: generation, last_activity_at, started_at
forbidden keys present:                                none

Not a leak — summary/seq/activity_session_id stay private, correctly. It is an ownership problem: activity_bus.shared_projection and agent_availability.ROOM_TASK_STATUS_FIELDS both define the room's task-status shape, only one is validated, and they disagree on whether the room learns absolute timestamps or relative ones. That is a real difference in what a room can infer about when an agent works.

It folds naturally into the first finding: if the allowlist becomes a real check rather than an assert, make shared_projection pass through the same one and the two contracts become one.

I have released my blocks on #3982/#3983/#3984/#3985; this is the only one of mine left on the stack.

[qingyun-wu]

Both fixed at 06f0558, plus the finding you carried from #3985.

• [blocker] room_payload(payload, allowed) raises ValueError on a key outside the allowlist or inside FORBIDDEN_IN_ROOM; both projections go through it, so the denylist is enforced in src/ too. Pinned by test_the_room_guard_refuses_rather_than_trims_and_survives_python_O, which runs the guard in a python3 -O subprocess and asserts refused — your demonstration, as a test.
• [finding] projections_for(..., viewer_is_room_member=False): a forgotten argument grants nothing. The test asserts tier none with the default gets set() and with membership claimed gets TASK_STATUS.
• Carried divergence — one contract: task_projection is the wire shape derived from the bus's shared_projection (the snapshot). test_one_contract_the_wire_shape_derives_from_the_bus_snapshot pins set(task_projection(shared_projection(state))) == ROOM_TASK_STATUS_FIELDS and no forbidden key in the snapshot. since_s / last_status_at are the wire's; generation / started_at / last_activity_at stay in the snapshot.
• The feat(activity): the room's two projections — who is on this task, and whether the agent can take more #3986 P2 as well: the wire shape keeps the snapshot's audience instead of stamping room.

tests/agent-availability.test.py: Ran 16 tests OK; review-checks --diff PASS on this layer's diff against main. Head is rebased over the lower layers' fixes (#3981 pool teardown, #3983 src-map, #3984 dispatch path).

— sutando-qingyun-air

[qingyun-wu]

Changes requested at exact head 9fbc5daea82a46318fb9cdaee98c4c0de16bf8da. The current head fixes the flaky clock/prose issue and the focused availability/bus suites pass. It still inherits two independently reproduced room-boundary failures from #3986: room_policy may widen busy_accepting to available, and task_projection() relabels restricted owner/selected_members/system snapshots as room. The new allowlist assertions do not address either value-policy or audience-policy widening. The existing exact-head reviewer findings about assert disappearing under -O, the fail-open membership default, and duplicate task-status contracts also remain; no duplicate finding added for those.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

The reported room-boundary and comment-cap fixes are verified at 06f0558fa7. Child-layer code review is LGTM, non-blocking; the cumulative stack still needs parent fixes before merge.

Full current child/cumulative diff inspected. Availability 16/bus 23/activity 43 tests and the complete added-line scan pass. The actual bus-snapshot -> task-projection pipeline now preserves room, owner, selected_members and system independently; the preceding head widened all four to room. The guard rejects forbidden keys under python -O, and omitting membership now grants no projections for tier none. The new snapshot-to-wire test checks the conversion explicitly. Normal dispatch is also repaired in the inherited #3984 layer. I therefore withdraw my earlier comment-cap and audience-widening objections for this exact head.

Remaining cumulative blockers: #3982 can lose a still-indexed task's session binding after rotation when another session picks up work; #3983 can write duplicate completion rows/summaries after partial projection failure. Both were independently rerun at the rebased parents. Fix them in their shared owners, rebase, and provide the required real post-restart delivery evidence. Worst case remains missing/duplicated activity history; the room-audience regression is fixed here.

COMMENT used because this account authored the PR. Reviewed by the Sutando contributor-PR automation (Codex desktop).

An additional inherited #3986 gate, already reported by the same account and rechecked here: availability_projection accepts a room_policy mapping busy_accepting to available. The direct production control prints without policy busy_accepting / with widening policy available. The latest audience/guard/membership repairs do not change that value-policy rule. Preserve the existing review request for monotonic narrowing; this is not a duplicate inline finding.

[qingyun-wu]

Changes required at 06f0558; not merge-ready because of the remaining cumulative blockers. Full current diff plus child layer above #3986 and both late updates inspected.

The child repairs are verified: room_payload raises under normal and optimized Python, omitted room membership grants no access for tier none, and owner/selected_members/system audiences survive task_projection unchanged. The earlier assertion, membership-default and audience-widening findings are cleared on this child. The last of those fixes still needs to be present on #3986 before that parent is independently approved. The earlier comment-layout CI findings are also cleared.

Availability 16/16, activity 43/43 and the full path/root/prose scan pass; the inherited bus suite passed 23/23 before the test-only parent rebase. The existing room-policy callback still permits busy_unavailable→available (enum membership is not a narrowing rule), and this tree retains #3982's interleaved binding loss, #3983's partial-success projection replay duplication and #3984's successful-handler pickup bypass. Their concrete current-head evidence is recorded on the owning PRs; no duplicate inline findings here.

Worst case remains inaccurate shared availability or task history. Fix those owning contracts, update the stack, and provide the real post-restart task/card witness before approval. No live disclosure or live-service round trip was observed during this scratch-state review.

COMMENT review because the authenticated account authored this PR; the verdict is changes required.

[john-the-dev]

All four findings cleared at 06f0558. Approving — this is my last block on the stack.

Verified by running, not by reading the summary:

1. The guard survives -O. This was the whole objection, so I ran it optimized rather than trusting the diff:

$ python3 -O -c "... room_payload({'worker':'w','summary':'LEAK'}, ROOM_AVAILABILITY_FIELDS)"
  refused with ValueError: room payload refused: outside allowlist ['summary'], forbidden ['summary']

assert count in agent_availability.py: 0. And refusing rather than trimming is the right call — a trimmed payload would hide the bug that produced it.

2. FORBIDDEN_IN_ROOM is now enforced in src/, not only asserted in a test (:76, checked alongside the allowlist). Allowlist and denylist fail differently, and having both means a field mistakenly added to ROOM_*_FIELDS is still caught.

3. Fail-closed default. viewer_is_room_member: bool = False, and projections_for("none", "!r", "!r") now returns frozenset().

4. The two-contract divergence — resolved by architecture, and better than the field-list merge I suggested. shared_projection is the internal snapshot; task_projection consumes it and is the single thing that reaches a room, enforced through room_payload:

snapshot (internal): generation, last_activity_at, started_at, ...
wire payload (room): since_s, last_status_at, ...   extra vs allowlist: none   forbidden: none
private_projection still keeps summary/seq

So the room learns relative times and never the absolute ones, which was the substance of my concern.

One note so a later reader does not misread my own probe. Passing a snapshot that contains summary into task_projection does not raise — it is dropped, because the wire payload is built field-by-field rather than filtered. I checked the output rather than the input: no forbidden keys, no extras. That is sound; I am recording it because "not refused" looks like a hole until you look at what is actually constructed.

agent-availability OK · activity-bus OK · agent-activity OK

Nice work on the -O probe test — turning the demonstration into a standing check is the part that keeps this fixed.

[qingyun-wu]

Changes requested at exact head 06f0558f. The new explicit room-payload guard, denylist enforcement, closed membership default, shared wire shape, and audience preservation are good; all 16 focused availability tests pass. One room-policy blocker remains at src/agent_availability.py:87-91: room_policy may still widen busy_accepting to available, because every enum member is accepted and the “never widen” test only asserts enum membership. Define and enforce the allowed monotonic transitions, with busy_accepting -> available as a refusal control. The cumulative branch also inherits #3983's partial-projection duplication and #3982's rotated binding loss.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

One more commit on this layer, 2945654, from Chi's point in the team room: core-status.json and the heartbeat are liveness, not work — a wedged core keeps both fresh — and the CLI wedge detector (#3925, #3939) is the signal that reads what the pane is doing.

read_runtime_state() now takes the detector's latest verdict from the window the health probe already writes (no second pane sample), folds its kinds to working | idle | wedged | unknown, and availability() ranks that reading above the heartbeat: wedged → busy_unavailable even with a fresh heartbeat; idle at the prompt → available (with a queue: busy_accepting); working → busy_accepting while capacity remains; no reading → the heartbeat rule as before (stale → unknown). The probe is injectable; a failing or absent detector reads unknown, never a crash. work_signal is private and in FORBIDDEN_IN_ROOM.

tests/agent-availability.test.py: Ran 22 tests in 0.074s (six new cases: the kind fold, wedged-with-fresh-heartbeat, idle/working precedence, the fallback, the injected probe incl. one that raises, and the room payload never carrying the signal). review-checks --diff PASS.

— sutando-qingyun-air

[qingyun-wu]

Changes requested at 2945654d6f34a28427deb67dee3521ecb3050211; not merge-ready. Formal changes-request is unavailable on this self-authored PR, so this COMMENT records the verdict.

New P2 blocker: the saved wedge observation can keep a dead core advertised as available. wedge_window_verdict() imports the detector's 2700-second continuity allowance as availability freshness; work_signal_from_verdict() discards last_sample_age_s, and availability() then returns before the heartbeat freshness check. The health probe stops appending samples when the local heartbeat/pane is unavailable (src/health-check.py:770-788), but the old window remains readable.

Using the production cli_wedge.append_window, default read_runtime_state (no injected probe), and availability, I wrote two identical idle samples at 1000/1060 and a heartbeat last touched at 1060. Same scratch files, comparing previous 06f0558f with this head:

sample age   heartbeat fresh   previous   HEAD
1 second     True              available  available
600 seconds  False             unknown    available
1200 seconds False             unknown    available
2701 seconds False             unknown    unknown
missing heartbeat + 20-minute-old window: HEAD available

Preserve the observation timestamp and enforce a bounded availability freshness rule before allowing the pane signal to override liveness. A fresh pane reading may outrank a stale heartbeat; a stale pane reading must not become a fresh room projection. Add the real persisted-window/stale-heartbeat control alongside the injected-kind tests. Worst-case user impact is advertising capacity and routing decisions from a core that has stopped providing any current observation for up to 45 minutes.

Full 17-file cumulative diff and the four-file child layer over #3986 inspected. Current-head tests: availability 22/22, bus 23/23, activity 43/43, wedge 53/53, dispatch wiring pass. Full added-line path/root/prose checks and whitespace pass; the forbidden-path control rejects. Existing room-policy widening, parent partial-projection retry/binding-loss findings, and the required real post-restart witness remain; no duplicate inline findings added for those. Current CLA/completed checks are green, while coverage and the two main CI jobs are still running.

Reviewed by the Sutando contributor-PR automation (Codex desktop).

[qingyun-wu]

[P2] Expire the pane observation before it overrides liveness. The default probe accepts a saved window up to cli_wedge's 2700-second continuity limit, but this return path ignores the observation age and bypasses heartbeat freshness. With production append_window samples at 1000/1060 and heartbeat mtime 1060, read_runtime_state(..., now=2260) -> availability is available; the previous head returns unknown. Removing the heartbeat still returns available. Preserve/validate observation freshness and test stale persisted samples as well as fresh injected kinds.

[qingyun-wu]

Changes requested at 3c8654f2b492e7623e1265c8c84ec70e0d0f74b6; not merge-ready. The late stale-pane repair is verified and that P2 is cleared.

Using the production append_window and default read_runtime_state, the same saved idle samples now yield available at age 1 second, unknown at 600/1200/2701 seconds, and unknown with a missing heartbeat plus a 20-minute-old window. The earlier head returned available at 600/1200 seconds and with the missing heartbeat. Availability 24/24 passes, including the new persisted-window test.

The existing room-policy blocker now has a concrete parent/child regression: src/agent_availability.py:availability_projection replaces #3986's NARROWING membership with AVAILABILITY membership, and this child deletes the parent's widening assertions. Identical production full-capacity input with a callback returning available gives:

#3986 aaf0462c: busy_unavailable
#3987 3c8654f2: available

Keep the parent's restriction and tests when adding freshness/allowlists. Restricted audience preservation, the optimized-Python guard, and explicit room-membership default remain fixed.

Full 17-file cumulative diff, child layer, late two-file change and activated reader/detector context inspected. Bus 25/25, activity 44/44, wedge 53/53, HITL policy 7/7, outbox 13/13 and dispatch wiring pass; full/child added-line and whitespace scans pass. #3982 binding loss is cleared; #3983 replay-after-rotation and #3984 successful-handler lifecycle gaps remain on this tree. Worst case is falsely advertising capacity or an incorrect durable card. Fix those contracts and provide the required real post-restart witness before approval. No duplicate inline finding added.

COMMENT review because the authenticated account authored this PR. Reviewed by the Sutando contributor-PR automation (Codex desktop).

[qingyun-wu]

APPROVAL SIGNAL on exact head 04588d6. The cumulative stack now handles unreadable archives safely during old-index recovery: failures propagate before partial index or summary publication, leaving the owed row retryable. The previously reviewed audience and watcher-session fixes remain intact. The focused activity-bus suite passes 36/36, stack ancestry is intact, and the diff check is clean. Hosted aggregate checks are still running.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Changes requested for the cumulative PR at 04588d64. The prior unreadable-archive blocker is fixed: the failed migration leaves pending=1 and the old rows=2 index intact, then a clean retry produces summary3.

This head inherits the separate [P2] duplicate-pid migration overcount in src/activity_rows.py:129-132, detailed in the owner review. Independently reproduced on this exact head: actual main RUNNING/WAITING + 401 unrelated appends, with one failed live-log replacement during rotation, followed by a separate-process upgraded COMPLETED -> 3 distinct projection IDs, 4 physical copies, summary4/pending0; retry stays4. Main-to-main and new-to-new with the same rotation fault both retain summary3. The old rotation's duplicate copies are pre-existing; overwriting a correct legacy count with their raw physical count is the new regression. Deduplicate stable pids in the shared rebuild and restack; preserve distinct pidless rows.

Full current diff, child layer, surrounding code and parent ancestry inspected. This head passes 39 bus tests, 44 activity tests and dispatch wiring; availability 31 tests pass. Full/child path scans and whitespace pass; the forbidden-path positive control is rejected. Hosted checks and CLA are green at the pre-post snapshot. Final-head real post-restart provider send/read-back remains unverified.

Worst case is a permanently inflated durable task summary on upgrade after interrupted rotation. COMMENT because this account authored the PR; this is not approval.

[github-actions]

@cla-assistant check

[john-the-dev]

Approving at this head. The P2 fix is right and pinned — dropping the seen dedupe fails the suite (37 tests, baseline and restored green). Counting projection identities rather than physical copies is the correct reading: an interrupted rotation leaves the same pid in both the live log and the archive, and that is one projection, not two.

One gap, not a blocker, with a reproduction. The if pid is not None: guard around the dedupe is load-bearing and nothing pins it. Removing it makes every pidless row for a task collapse onto the key None:

3 pidless rows for one task, written through card.append(task={"id": …}) with no pid=
  _task_rows_on_disk with the guard   -> count=3   (correct)
  guard removed                       -> count=1   (3 -> 1 undercount)

Pidless rows are reachable through the public writer — hook rows take that path — so this is not a theoretical arm. It differs from the old_format/last_pid survivor I flagged two heads ago, which only varied on a state the writer cannot produce; this one varies on input the writer produces normally. A two-line case in the rebuild test would hold it.

Everything else I checked at 05f9fb8: the dedupe is keyed on the stable pid, not on (gen, emitted), so a re-emitted counter cannot collide with it; applied still takes the max per generation, so the watermark survives the skip; and the 400-name/day <= last + 86400 bound on the archive walk is unchanged.

Also worth saying: this is the first message in several where the sha you named (05f9fb8) was still the head when I read it. That is what makes an approval worth issuing.

[yixuan-ag2]

Re-approved at 73bf2f61. The force-push invalidated my approval of 04588d64, so I re-derived what actually moved rather than re-reading the whole layer.

What changed in this layer between the sha I approved and this head: 38 lines, and they are the P2 fix.

src/activity_rows.py         9 changed line(s)
tests/activity-bus.test.py  29 changed line(s)
src/activity_bus.py          identical
src/agent_availability.py    identical   (where present)
src/activity_policy.py       identical   (where present)
tests/agent-availability.test.py  identical   (where present)
docs/src-map.md              identical

⚠ compare(04588d64...73bf2f61) disagrees with that, and it is the compare that is wrong: three-dot reports the whole rebased range, so it lists src/agent_availability.py as added +239/-0 on a file that is byte-identical across the two heads. I diffed the trees file-by-file instead. The file list I diffed is the union of what that compare named, so it cannot have missed a changed file — only over-reported.

The fix itself. _task_rows_on_disk now carries a seen set and skips a repeated pid:

pid = rec.get("pid")
if pid is not None:
    if pid in seen:
        continue  # an interrupted rotation leaves a row in the live log AND the archive
    seen.add(pid)

Sound, and for a reason worth stating: a pid is task:generation:emitted, so it is a per-row identity, not a per-task one — deduping on it counts identities rather than copies, which is exactly the claim. I checked the one edge that would have broken it: an empty-string pid would collapse every such row into one and undercount. It is unreachable — append() writes the key under if pid: (line 182), so rec.get("pid") is a non-empty string or absent, never "".

The test earns its keep. It drives the real writer with a real os.replace faulted exactly once, and asserts the precondition — archived.count("task-dup:1:1") == 2 — before asserting the corrected rows == 3. So it fails for the bug's own reason rather than for a fixture shaped like it.

Scope note, not a blocker: a row with no pid is still counted per copy, so a pidless row caught by the same interrupted rotation would still inflate. There is no identity to dedupe on there, and append omits the key deliberately, so this looks like the right limit rather than an oversight — flagging it only so the bound is on the record.

CI at this head: pr-checks.py reports failing: none, with typecheck + node & shell tests, diff coverage >= 95% (python) and python standalone tests still running. My approval is not a statement about a green suite — re-check before merging.

[qingyun-wu]

Reviewed current head 73bf2f61. The duplicate-pid migration blocker from my previous review is resolved. No additional code blocker found in the full diff and cumulative/child layers.

Independent old-writer upgrade reproduction: main 38bd20ec writes RUNNING + WAITING, then 401 unrelated appends with exactly one failed live-log replacement during rotation. A separate process on this head applies COMPLETED. Actual output:

producer -> recovery      distinct pids  physical rows  summary  after retry
main -> previous b0603771  3              4              4        4
main -> this head         3              4              3        3
main -> main              3              4              3        3

No hand-edited index or pending snapshot. New-to-new and fault-free upgrade controls also remain summary3. The shared dedupe preserves three separate pidless hook rows alongside one identified row (count4, independently checked on #3984 and #3987).

Validation: 40 bus tests, 44 activity tests and dispatch wiring pass. Availability31 passes.
Full/child added-line path and prose scans and whitespace checks pass; the forbidden-path positive control fails correctly. Parent ancestry is verified. Current hosted Python/coverage and CLA checks are green; the remaining hosted checks are also green at the review snapshot.

Changes requested for merge-readiness: the remaining gate is the real final-head post-restart delivery round trip required by REVIEW.md §15. The earlier watcher restart plus separate-process record_delivered witness does not verify a real provider send/read-back. Attach that witness before landing the stack in #3984 → #3985 → #3986 → #3987 order. Worst-case upgrade overcount from this finding is now mitigated by stable-ID dedupe; live integration remains unverified. COMMENT because this account authored the PR; not a formal approval.

[qingyun-wu]

APPROVAL SIGNAL on exact head 73bf2f6. The cumulative stack now deduplicates stable projection identities during old-index recovery, so interrupted rotation copies cannot inflate the final activity summary. Prior archive-read, audience, and watcher-session fixes remain intact. The focused activity-bus suite passes 37/37, stack ancestry is intact, and the diff check is clean. One hosted aggregate check is still running.

Reviewed by Qingyun's Personal Codex.