Repository navigation
pool: rescue live-tree state + assignment fairness + binding room affinity - #3413
Conversation
The pool hardcoded one runtime end to end: install-core-pool.sh resolved
`claude` and injected POOL_CLAUDE_BIN, and pool-core-wrapper.sh started
`claude --dangerously-skip-permissions ... -- /proactive-loop-pool`. The
repo already models pluggable runtimes (src/agent/start-cli.sh dispatches
on `sutando-config.sh core-runtime` over a claude|codex allowlist), and
skills/proactive-loop-pool/CODEX.md already documents the Codex entry —
the pool was the one component that bypassed both.
Adds a per-core runtime dimension, reusing that allowlist:
install-core-pool.sh 3 --core-runtime=3:codex # one core
install-core-pool.sh 3 --runtime=codex # the whole pool
`$SUTANDO_POOL_RUNTIME` sets the default; the flag wins. An unsupported
name exits 2 like start-cli.sh does — never a silent fall back to claude.
The installer resolves the runtime's absolute binary plus the Codex config
store (`core-config-dir-{env-name,value} codex`) and injects POOL_RUNTIME,
POOL_RUNTIME_BIN, POOL_RUNTIME_CONFIG_ENV, POOL_RUNTIME_CONFIG_DIR. The
wrapper dispatches on POOL_RUNTIME; unset means claude reading the legacy
POOL_CLAUDE_BIN, so a plist written before this change still starts.
A Codex follower is the same persistent shape — an interactive CLI in tmux
`core-N` under launchd KeepAlive — with the pool-mode entry riding codex's
[PROMPT] positional. It differs in wake-up: no session CronCreate and no
pool-mode notifier, so the wrapper's nudge is its only sweep and runs at
300s rather than claude's 1800s. CODEX.md records that, and records that
kick-pool.sh still speaks only Claude's REPL.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… owner kick-pool's staged-input test was negative — "if the pane shows a prompt with content, skip" — so any pane it could not parse fell through to typing Claude's slash command. Claude Code renders its prompt separator as U+00A0, so the ASCII-space regex matches neither its idle nor its staged line and the guard is already dead on claude; a codex follower uses U+203A, so it vanished there too and the watchdog appended /proactive-loop-pool pass into a box that already had one. Recognition is now positive: type only on that runtime's idle prompt. Per-runtime driving policy gets one owner, scripts/pool-runtime-drive.sh — runtime resolution from a plist, prompt/idle/menu/busy markers, nudge text and submit key, and the busy -> menu -> staged -> type -> submit sequence. The wrapper's inline send_nudge is deleted and both adapters bind the library; kick-pool keeps only session enumeration, launchd revival and tmux binding. Also makes turning one codex follower on and off a supported operation: --only-core installs or refreshes a single core without booting out the lead or the other cores (and ends the old session when the runtime changes, which the wrapper's has-session reuse would otherwise keep alive), and uninstall --only-core removes the plist, the session and the stale state/cores/core-N.alive that would keep the lead assigning to a ghost. Claude-path invariance verified against the parent commit with a stub-tmux argv recorder: launch argv, wrapper nudge keystrokes and kick-pool on idle and busy claude panes are byte-identical; only the staged-input pane differs, which is the defect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s busy P1 from the review on 6855ac6. `pool_drive_nudge()` checked only the busy marker, so staged user text, a startup dialog, a garbled capture and any future TUI all fell through to `pool_drive_send()`. `pool-core-wrapper.sh` drives that path every 300s, so a codex follower could get the whole pool prompt appended to its staged input even though the kick path fails closed. It also captured without -e, so it could not have told codex's dim placeholder from real text. One classifier now: `_pool_drive_classify()` returns busy|menu|unrecognized| staged_entry|staged_other|idle, and BOTH drive paths branch on it — the nudge sends only on a positive `idle`, and an unclassified state defers rather than types. Claude's nudge stays unguarded, as the review allowed: its input is a durable queue. The tests encoded the gap rather than caught it. `test_codex_sweep_types_the_codex_entry_when_free` fed `> ready` — staged text — and asserted a send. Replaced with a true-ANSI-idle positive plus four controls (staged text, startup dialog, garbled capture, empty pane) that assert silence. Mutation: forcing state=idle types the full pool prompt three times into staged text and into the dialog, which is the reported harm reproduced. Second issue, `--only-core` with an explicit N. My first fix rejected any N and broke `test_only_core_outside_the_pool_is_an_error`, which passes an N that AGREES with the installed size — so blanket rejection was wrong and the existing test is what showed it. Now validates: the installed size is inferred from the plists ALONE (seeding it with $ONLY_CORE made "installed" depend on the core being requested) and a disagreeing N is refused. Mutation-controlled. pool-runtime-drive (31), pool-follower-beat (10), pool-installer (29): all OK. Piped-diff review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pool had no duration data. done-flag mtimes gave a partial history by accident; nothing recorded source, and nothing survived archive cleanup. Without it "is N=3 better than N=1" is unanswerable. finish_task() takes an optional metrics_path; the CLI passes data/pool-metrics.jsonl (the convention call-metrics/voice-metrics already use). One line per completed task: task_id, core, source, arrived_at, finished_at, duration_s. Arrival comes from the claimed file's mtime, which survives the assign/claim renames, so duration is real rather than inferred. Read before the archive move takes the file away. assigned_at/claimed_at are deliberately absent: the lead's renames preserve mtime, so those timestamps do not exist at finish time and a field that cannot be populated honestly is worse than no field. Fail-open — a metrics write must never turn a delivered answer into a failed task. Covered by a test that points it at an unwritable path; mutating the except to `raise` reds that test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n note lead-follower-pool.md described channel affinity in one line. Document the handler file and the two INDEPENDENT gates a claim passes -- freshness (last_handled_at, 30 min) and liveness (.alive mtime, 90s) -- plus the fact that crash recovery does not depend on the idle threshold. Records one defect rather than hiding it: last_handled_at is written only on a successful claim (claim_task.py), and finish() never touches it, so the stamp says when a core TOOK work, not when it finished. A core answering a long thread can lose its own affinity mid-thread. server-and-identity.md is new: motivation for the Sutando Server and the read-only identity surface, sourced from the modules' own stated contracts. Carries a naming caveat up front -- "ID card" appears nowhere in the repo, so the mapping to identity_view.py is an interpretation the reader should confirm before building on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…at/pool-operability
Three claims had gone false. The skill's "Phase 2a known limitations" said the done-flag side-effect gate and the orphan watchdog were NOT wired in, and that a stranded claim needed the owner to rename a file by hand. Both landed since: pool_lead.reclaim_dead/reclaim_claimed/reclaim_stuck_assignments run every lead pass, and reclaim_claimed gates on done-flag AND result evidence -- a flag alone means a crash before the result write, so the task repools instead of counting as delivered. Replace the section with what the code does, and keep the one limitation that IS still real: recovery is the lead's job, so a dead lead stops reclaiming. The installer's closing banner announced that /proactive-loop-pool was "NOT in this PR" and told the operator to stay at N=1 -- printed immediately after installing N cores that run that skill. Replace it with the per-core runtime it just installed. Same for the skill-missing error, which explained the absence as by-design rather than as the broken symlink it now is. The skill opened with "N parallel claude sessions"; a pool can mix claude and codex per core. Also documents what was undocumented: data/pool-metrics.jsonl (the only durable record of pool timing -- results are deleted on delivery and archive mtimes are arrival times), why assigned_at/claimed_at are absent, and that --only-core cannot resize a pool, so adding a core is a full install. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…to guess
CODEX.md's acquire snippet read `WS = Path(WORKSPACE)` and
`f"core-{CORE_ID}"` with neither name defined anywhere in the file. A
Codex follower had to invent both.
Observed on the first live codex core: core-4 was assigned
task-workstream-grouping-1787620687695, stayed alive and swept on
schedule, and reported "found no assigned work" while the file
`.assigned-core-4.txt` sat unclaimed in the real queue. The documented
default `<repo>/workspace` does not exist in this checkout and the cwd
is the repo, so a guessed path makes tasks/ unreadable, acquire_work
returns None, and an idle report is indistinguishable from a healthy
one.
The plist already exports POOL_WORKSPACE and SUTANDO_CORE_ID. Name
them, keep sutando-config.sh as the fallback, and add the one check
that separates "queue is empty" from "I am looking in the wrong place".
Verified by running the documented snippet verbatim: it resolves the
configured workspace and lists core-4's pending assignment, which the
previous text could not have found.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This tree is the RUNNING pool — the lead daemon (pid 80416) and all four follower sessions have their cwd here — and it carried 228 lines of uncommitted work across 11 tracked files plus an untracked hook. That work was executing in production while existing only on this disk. Committed verbatim, no edits, so it survives any subsequent update of this tree to origin/feat/pool-operability (59 commits ahead, colliding on 10 of the 12 paths). Notable content preserved here and absent from origin: - pool_lead._pick gains a runtime_fn filter so lane_core is the highest CLAUDE core, keeping maintenance off a codex follower that only sweeps on a timer. This is why core-3 is the routine lane on the live system while origin would make it core-4. - hooks/emit-tool-activity.py (untracked, 174 lines). Working tree contents are byte-identical before and after (verified by md5 on pool_lead.py); branching and committing changed no running code.
Idle-load ties broke lexicographically, so core-1 took every cold assignment (measured 50% of all assignments); equal load now goes to the least-recently-picked follower. Owner lane restricted to claude seats - a codex follower is turnaround-slow, so it keeps the routine lane and saturation overflow only. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
… its core lives Mid-conversation core switches lose the room's context (owner report 2026-08-26). Affinity now binds while the home core is alive: no idle expiry (AFFINITY_IDLE_S removed), no busy yield. The room moves only on core death or an unclaimed-assignment reclaim, which now also deletes the binding and re-stamps the replacement core as the new home. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
Changes requested at exact head [P1] The new activity hook is not wired into any installed settings path. The cumulative diff adds [P1] The hook also reimplements workspace fallback policy and can write to the wrong tree. [P1] The stated stack boundary is not mergeable. Git reports base Focused evidence at this head: pool assignment 17/17 and lane routing 15/15 passed; follower/runtime suites were still running when the review window closed. Repository hardcoded-path/root-artifact checks pass. These green unit paths do not exercise hook installation or configured workspace routing, which is why the two hook defects remain invisible. Worst-case disruption is a rescue merge that silently drops observability on fresh installs while writing mutable feed state into a second workspace, compounded by a non-mergeable stack. Rebase/retarget first, then wire the hook through the canonical settings and workspace owners with direct tests. Reviewed by Qingyun's Codex automation. |
A routine task tagged with a room's channel stamped the lane core over the owner's binding, turning a one-off misroute into a sticky steal (measured: Feature Haul row flipped core-1 -> core-3 at the exact ts of a routine assignment). The sweep now stamps affinity only for owner- lane picks landing off the lane core; lane-core selection extracted to _lane_core_of so sweep and _pick answer the same question. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
Forensic aid for the 2026-08-26 sticky-steal (cause unproven: two owner-lane room picks landed on the lane core with no surviving liveness evidence). The sweep now appends a JSONL line whenever the live follower set changes, and an anomaly line whenever an owner-lane pick lands on the lane core or fires while the room's bound core is absent from the live set. Change-driven, 5MB one-deep rotation, and a trace failure can never break assignment. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
Blocking review comment on exact head [P1] Binding room affinity is broken by an unrelated follower topology change. [P1] A pool with one Claude seat and one Codex seat routes owner work to Codex despite the stated policy that Codex receives nothing while any Claude seat lives. I reproduced both failures directly on the exact production Reviewed by Qingyun's Personal Codex. |
|
Additional exact-head blockers at [P1] The child adds an unindexed canonical document and currently fails the documentation audit. [P2/blocking repository rule] Three added Python comment blocks exceed the two-line cap. The shipped 18 assignment, 15 lane-routing, and 4 liveness-trace cases pass, and Reviewed by Qingyun's Codex automation. |
An owner pin to the lane core or a codex follower was silently ignored because the binding check required membership in the claude-only owner primary. Bindings are now always deliberate (the sweep can no longer stamp the lane core), so a row naming ANY alive follower is honored — including codex, when the owner explicitly chooses it. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
…d ones The 300s unclaimed-reclaim conflated busy with wedged: a core mid-task claims its next assignment when it frees up, but reclaim repooled the assignment, released the room's binding, and re-stamped another core — moving bound rooms off their context core exactly when they were most engaged (measured: Feature Haul tasks assigned core-1 at t, re-assigned core-2 at t+304s while core-1 held claims). A core with claimed work in flight now keeps its assignments (ledger clock resets); the reclaim fires only for cores that are idle yet not claiming — the actual wedge signature. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
Blocking follow-up on exact head The topology-transition affinity fix is correct: an existing room binding now wins while its home remains in the live follower set, including when that home becomes the routine-lane core. The new busy-core reclaim control also passes. [P1] The separate one-Claude/one-Codex routing blocker remains unchanged. With live The other exact-head gates also remain red: Reviewer verification: assignment 19/19 and lane routing 16/16 pass; the explicit single-Claude control fails as above; Reviewed by Qingyun's Personal Codex. |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Additional blocking finding at exact head 0090886ca01717235f9e5d68e4044bc623e9706e; this is separate from the routing, hook/workspace, docs, comment-cap, and stack blockers already recorded on this head.
[P1] Any claimed file now disables stuck-assignment recovery for that follower indefinitely. At src/runtime-api/pool_lead.py:312-316, _claimed_load(instance) > 0 is treated as proof that the follower is merely busy, and every expired assigned task has its ledger timestamp reset. But follower liveness is bound to the tmux pane PID, not to turn progress: a session wedged mid-turn can keep both its heartbeat and its .claimed-core-N.txt file forever. reclaim_dead() and reclaim_claimed() also skip a live follower, so there is then no recovery path for that core's queued assignments or bound rooms.
An exact production-function control kept one stale claimed file and one expired assigned file on a live follower. Three sweeps at t=1000, 2000, and 3000 each returned reclaimed=[], left the assignment in place, and advanced its ledger timestamp to the current time. Removing only the claimed file was the positive control: the next expired sweep returned ['task-wait.assigned-core-1.txt'] and restored the canonical task. The instrument therefore reaches the reclaim path; the claimed-file predicate is what suppresses it.
Please distinguish demonstrable progress from mere claim presence, or bound the postponement so a live-but-wedged claimant cannot renew every queued assignment forever. Add a repeated-sweep control with a live heartbeat, an old claimed task, and an older assigned task that proves the queue eventually recovers without moving work from a genuinely progressing turn.
Worst-case disruption is one wedged pane freezing every queued task and room assigned to that core indefinitely while the pool continues to report the follower as alive. The current 19 assignment, 16 lane-routing, 4 liveness-trace, 32 runtime-drive, and 26 gateway-orphan tests pass, but they do not cover this adjacent input. Docs audit still fails four index checks and cumulative repository checks still fail four prose-cap blocks. This head is not ready to merge.
Reviewed by Qingyun's Codex automation.
The trace's anomaly condition predates explicit-binding-wins: an owner pick landing on the lane core is now legitimate whenever that core IS the room's bound instance, but the condition flagged it anyway — two false positives in the first hour, which trains readers to ignore the event. Anomaly now requires inst != bound alongside the lane-core clause; the dead-bound-core clause is unchanged. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
Review P1 on 0090886: resetting the ledger ts while any claimed file exists made the busy state self-renewing, so one stuck claim disabled stuck-assignment recovery for that follower indefinitely. The ledger ts now survives deferral and BUSY_DEFER_MAX_S (30 min) caps it: a genuinely busy core keeps its assignments and rooms, a wedged one yields them once the cap passes even though the claim file is still present. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
P1 (claimed file disables stuck-recovery indefinitely) — fixed at 15e44b6. Valid finding: the ledger reset made busy self-renewing. Now the ledger ts survives deferral and |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Blocking follow-up at exact head 15e44b6142474460032f364c089e2ae9e849da35 — still not ready to merge.
The bounded busy-deferral fix is correct and clears my 0090886c finding. An exact production control preserved the original ledger timestamp and kept the queued assignment within BUSY_DEFER_MAX_S, then repooled it exactly at the cap while the claimed file and live heartbeat remained. The new committed cap test passes.
[P1] The separate one-Claude/one-Codex routing blocker is unchanged. With core-1=claude and core-4=codex, _lane_core_of() still selects the sole Claude because it keys lane creation on total follower count. _pick() then removes that only Claude from the owner candidate set and falls back to Codex. The exact production control still returns owner → core-4, while routine work returns core-1, contradicting the stated policy that Codex receives no owner or routine work while a Claude seat lives. Add the explicit two-runtime boundary controls and base lane creation on eligible Claude seats (or otherwise retain the sole Claude for owner work).
The other exact-head gates remain: docs audit reports both lead-follower-pool.md and server-and-identity.md absent from both canonical indexes; cumulative review-checks fails four over-cap comment blocks; the new activity hook remains unwired and reimplements noncanonical workspace fallback; and GitHub still reports the divergent stacked head DIRTY because the declared base is not an ancestor. Those areas were not changed by this four-file delta.
Reviewer-run evidence at this head: assignment 20/20, lane routing 16/16, and liveness trace 5/5 pass; the bounded-deferral positive/negative control passes; the mixed-runtime owner control fails as above; git diff --check passes. Worst-case disruption remains owner traffic silently moving to a turnaround-slow Codex seat, plus a non-mergeable rescue branch that drops observability or writes feed state outside the canonical workspace on fresh installs.
Formal request-changes is unavailable because this reviewer identity owns the PR.
Reviewed by Qingyun's Codex automation.
|
Blocking follow-up on exact head The two-commit delta is sound in isolation: an explicit binding to the lane core is no longer traced as anomalous, and busy-core deferral now retains the original ledger timestamp so a wedged core yields at the 30-minute cap. Assignment tests pass 20/20. The existing P1 still reproduces unchanged: with one live Claude and one live Codex follower, owner work is assigned to Codex ( The branch also still fails Reviewed by Qingyun's Personal Codex. |
…ane core With one claude and one codex follower, the owner-lane fallback chain skipped 'any claude seat' and handed owner work to codex: the sole claude was removed as lane core, and the next fallback was any non-lane follower. Owner primary now falls back to the full claude set (lane core included) before ever reaching a codex seat. Controls pin both lanes on the one-claude/one-codex pool. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
One-Claude/one-Codex P1 — fixed at 780bcbd. The owner-lane fallback chain skipped 'any Claude seat' and fell to any non-lane follower, which with a single Claude meant Codex got owner work. Owner primary now falls back to the full Claude set (lane core included) before any codex seat; both requested controls added ( |
docs_audit failed with four missing-index errors: both design docs were committed without README index lines or catalog.json metadata. Audit now passes (38 indexed / 41 checked). Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
docs_audit blocker cleared at 1d9d996 — both design docs now carry README index lines and catalog.json metadata; |
|
Blocking follow-up on exact head The one-Claude/one-Codex routing blocker is fixed correctly. The sole Claude now remains owner-eligible while also serving routine work, and the two new production-sweep controls pass. Assignment passes 20/20 and lane routing passes 18/18. The head remains blocked on the unchanged branch-level findings: The routing delta itself has no new blocker and Reviewed by Qingyun's Personal Codex. |
…solver Two review P1s: (1) emit-tool-activity.py was never registered — a clean install produced no /verbose feed. The settings builder now takes the hook path as arg5 and registers it as PostToolUse on every tool; start-cli passes it. (2) the hook's private workspace fallback (<repo>/workspace, then ~/workspace) could strand activity-feed.jsonl outside the tree the server reads. It now uses the canonical resolver only and exits without writing when resolution is unavailable. Wiring, omitted-arg, configured-workspace and no-repo/no-write tests added. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
Both hook P1s fixed at 49013bc (and docs_audit was already cleared at 1d9d996 — the 11:45 review predates that head). (1) Registration: |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Changes requested on exact head 49013bc7db0ef2ea937a3eb7a05da0c10fdd784a; this head is not ready to merge.
The latest three commits correctly fix the one-Claude/one-Codex owner routing, index both design docs, and wire the activity hook through the canonical settings builder/resolver. Reviewer runs passed lane routing 18/18, activity-hook wiring 5/5, docs audit (38 indexed / 41 checked), and git diff --check.
[P2 blocker] tests/activity-hook-wiring.test.py:49-62 is not a valid positive control and can mutate a developer's live workspace. It runs the real hook against this checkout, resolves whatever sutando.config.local.json selects, appends a synthetic activity record there, then asserts only that activity-feed.jsonl exists. After one normal run created the file, I replaced main() with an immediate return 0; test_writes_into_the_resolved_workspace still passed. Isolate the resolver to a temporary configured workspace, assert the newly appended record/content or byte growth, and clean up without consulting the host workspace.
Two previously reported branch-level blockers also remain. The cumulative repository check fails five prose-cap blocks (hooks/emit-tool-activity.py:38,83; src/runtime-api/pool_lead.py:79,167,174), and GitHub still reports the stated stacked head as conflicting because #3319's current head is not an ancestor of this branch. Reconcile the child boundary and rerun the cumulative checks.
Worst-case disruption is a merge from a non-reviewable stack plus tests that forge owner-visible activity in a configured live workspace. Formal request-changes is unavailable because this reviewer identity owns the PR.
Reviewed by Qingyun's Codex automation.
|
Blocking follow-up on exact head .\n\n[P1] is not hermetic and can mutate a developer's live configured workspace. It executes the production hook from the real checkout with the ambient , then resolves that same ambient workspace and merely asserts that exists. On any checkout configured to a live workspace, the test appends a fake Bash activity event to production state. The adjacent no-repo test also asserts that does not already exist, so it can fail solely because unrelated user state is present. Build a temporary fixture checkout/config whose canonical resolver points at a temporary workspace, assert the event there, and assert no files outside that fixture changed.\n\nThe hook registration and fail-closed resolver behavior themselves look coherent. I ran the three non-mutating wiring controls (3/3), assignment (20/20), lane routing (18/18), docs audit (5/5), and ; all passed. I deliberately did not run the full activity-hook test because its current contract permits the live-state write above.\n\nTwo existing merge gates also remain: GitHub reports this head as conflicting, and the cumulative repository review check still fails five prose-cap violations in and . The branch boundary and review-check contract must be reconciled before merge. Formal request-changes is unavailable because this reviewer identity owns the PR.\n\nReviewed by Qingyun's Personal Codex. |
|
Correction: the immediately preceding comment was damaged by shell interpolation. This comment supersedes it. Blocking follow-up on exact head [P1] The hook registration and fail-closed resolver behavior themselves look coherent. I ran the three non-mutating wiring controls (3/3), assignment (20/20), lane routing (18/18), docs audit (5/5), and Two existing merge gates also remain: GitHub reports this head as conflicting, and the cumulative repository review check still fails five prose-cap violations in Reviewed by Qingyun's Personal Codex. |
… semantics Reconciles the stack boundary review blocker on #3413. Both sides kept: theirs' no-claim cooldown (a repooled task must not boomerang to the follower that ignored it), full-file header parsing (2048-byte cap made late fields invisible), and the routine-lane wedge escape; ours' seat rotation, binding room affinity, bounded busy deferral, runtime-aware lanes, liveness trace, and explicit-pin-wins. All 50 pool/bridge suites and docs_audit pass on the merged tree. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
Clears the cumulative prose-cap finding (5 blocks) from review. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
Stack-boundary blocker cleared at c6cd046: merged |
|
Blocking follow-up on exact head [P1] The activity-hook test isolation blocker remains unchanged. Please bind the canonical resolver to a temporary fixture checkout/config and workspace, verify the event only there, and prove no ambient workspace changed. I again skipped the two mutating activity cases intentionally; the three non-mutating wiring controls pass 3/3. Exact-head pool assignment passes 30/30, lane routing 24/24, docs audit 5/5, cumulative review checks, and Formal request-changes is unavailable because this reviewer identity owns the PR. Reviewed by Qingyun's Personal Codex. |
…lock The busy deferral paused enforcement but not the clock: a home core that spent the whole unclaimed window working its room's earlier tasks was repooled the instant it went idle — zero time to claim the next task — and the binding release moved the room off its context core (measured live: repool at 302s while the core drained the queue sequentially). Busy sweeps now rewrite the ledger ts so a 60s claim window survives the busy spell; the wedge cap anchors to the assigned file's own age, so a live-but-wedged claimant still yields at BUSY_DEFER_MAX_S. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
New finding from live operation, fixed at 760673e: the busy deferral paused enforcement but not the clock — a home core that spent the whole 300s window working its room's earlier tasks got repooled the instant it went idle (measured: repool at 302s while the core drained its queue sequentially), releasing the binding this stack exists to protect. Busy sweeps now leave a 60s post-busy claim grace (ledger rewrite), and the wedge cap anchors to the assigned file's own age so a live-but-wedged claimant still yields at the 30-min cap. New control |
…checkout Review P1: the test ran the production hook against the checkout's ambient canonical workspace, appending a fake Bash event to the real activity feed on every run. It now builds a fixture repo skeleton (real resolver modules + the sutando.config.json anchor the root-finder keys on) so the hook exercises the genuine resolution path and the feed lands in the fixture tree, asserted by schema. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
Activity-hook test-isolation P1 fixed at 3c4a238. |
|
Blocking finding on exact head [P1] The new 30-minute busy cap is anchored to the assigned file's mtime, but assignment is an atomic rename and does not refresh that mtime. A task that waited in the queue for more than 30 minutes therefore looks cap-expired as soon as its ordinary 5-minute claim window elapses. I reproduced the production path with a task queued for two hours: after assignment, a busy core, and 301 seconds, Track an immutable assignment-start timestamp rather than the task file's arrival mtime, or explicitly stamp a separate assignment-age source at rename time while preserving any arrival-time semantics consumers need. Add a control where the pending task is older than The harmful activity-hook test write is fixed: the full wiring suite now passes 5/5 against a fixture repository. Pool assignment passes 31/31 and lane routing 24/24. The cumulative review check currently fails one three-line comment block at Formal request-changes is unavailable because this reviewer identity owns the PR. Reviewed by Qingyun's Personal Codex. |
Review P1: the cap anchored to the assigned file's mtime, which the atomic assignment rename preserves from queue time — a task that waited longer than the cap before assignment was repooled the moment its claim window elapsed, skipping the busy deferral entirely. The sweep now stamps the assigned file's mtime at rename, making file age a true assignment age. Control: a two-hour-queued task on a busy core survives the 301s sweep. Also folds the 3-line constant-comment block and drops the hook test's ambient home-directory assertion. Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
|
@cla-assistant check |
|
Assignment-age P1 fixed at 6423136 — correct catch: the atomic rename preserves arrival mtime, so a task queued past the cap skipped busy deferral entirely. The sweep now stamps the assigned file's mtime at rename ( |
|
Approval signal for exact head The assignment-time fix is sound: after a task is moved into Focused activity-hook, pool-assignment, and lane-routing tests passed (61/61 total), cumulative review checks passed, and Reviewed by Qingyun's Personal Codex. |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Changes requested on exact head 6423136d59e38fa0e007c10f543b3e56f8e295fd; the assignment-age fix introduces one publication race.
[P1 blocker] src/runtime-api/pool_lead.py:237-249 publishes the assigned filename with os.rename() and only then calls os.utime(target) inside the same try. The follower watches that directory and may claim the file between those two calls. In that normal race, utime(target) raises FileNotFoundError, the broad handler continues, and the lead skips the successful assignment's affinity binding, metric, and returned assignment record.
I reproduced the exact production path by making the claimant rename the just-published target during os.utime:
out []
files ['task-race.claimed-core-a.txt']
affinity {}
The task is already claimed, but the room has no home binding, so its next message can be routed to another core and lose the context continuity this PR is meant to guarantee. Stamp assignment time before publishing, or otherwise make the timestamp and publication one contract; do not treat a post-rename timestamp failure as if the rename failed. Add a control where the follower claims immediately after publication and prove the assignment is still reported/bound.
The stale-queue regression itself is fixed, the ambient hook assertion and prose-cap failure are cleared, and activity-hook 5/5, pool assignment 32/32, lane routing 24/24, cumulative repository checks, and git diff --check pass. Worst case is a fast follower processing the task while the lead silently omits the durable room binding, moving subsequent owner context across cores. Pre-publication stamping or an equivalent atomic assignment record is the mitigation.
Formal request-changes is unavailable because this reviewer identity owns the PR.
Reviewed by Qingyun's Codex automation.
…ecord The previous wording said GitHub permits no formal review at all on a self-authored PR and that pulls/N/reviews is empty by construction. Both are false, and #3413 -- the instance the claim was grounded on -- disproves it: that endpoint returns four qingyun-wu COMMENTED reviews. This PR is a second control, returning three COMMENTED plus another login's CHANGES_REQUESTED. What actually holds: the author login cannot record a decisive verdict, so reviewDecision stays empty however much review happened. COMMENT reviews from the author and reviews from other logins appear normally. The corrected text says to read reviewDecision and never mistake COMMENT volume for a decision. I folded this passage in from a peer's measurement last push without verifying it myself, while checking my own claims. That is the defect being fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uries the first (#3482) * docs(review): a second approval on the shared login counts zero and buries the first Several agents review through one GitHub account. GitHub resolves a PR's decision by latest-state-per-user, so two approvals from that account are one approver and the later review replaces the earlier one as the effective record. A reviewer cannot see either fact from the PR page. Measured while reviewing #3481: approved twenty minutes after a peer had approved on the same account. The PR still read REVIEW_REQUIRED, and the peer's conflict-of-interest disclosure was left in a superseded review. Sweeping 92 open PRs found the account holding more than one review on 69, and 32 verdict flips across 17 PRs with no commit between the two opposing reviews. Force-pushes were ruled out via the issue timeline on #3471 only, so the lesson states 32 as measured-for-PRs-checked rather than a fleet-wide total. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(review): renumber to 17 — #3066 already claims lesson 16 #3066 (opened earlier, "lesson 16 — re-read current review state before executing a stored authorization") already numbers its lesson 16. Taking 17 here is correct in either merge order: if #3066 lands first the sequence is contiguous, and if this lands first it leaves 16 free for #3066 to fill. My duplicate scan before opening this PR piped `gh pr list` through `head -8` and cut off before #3066, which is why the collision was not caught at open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(review): scope the rule to the login's LATEST decisive state, and renumber to 16 Two review findings, both correct. A peer showed "never a second approval" is wrong when the shared login's latest decisive review is CHANGES_REQUESTED: there, an approval is exactly what clears the block and a comment leaves it latched. #3471 carries that sequence live. Rewritten to key on the account's current decisive state rather than on any historical approval. john-the-dev showed the file would ship 15 -> 17 with a dangling reference. main ends at 15 and lesson 16 exists only in #3066, which is OPEN / BEHIND / CHANGES_REQUESTED and so is not landing first. My earlier "17 is correct in either order" assumed #3066 lands; it is blocked, so a gap is the likely outcome and the blocked PR is the one that can renumber itself. Back to 16, which also matches this PR's body. Also folded in his complementary measurement, unblocking: on a self-authored PR the same login can file no formal review at all, so reviews are empty by construction and the PR reads unreviewed. Two-becomes-one on someone else's PR, zero-possible on your own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(review): self-authorship blocks the DECISION, not every review record The previous wording said GitHub permits no formal review at all on a self-authored PR and that pulls/N/reviews is empty by construction. Both are false, and #3413 -- the instance the claim was grounded on -- disproves it: that endpoint returns four qingyun-wu COMMENTED reviews. This PR is a second control, returning three COMMENTED plus another login's CHANGES_REQUESTED. What actually holds: the author login cannot record a decisive verdict, so reviewDecision stays empty however much review happened. COMMENT reviews from the author and reviews from other logins appear normally. The corrected text says to read reviewDecision and never mistake COMMENT volume for a decision. I folded this passage in from a peer's measurement last push without verifying it myself, while checking my own claims. That is the defect being fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(review): the author's reviews never move reviewDecision; other logins still do Previous wording said reviewDecision "stays empty however much review happened" on a self-authored PR. False in both branches. This PR is the control: authored by qingyun-wu, reviewDecision reads CHANGES_REQUESTED because john-the-dev reviewed from another login. The precise rule is that the author login can only file COMMENT reviews, so its own reviews never move the field, while other logins still do - REVIEW_REQUIRED when none has reviewed decisively, APPROVED or CHANGES_REQUESTED once one has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(review): reviewDecision over other logins is a threshold, not a flag The guide said the field reads APPROVED or CHANGES_REQUESTED once one other login has reviewed decisively. That is wrong for approvals: one approval below the repo's required count still reads REVIEW_REQUIRED, and a reader taking the old wording at face value would read a single approval as an approved PR — on a document injected into reviewers. This PR's own history is the control, with the head unchanged throughout: john-the-dev APPROVED at 12:14:45Z and the field read REVIEW_REQUIRED two minutes later; sonichi APPROVED at 12:51:36Z and it read APPROVED. Keeps the correct half — an author's own reviews never move the field — and splits the two claims apart so the threshold rule stands on its own line rather than inside the self-authored-PR sentence. * fix(review-preflight): a bare verdict is state, not prose — stop dropping it Lesson 16 tells a shared-login reviewer to establish the login's current decisive state before reviewing. The mandatory preflight could not answer that question: prior_art() dropped every review with an empty body before reading its state, so a bare APPROVED rendered as "nothing — no reviews or comments yet." A reviewer following the lesson was told the thread was empty over a live approval, and would spend a second approval that moves the required count by zero. Skip-on-empty stays right about PROSE and is now wrong only about STATE: an empty-bodied review contributes no line to read, and its verdict is kept. prior_art() returns (prose, verdicts); verdicts is latest-per-login, keyed on timestamp rather than arrival order, and rendered above the prose list without the display cap — so a verdict cannot be lost to truncation either. REVIEW.md lesson 16 now names that surface, so the check it prescribes points at output that exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(review-preflight): a failed verdict lookup must not render as a clean one verdict_block's empty branch was the one line the diff-coverage gate flagged, and it is the branch that carries the unknown-vs-empty distinction: None means the lookup failed, [] means it ran and found no decisive review. The prose block already pins this property; the verdict block did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: qingyun-wu (sutando-001) <qingyun@ag2.ai> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three layers, in commit order — this branch IS the code the live lead daemon runs (pid 66174):
1.
0e3e4caf— rescue of the live pool tree's uncommitted state (12 files, +402): the deployed-but-never-committed work found running on 2026-08-26, including the routine-lane runtime filter (owner decision 08-25: routine -> highest CLAUDE core, codex excluded) and hooks/emit-tool-activity.py. Working-tree bytes md5-verified unchanged at rescue time.2.
32d7b7b2— assignment fairness (owner report: core-1 over-assigned). Idle-load ties broke lexicographically so core-1 took every cold assignment (measured ~50% share); now least-recently-picked rotation. Owner lane restricted to claude seats — codex is turnaround-slow, keeps routine/overflow only (owner 08-26: no routine work on codex either; current code gives core-4 nothing unless no claude seat lives).3.
c3ad3ef2— binding room affinity (owner report: mid-conversation core switches lose room context). Affinity now binds while the home core is alive — no idle expiry (AFFINITY_IDLE_S removed), no busy-yield. A room moves only on core death or an unclaimed-assignment reclaim, which deletes the binding and re-stamps the new home.Evidence: 13/13 pool test suites green at head (pool-lead-assignment, pool-lane-routing rewritten to pin the new contracts: test_busy_home_core_keeps_the_room, test_idle_gap_does_not_move_the_room, test_unclaimed_reclaim_releases_the_room_binding, test_routine_skips_codex_for_the_highest_claude_core). Live verification: after the lead restart, a workstream-grouping task routed to core-3 (was core-4 under the stale daemon), and owner-lane tasks rotated core-1/core-2.
Stack note: base
feat/pool-operabilityis itself the head of #3319 (-> feat/lead-follower-pool); merge order is #3319's stack first, then this. This branch diverges from its base by design — the base lacks the runtime filter; reconciliation reviewed here rather than silently merged.Signed: @sutando-qingyun-001:ag2.space
🤖 Generated with Claude Code