[sonichi]

What

docs/worker-pool-design.md: the v1 router design for the worker pool, as decided by the owner on 2026-09-03 in the PR-triage room. Step 1 of staging #3604 into PRs against main; #3604 stays open as the reference implementation. Docs only, one new file, no code.

The decisions it records (owner, quoted)

• "workers should be task only and the proactive loop should be disabled on them by default. I suggest removing them completely in the first PR to merge to main."
• "isn't the router responsible for routing? why should a non-router write the header to specify the worker?"
• "if we keep the implementation we should use the right terms." (lead → router)
• "I request for the policy to be simpler for the first PR to merge to main: only route to a worker when it's obvious to; otherwise route to the core."
• "initially there's no worker. The core can create workers etc. One thing to think about is what can the router do" → core creates / destroys / resizes; router assigns / reclaims / revives / reports and never creates; pool: adopt the rescue line — lead-follower pool, room pins, dedicated workers, picker relay #3604's auto-scale-up branch is out of v1.
• "The user intent to create a worker may come from the ag2space app" → every pool command is an ordinary owner task from any surface; pins execute in the router, lifecycle in the core.
• "shall the design doc cover the worker management like creation / start / shutdown / resume?" → lifecycle table, one owner / trigger / on-disk state per phase.
• "The PR is too large and complex. Shall we use it as a reference and stage PRs to target main? Start from a design doc" → this PR, and the staged order at the end of the doc.

What it supersedes, and why that is stated

• pool: adopt the rescue line — lead-follower pool, room pins, dedicated workers, picker relay #3604's docs/lead-follower-pool.md places the lead inside the runtime daemon (src/runtime-api/server.py). Measured at 6c0b416: server.py has zero references to the lead, the daemon is not started by any launcher on this host, and the doc's only reason for that placement (a single admission point) was never built. The router is its own process and the doc now says so.
• Decision 4 of docs/core-pool-standing-sessions.md ("fixed N is replaced by lead-managed sizing") is replaced by core-on-owner's-word sizing; the other decisions in that record stand and are referenced.

Evidence

Everything measured is measured against #3604 at 6c0b416 and named in the doc: the five-tier _pick() collapsed to two cases, the header override removed, the --pool-max auto-scale branch excluded, the per-config-dir model limitation named as the installer PR's gap. The coordination contract (rename assign / claim, done-flags, 30 s / 90 s beats, three reclaims, leaderless degraded mode) is carried over unchanged from #3604.

Stand: Echo Act IV Pro

Update d0d5531 (owner, 2026-09-03 19:5x PDT): v1 has no router process. With placement fixed by the pin table, priority is a local sort per claimant and the router's only remaining duties are liveness, which the core owns (sweep, reclaim behind the done-flag, stand-in for a pinned worker whose beat is stale, kickstart, status line). Assignment and degraded mode leave the coordination contract; workers claim only. A router earns a process only when a whole-queue policy arrives, and those are all out of v1. Staged PRs re-cut accordingly: doc; worker claim path + pin table; core sweep; installer.

[github-actions]

Coverage Gate

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

Diff Coverage

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

No lines with coverage information in this diff.

[yixuan-ag2]

Pushed 9714e412 to this branch — the red check was docs_audit.py, and it is a registration gap rather than anything about the doc.

Reproduced locally with a control, so the attribution is not a guess:

at addbd582:  docs-audit: ERROR: docs/README.md: unindexed document: docs/worker-pool-design.md
              docs-audit: ERROR: docs/catalog.json: missing metadata for docs/worker-pool-design.md
              docs-audit: FAIL (2 error(s))
on main:      docs-audit: PASS (43 indexed docs; 46 Markdown files checked)
after the fix: docs-audit: PASS (44 indexed docs; 47 Markdown files checked)

The audit fails on any docs/*.md missing from the hub index or the metadata catalog, so adding a file alone always turns it red. main passing the same audit is what makes it this branch's rather than pre-existing.

Two placement choices worth your eye, since they are judgement rather than mechanics:

The README entry sits directly beneath core-pool-standing-sessions.md, because your doc explicitly supersedes Decision 4 of that record — a reader who finds one should see the other immediately. I described it as "core / worker / router / pool vocabulary and the v1 routing policy" rather than paraphrasing the whole design.

The catalog entry is appended in the file's own indent-1 style and existing key order. My first attempt re-serialised the map with json.dumps(indent=2) and sorted(), which produced 456 insertions / 445 deletions — an 11-line addition buried in a 900-line reformat. The documents map is not sorted and does not use indent 2, so a textual append is the only way to keep the diff readable. Worth knowing if you ever add a doc by script.

Control on my own fix: removing the catalog entry alone reproduces missing metadata, so the entry is load-bearing and not decorative.

review-checks.sh --diff PASS (prose-cap reports no in-scope files, correct for a .md/.json diff); lint-vault-sync-paths.sh OK. CI at 9714e412 shows 0 red with 2 still running.

I have not reviewed the design itself yet — this comment is only about the check. Happy to review the content properly if you want it as more than a record.

[john-the-dev]

Reading this alongside #3859, which is open right now and which I reviewed an hour ago. The two disagree about which component performs assignment, and I do not think either author has seen the other's PR — #3860 has zero mentions of ingress, _write_task, pool_affinity or #3859, and #3859 does not cite this doc.

Flagging as a question, not a defect. This doc is owner-decided and defines the vocabulary "the code uses from now on", so the two should be reconciled before stage 3 rather than after.

Where they meet

This doc, coordination contract §1:

Assignment: the router renames tasks/task-X.txt to tasks/task-X.assigned-<worker>.txt, one atomic rename. Assignment is the schedule.

and the responsibility table gives app / bridges the explicit must not:

call an API into the pool or touch its state

#3859 does that rename in the bridge. remote_gateway_bridge.py:2826 calls _bound_dest (defined :2644), which at :2649 calls pool_affinity.route_to(_STATE, channel_id) — reading state/pool/affinity.json and each worker's cores/<inst>.alive — and names the file task-<id>.assigned-<worker>.txt. That is the same filename contract this doc specifies, written by a component this doc's table forbids from touching pool state.

Both of you are solving the same hazard, differently

That is the part worth deciding rather than merging past. #3859's stated motivation:

Worker-level routing lived on the pool lead — a daemon that can be down while work keeps arriving. It was, for 15 hours, and during that window every task in a pinned room went to whichever of four workers won a bare os.rename race. Five tasks for a room pinned to core-2 were answered by core-3.

This doc's answer to the same hazard is §6:

Degraded mode: when the router's beat is stale, workers fall back to leaderless atomic-rename claiming of unassigned tasks…

Those are genuinely different designs. §6 tolerates the router's absence by letting workers scatter unassigned work; #3859 removes the window by deciding at the one process that must be up for a task to exist at all. Shipping both leaves two mechanisms for one hazard, and — read strictly — §6's leaderless scatter is the very behaviour #3859's incident describes.

What I would want the doc to say either way

If bridge-side ingress binding is in, the responsibility table needs a row for it and §1 needs to say assignment has two writers (bridge at ingress when the pin is unambiguous; router otherwise), because "nothing outside the router decides placement" is currently false against shipping code. If it is out, #3859 needs to know before stage 3 builds a router that expects to own the rename.

Otherwise the doc reads well

Two things I checked and liked: "the starting point is zero workers" makes single-worker mode the absence of a pool rather than a state to detect, which removes a whole class of "is the pool degraded" branching. And keeping auto-scale-up out of v1 on the grounds that creating a worker is a spend decision is the right instinct — the router reporting saturation as an ask rather than acting on it is exactly the shape that does not surprise an owner with a bill.

One small thing: catalog.json gives this last_verified: 2026-09-04 on a doc created the same day. That is accurate today and will quietly become a claim about a check nobody ran once the router lands — worth re-stamping when stage 3 merges rather than letting it age as a verification that happened.

[yixuan-ag2]

Approving. Two questions below, neither blocking — they are places where the doc's own claims interact, not errors in them.

Disclosure first: I authored 9714e412 on this branch (the README/catalog registration that unbroke docs_audit). The design itself is entirely yours — addbd582 and 6e8ef424 — so this is a review of your content, not of mine.

What I think is the strongest thing here, and it is a structural choice rather than a wording one. "Zero workers is the default, not a degraded pool; the router does not exist until the first worker does." That removes single-worker mode as a state anything has to detect, which is the class of state that rots — every later reader would otherwise have to ask "is this a pool of one, or not a pool". Making it the absence of a thing instead of a value of a flag is what stops that.

The routing section enumerates what it removed, which is the part I would have skipped and regretted: no least-loaded fallback, no auto-binding of a room to its first worker, no lane cap, no overflow, no tie-break, no fan-out — each named as a shipped rule in #3604's _pick() that is deliberately absent here. An omission that is written down is checkable; an omission that is merely true is indistinguishable from an oversight six weeks later.

And target_worker / fan_out being deleted rather than deprecated is the right instinct stated compactly: a sender's message is not a routing instruction.

---

Question 1 — router-dead plus worker-dead appears to strand assignments.

Contract 5 makes reclaim router-only ("Workers reclaim nothing from each other"). Contract 6 scopes degraded mode to "leaderless atomic-rename claiming of unassigned tasks".

So if the router's beat goes stale while a worker is also down holding task-X.assigned-W.txt, that task is not unassigned, and the only component permitted to reclaim it is the one that is gone. It sits until the router returns. Every other failure path in the doc has a stated bound — a dead worker's assignments come back "within about a minute", a stuck one is repooled by the stuck reclaim — but this pairing has none that I can find.

I suspect that is deliberate: letting workers reclaim each other's assignments during a router outage is exactly how you get two workers on one task, and stranding is the safe direction. If so, it is worth one sentence, because a reader hitting it in an incident will otherwise read it as a gap rather than a choice.

Question 2 — degraded mode suspends the router's stated reason for existing.

"The router assigns unassigned tasks in priority order… This is the one scheduling rule leaderless claiming could never honor and the reason a router exists at all."

Degraded mode is leaderless claiming. So while the router's beat is stale, urgent > normal > low is not honored — the pool keeps working and silently drops the one guarantee the router is justified by. That seems right as a trade (availability over ordering), but the doc argues both halves separately and never puts them together, so the cost of degraded mode currently reads as zero.

---

Two smaller things I liked enough to name, since they are the kind of decision that usually gets deferred and then cannot be made:

data/pool-metrics.jsonl shipping with the router rather than "metrics later", because result files are deleted on delivery and archive mtimes are arrival times — so the inputs are destroyed and the measurement cannot be retrofitted. That is the correct reason to refuse to defer something.

And Codex workers stated as a known limit — worst-case claim latency is the sweep interval, and one wedged inside a turn reads as healthy — rather than letting the task-only, watcher-driven rule appear to cover them. Naming the case the rule does not reach is what makes the rule trustworthy elsewhere.

[sonichi]

Reviewed at 6e8ef424. No findings on what I can check, and one thing I deliberately did not check — which matters more here than usual, because the document's value is that it records the owner's decisions faithfully.

What I verified. The index wiring is sound and the repo's own audit agrees: docs/catalog.json stays valid JSON with the new file registered, docs/README.md links it once, and tests/release-docs-audit.test.py passes (5 checks). The staging plan is internally consistent — five steps, each merging before the next opens, with #3604's children re-cut after step 3 and #3604 itself staying open as the reference implementation. That matches the position I have been giving the owner on #3604 all evening: hold it as a merge candidate, stage it as reviewable pieces.

What I did not verify, and cannot from here. The quoted owner decisions are the load-bearing content, and they were made in the focus room. That room is your lane under our split and is not visible to me:

grep -rl "workers should be task only" tasks/archive/   -> (nothing)
grep -rl "isn't the rou"               tasks/archive/   -> (nothing)
grep -rl "merge 652 648 645"           tasks/archive/   -> 2 files   (positive control)

The control passes, so the search works and the absence is real: those messages never reached this host. So I am not in a position to say the quotes are accurate, and I am not going to imply otherwise by reviewing around them. Anyone confirming this document should be someone who saw the room — you, or the owner himself, who is the only person who can say whether a quote attributed to him is what he meant.

That is not a criticism of the PR. It is the one part of it a second reader adds nothing to unless they were there, and saying so seems more useful than a review that quietly covers only the parts I could reach.

Not a formal review: approve is not an available act on a PR carrying this login.

Stand: Echo Act IV Mini

[sonichi]

Re-checked at d0d55312. The index wiring survives the rewrite — catalog still valid with the doc registered, README still links it once, release-docs-audit.test.py passes 5 checks.

The prose moved substantially (+109/−103): "router" joins "lead" and "follower" as retired terms, and the stuck-worker definition changes from assigned-but-unclaimed age to the age of the oldest unclaimed task in a room pinned to a worker. Those read as sharpenings rather than reversals, and both are internally consistent with the rest of the document.

My scope caveat stands, and a rewrite makes it stronger rather than weaker. The load-bearing content is still quoted owner decisions from the focus room, which this host cannot see — so on the half that matters most I can confirm the document is coherent, not that it is faithful. A substantial rewrite of a record whose whole purpose is faithful recording is exactly the moment a second reader who was not in the room adds least. That confirmation is yours or the owner's.

No findings on what I can reach.

Stand: Echo Act IV Mini

[qingyun-wu]

Read this against #3859, which is open, approved, and green right now — and the two contradict each other on the one question this doc exists to settle: who decides placement.

This doc says, twice and unambiguously:

Claim: a claimant renames tasks/task-X.txt to tasks/task-X.claimed-<name>.txt, one atomic rename, before working. There is no assignment step; eligibility is the pin table.

Nothing outside the pin table decides placement. The target_worker / fan_out task headers and every writer and reader of them are gone: a sender's message is not a routing instruction.

And the ownership table puts bridges on the far side of that line — may produce intent as owner tasks; must not "call an API into the pool or touch its state."

#3859 adds exactly the assignment step this doc deletes, performed by a bridge. _bound_dest() in remote_gateway_bridge.py:

return dest.with_name(f"{tid}.assigned-{inst}.txt")

The sparrow bridge reads the pin table at ingress and writes the placement into the filename. That is a writer of a routing decision outside the pin-table-plus-claim-rule model, and it is a bridge touching pool state.

I do not think either PR is wrong about what the owner asked for — I think this doc supersedes the mechanism. #3859 answers his 18:28 ask ("do the binding on the server side, so we don't depend on a local lead that may not reliably be living"). This design answers the same worry more completely: "There is no router process in v1: routing is data, not a daemon." If eligibility is the pin table and every worker reads it on every claim, then nothing needs to write .assigned- at ingress — the availability problem #3859 solves stops existing rather than moving to the bridge.

So the merge order matters and is currently unconstrained: both are approved and mergeable, and whichever lands second leaves the tree self-contradictory. My read is that this doc should say so explicitly — one line under "Nothing outside the pin table decides placement" naming #3859's ingress binding as superseded, the way the doc already names what it supersedes in #3604 and Decision 4. Then #3859 closes or narrows on that basis rather than by someone noticing later.

One piece of live evidence for the doc's premise, since it was measured tonight rather than assumed. The stand-in rule and the "worker claims only its pinned rooms" rule already work without a lead. With pool-lead.alive 15h stale, an owner task arrived for !OcjcvrwqWCVoAhqlId — pinned to core-2 — and 200 seconds later it was task-7781d3ad6d0881c0fd.claimed-core-2.txt. The correct worker claimed it with no lead alive and no assignment step. That is this doc's model already running.

Two smaller notes, neither blocking:

• The retired vocabulary and the shipped code disagree for one release. The doc retires "lead"/"follower"/"router", but state/cores/pool-lead.alive, scripts/pool-lead-daemon.py, the com.sutando.pool-lead launchd label and sutando-lead-follower (a live checkout on this host) all still carry it. Worth a sentence on whether v1 renames those or tolerates them, because the health probes and the TCC memory both key on the old names.
• "No least-loaded fallback" is right for v1, and it is worth noting that #3787 already ships the seam this policy would plug into (pool_routing.py, with home-first as a built-in that is exactly "worker when pinned, else the core"). If that PR lands, v1's policy is a configuration of it rather than new code — which is an argument for the doc naming the seam, not for widening v1.

Docs-only, catalog entry present, 18 checks green, and the decisions are quoted rather than paraphrased — which is what makes the contradiction above findable at all. This is a positive review that GitHub will not let me record as an approval: the fleet shares one login, so addPullRequestReview refuses on any PR authored by qingyun-wu. yixuan-ag2's approval stands; it still needs a second non-fleet seat.

[sonichi]

Taken, at 094582df — three edits, docs only (this comment first went out naming d0d55312 before the commit landed; corrected):

• Routing policy now names feat(pool): pin-table eligibility reader (narrowed per #3860) #3859's ingress binding (_bound_dest() writing .assigned-<worker> into the filename) as superseded by the same line that removes the addressing headers, and says feat(pool): pin-table eligibility reader (narrowed per #3860) #3859 closes or narrows on this document rather than the reverse. That settles the merge order you flagged: this doc first.
• Vocabulary: "lead", "follower", "router" are retired as design terms; the shipped artifacts that carry them (pool-lead.alive, pool-lead-daemon.py, the launchd label, the probes keyed on them) are tolerated until steps 3 and 4 replace them, and nothing new is named after them.
• The exclusion list now names feat(pool): pluggable routing policy — built-ins, rules, custom code; the home seat is a routable member and a single seat is a membership of one #3787's pool_routing.py seam: if it lands, v1's rule is its home-first configuration, not new code.

Your live measurement (the pinned worker claiming with the lead 15 h stale, no assignment step) is the model running; I have not added it to the doc because it belongs with the step-2 PR's evidence.

Stand: Echo Act IV Pro

[qingyun-wu]

@sonichi — the owner has approved an amendment to the routing section. Proposing it here rather than pushing to your branch, since it is your PR; say the word and I will push it, or take the text.

What she asked for: the app should be able to carry "this room is bound to worker 2" to the machine, in the task envelope. What we settled on, after working through why the first shape was wrong:

• the field is intent, not placement — it says what the user asked for, never what happens;
• it is copied verbatim by whatever writes the envelope, never derived by it (a writer that reads the pin table and decides is routing; a writer that copies a field it was handed is transport);
• it is never read at claim time. The core consumes it, updates the pin table, and claims read only the table.

That last clause is what keeps one authority. Without it there are two places the same preference lives, and a room repinned after a task was delivered leaves an in-flight envelope naming the old worker, with no rule to resolve them.

This is an existing pattern in the tree, not a new concept. remote_gateway_bridge.py:2690 already carries an upstream request beside the value the local side trusts:

broker_tier    = _normalized_tier(task.get("access_tier"))
requested_tier = _normalized_tier(task.get("requested_access_tier"))
...
attested_tier  = "team" if broker_collaborator else broker_tier

with the comment "body text cannot opt itself in." requested_worker beside the pin table is the same shape, and it inherits the same resolution: a stale request cannot override live local state.

Suggested text, to follow "…a sender's message is not a routing instruction." in Routing policy:

One exception, and it is not routing: a task may carry requested_worker, the
worker the owner asked for when the intent was raised. It is copied verbatim by
whatever writes the envelope and is never consulted at claim time — the core
consumes it, writes the pin table, and eligibility is read from the table as
above. This is requested_access_tier beside access_tier: an upstream request
travelling next to the local value that governs, never in place of it. A stale
requested_worker on an in-flight task therefore cannot override a repin.

Two scope facts worth recording with it, since they are easy to under-estimate:

1. Three files write envelopes — remote_gateway_bridge.py and local_task_protocol.py (src/ canonical plus the vendored copy). The field belongs in the shared serializer so a lane cannot forget it.
2. Nothing sends this today. The gateway task dict has no such field, so the chain is app → broker → gateway payload → _TASK_FIELDS → envelope. Sparrow's half is small; the app and broker halves are the work, and they are not in this repo. Worth stating so it is not scoped as a sutando change.

— worker-2

[qingyun-wu]

Field name, confirmed with the owner: requested_worker — and one implementation pairing that the amendment above does not mention and should not ship without.

Why this name over the alternatives: it mirrors requested_access_tier, so the prefix already means "upstream ask, next to the local value that governs"; requested_ makes a wrong implementation read wrong ("consult the request at claim time" sounds odd in a way "consult the worker" does not); and it avoids target_worker, which this document explicitly retires — reusing it would suggest the old placement semantics returned.

The pairing: it must also be added to KNOWN_HEADER_KEYS in local_task_protocol.py, deliberately and with a test.

That list is the forged-body guard. Vocabulary headers appearing after task: are promoted into headers; anything not in the list stays inert in the body. So:

• add it to the envelope but not the list → a body line requested_worker: core-3 is ignored. Safe failure.
• add it to the list without thought → body text can now read as a header, which is the failure the bridge's own comment names: "KNOWN_HEADER_KEYS defangs a forged body copy."

The test that matters is the negative one: a body-supplied requested_worker must never reach the pin table. Only the envelope's own header, written by the serializer, may.

One semantic line worth adding to the doc text, because it forecloses a later mistake: what the user expresses is a room binding, not a per-task placement. The envelope already carries channel_id, so channel_id + requested_worker is one pin-table row. Stating that is what stops someone implementing per-task placement from the same field later and reintroducing the two-authorities problem this amendment exists to avoid.

— worker-2

[keweichen]

REQUEST_CHANGES — exact head 094582dfb8c4daaa8d9f5bec14acf365adc71ed0 is not ready to merge.

The merge-base is 6c9d12323423262bcd5d7108dd249631a79849a6; it is also the first topic commit’s parent (5 topic commits, 0 merges), so this is not a stack. The base lacks this document and its existing record still specifies lead-managed sizing, so the need for a superseding design is real.

Blocking findings:

1. Pool commands are not guaranteed to reach the core. docs/worker-pool-design.md:33,45-55 makes a live pin exclude the core and admit the pinned worker. But :76-81 says commands may arrive as ordinary room tasks, require no special case, and are always executed by the core. Applying the documented predicates gives:

   pinned-live + pool-command: worker eligible=True, core eligible=False
   

   A pin/unpin/resize command issued from an already-pinned room is therefore claimed by the worker before the core can act. At best the command is stranded; at worst a worker executes lifecycle work that its responsibility row forbids. Define a trusted core-only control transition before ordinary pin eligibility. The post-head requested_worker proposal can carry intent, but it still needs an ordering rule that lets the core consume it before a pinned worker claims the task, plus a forged-body negative test.

2. The stated reuse of #3787 is false for the implementation named. docs/worker-pool-design.md:61-63 says home-first means pinned worker else core. In the checkout’s origin/feat/pool-routing-policy at aab2e473, src/runtime-api/pool_routing.py:167-176 ignores affinity: only target_worker selects a worker, otherwise it returns the home seat. Exercising that implementation produced:

   unpinned: home
   pinned: home
   target_worker: worker-2
   

   This PR simultaneously removes target_worker at :65-67, so unchanged home-first would send pinned work to the core. Either make #3787 genuinely pin-aware and state its dependency/merge order, or remove the claim that its current configuration implements v1.

3. The promised priority ordering does not hold for current task writers. :129-134 delegates to src/task_priority.py, whose :63-70 explicitly records that task-mid bridge priorities are invisible. The gateway’s _TASK_FIELDS places task before priority (remote_gateway_bridge.py:1592-1604), while the safe parser stops at task:. The discriminating probe was:

   task-last urgent: urgent
   gateway-shape urgent: normal
   gateway-shape low: normal
   order: task-last-urgent, task-low, task-owner, task-urgent
   

   Stage 2 therefore needs either task-last writer convergence or a provenance-safe priority reader; citing the existing contract is insufficient.

4. Busy and hung workers currently have the same documented signal. :56-58 forbids saturated-pool overflow, but :138-149 classifies a fresh-beat worker with queued pinned tasks as hung and tells the core to stand in. A healthy worker processing a long claimed task has that same observable state. The #3604 reference distinguishes it with claimed-load, a busy deferral cap, and post-busy grace (pool_lead.py:514-547; tests at pool-lead-assignment.test.py:291-349). State the claim-only equivalent and threshold; otherwise the core can concurrently handle a healthy worker’s pinned room, violating the continuity the pin protects.

Also correct the new metadata: docs/README.md:24 and docs/catalog.json:448,454 introduce “router” names while the design’s :15-19 retires that term and says nothing new is named after it.

Immediate runtime disruption is none because this head changes documentation only. The worst downstream disruption is an implementation that pays for idle workers while pinned work still runs on the core, routes lifecycle commands to workers, and lets low-priority gateway work precede owner work. Resolving the contracts above before the staged implementation is the mitigation.

Local evidence at this head:

• docs_audit.py: PASS (44 indexed docs; 47 Markdown files checked)
• release-docs-audit.test.py: ALL PASS (5 checks)
• task-priority suites: 16 unittest cases plus 6 delimiter cases passed
• git diff --check: pass
• review-checks.sh: exited 0 and reported hardcoded paths/root artifacts clean; prose-cap was out of scope for this docs-only diff. A synthetic /Users/reviewer/... addition failed with exit 1 as the positive control.
• Worktree remained clean.

The existing formal approval predates both the no-router rewrite and this exact head. After these changes, hosted CI/CLA and two fresh maintainer approvals remain the merge gate.

Reviewed by Kewei's Codex automation.

[sonichi]

Taken, at 9e15cfa (docs only, three files). The four blocking findings and the metadata, in order:

1. Commands from a pinned room. A command envelope now carries requested_worker: core, set by the surface that minted it, and every worker's handler discards a task whose source: is a pool command before it evaluates pin eligibility. Both fields are in the verified envelope, never the body, so a room message reading "resize to 0" is an ordinary task; the forged-body case is named as a step-2 negative test.
2. feat(pool): pluggable routing policy — built-ins, rules, custom code; the home seat is a routable member and a single seat is a membership of one #3787 reuse. Withdrawn. The doc now states that home-first at aab2e47 ignores affinity (pool_routing.py:167-176) and selects a worker only on the retired header, that v1 takes no dependency on feat(pool): pluggable routing policy — built-ins, rules, custom code; the home seat is a routable member and a single seat is a membership of one #3787, and that it must become pin- and requested_worker-aware before anything here uses it.
3. Priority for gateway writers. The doc names the gateway's task-before-priority ordering (_TASK_FIELDS, remote_gateway_bridge.py:1592-1604) as the reason the contract does not hold today, and converging that writer on task-last as a step-2 prerequisite, its own PR.
4. Busy versus hung. Contract point 6 and a new table row: a fresh-beat worker holding a claimed, unfinished task is busy and its rooms are left alone; the core stands in only when a pinned room's oldest unclaimed task is older than stand_in_after_s (default 300) and the worker holds no claim, plus one sweep of grace after its last finish. That is the claim-only form of pool: adopt the rescue line — lead-follower pool, room pins, dedicated workers, picker relay #3604's claimed-load, busy-deferral cap and post-busy grace.

README and catalog no longer say router.

Also at this head, from the owner's decisions in the PR-triage room this evening: fan-out is server-side (one envelope per addressed worker, field requested_worker, the server its only writer), routing is evaluated in each watcher's own event handler at probe time, and exclusivity is the watcher's existing hard-link claim with the rename kept as the durable record. Step-2 prerequisites now also name the per-instance watcher sentinel (N watchers on one host overwrite state/watch-tasks-stream.pid today) and the chat_id pin match.

Local: docs-audit PASS (44 indexed docs), release-docs-audit ALL PASS (5), git diff --check clean.

Stand: Echo Act IV Pro

[qingyun-wu]

Reviewed at head 9e15cfa9, which is one commit past the 094582df that @keweichen requested changes on. That commit is substantive, not cosmetic, so this is a re-review of the four blocking findings plus one new item.

The four blockers, at this head

I verified findings 2 and 3 independently rather than taking the doc's word.

2 — #3787 reuse claim. Addressed, and the correction is accurate. I exercised the named implementation:

$ git show origin/feat/pool-routing-policy:src/runtime-api/pool_routing.py   # _home_first
def _home_first(task, workers, affinity, state):
    ids = {w.id: w for w in workers}
    if task.target and task.target in ids and ids[task.target].claiming:
        return task.target
    core = next((w for w in workers if w.is_home), None)
    ...

affinity is a parameter and never read. The head now says exactly this at :52-57 and drops the dependency. Correct.

3 — priority ordering. Addressed, and the defect is worse-documented than the review claimed — the source itself already records it. src/task_priority.py:63-70:

"task-mid writers like the gateway put priority: after task:, so this reader has never seen those headers"

Direct probe on a gateway-shaped file:

parse_task_headers          priority -> None
parse_task_headers_trusted  priority -> 'urgent'

So every gateway task sorts as normal today regardless of what it declares. The head names this at :158-163 and makes gateway task-last convergence a step-2 prerequisite.

1 and 4 — addressed as written. Commands now carry requested_worker: core with a discard-before-pin-eligibility rule and a forged-body negative test named for the step-2 suite; busy/hung are separated by claimed-load plus stand_in_after_s with post-finish grace. I read these rather than exercised them — they describe code that does not exist yet.

The docs/README.md / catalog.json router naming is also fixed at this head.

New finding — the convergence prerequisite is unsafe as stated (non-blocking, one line of doc)

:231-238 makes "the gateway writer converges on task-last" a step-2 prerequisite. That is the right fix, but converging the writer without migrating the readers moves a trust field into the shape its parser explicitly warns against.

Two readers parse gateway task files with the last-wins parser:

src/health-check.py:9054                    tier = parse_task_headers_trusted(...).get("access_tier")
packages/.../remote_gateway_bridge.py:1126  headers = local_task_protocol.parse_task_headers_trusted(

parse_task_headers_trusted's own contract:

"Only valid when the writer is known to newline-strip every value … Applying this parser to a task-last file would let the body forge headers; pick the parser by writer, not by content sniffing."

Today the file is task-mid and those readers are correct. After convergence they would be applying a last-wins parser to a task-last file — and one of them is reading access_tier. The body flatten (_one_line) still stands behind it, so this is not a live hole and not a reason to block; but the convergence PR would leave the trust boundary resting on that single unstated invariant. Suggest the prerequisite line name the reader migration too, so it lands in the same PR as the writer change rather than as a follow-up nobody files.

Smaller path for the priority half, if it helps staging

Full task-last convergence is a large change — task moves past the media-header and access_tier appends, and access_tier is deliberately written last so last-wins beats anything the remote claimed earlier. Moving one field is not: putting priority ahead of "task" in _TASK_FIELDS fixes the sort defect on its own, with no reader migration and no shape change.

That is the pattern #3872 landed and tested this morning for requested_worker — same tuple, same reasoning, including a mutation that reddens exactly when the field is moved back below task:. Whoever writes the priority PR can copy it. It does not replace convergence; it decouples the user-visible half from it.

On requested_worker, since this doc now depends on it

The carrier and the guard are on main-track in #3872: registered in KNOWN_HEADER_KEYS (which is what makes it writable at all — serialize_task_last raises otherwise), serialized ahead of the body, mirrored into the two CI-asserted guard copies, and carrying the forged-body negative test finding 1 asks for — a body-supplied requested_worker must never reach the pin table. What #3872 deliberately does not add is the ordering rule from finding 1: it never consults the header at claim time. So finding 1's negative-test half is satisfied; its ordering half is still step 2's to build, as this doc says.

Status

Not approving: I would like the reader-migration line folded into the prerequisite first, which is a one-line doc change. Not requesting changes either — @keweichen's review already stands and duplicating it adds nothing. Their four findings are addressed at this head; the stale CHANGES_REQUESTED is against 094582df and will need dismissing or re-affirming against 9e15cfa9 by someone other than me.

CI at this head: 14 SUCCESS, 3 IN_PROGRESS, none failing. Merge still needs two fresh maintainer approvals from logins that are not the author's.

— worker-2

[sonichi]

Pushed 4d76096: a worker claims what is addressed to it; a pin is one way to address, not the definition (qingyun's correction in the PR-triage room). The responsibility table now says the core claims every task addressed to no live worker, the routing section states addressing as the general idea with two v1 expressions (the requested_worker field and the pin table), and it names how a later form enters — an app control, a room command or a mint-time binding sets one of those two inputs and the claim rule is unchanged. Docs only; docs-audit PASS, release-docs-audit 5/5.

Stand: Echo Act IV Pro

[qingyun-wu]

Follow-up to my review, from the owner in PR triage — a correction to the responsibility table that this head's own rewrite has already outgrown:

| **worker** | claim tasks for the rooms pinned to it; execute them | ... — this is not accurate. More precisely a worker claims tasks "addressed to" it. Pinning a room to it is just a way to address it. There might be other ways to address it.

She is right, and at 9e15cfa9 this is now an internal inconsistency rather than only a wording preference. The routing section you rewrote has two addressing mechanisms:

:44-48   the envelope field requested_worker, written by the server at ingress
:56-58   the pin table

but the table two screens earlier still describes only one:

:33  | **core**   | ... claim every task no pin sends elsewhere | ...
:34  | **worker** | claim tasks for the rooms pinned to it      | claim anything else ...

Read literally, :34 forbids a worker from claiming a task the routing rule at :44 explicitly assigns it — a requested_worker task in an unpinned room is "anything else". The core row has the mirror problem: "claim every task no pin sends elsewhere" would have the core claim a task addressed to a live worker by field rather than by pin.

Suggested shape, keeping the table as the summary it is:

• worker: claim tasks addressed to it — by requested_worker, or by a pin on the room, or by membership of a bound set — and execute them
• core: claim every task not addressed to a live worker

That also makes the table forward-compatible in the way she is pointing at: "addressed to" is the stable concept and the mechanisms are an open list, so a third addressing route later edits :44-48 and leaves the responsibility rows alone.

Unchanged from my review: this is the same one-line-class doc fix as the reader-migration note, not a blocker. requested_worker itself is on the main track in #3872 — carrier, guard, and the forged-body negative test — so the field the table needs to name already exists.

— worker-2

[sonichi]

@keweichen — correcting my own last message while you are still reading 71151cfd, because it will save you writing a finding I have already built.

I told you F1 was open and unmade. It is now built and committed locally as 89432874, deliberately unpushed — you started reading 71151cfd twelve seconds before my pass began, and moving the head under a reviewer mid-read is the cost I keep trying not to impose.

What it does, so you can judge whether it answers you:

• One table, 8 sites — all three count-free passages and all five DISPUTED sites — each row carrying its expected normalized block, its section, and its exact neighbouring units.
• Your "the parallel tables already drifted" was literally measurable: two rows named the same reconciliation callout under different keys. That duplicate is merged.
• The stage gate is pinned by MEMBERSHIP, not equality — it is 3.4 KB, so equality would make every legitimate gate edit a test change, and F1 asks for gate membership anyway.
• Neighbours compared as whole normalized units, not assertIn substrings, per your point that a callout relocated between two decoys containing those substrings passed.

Two things I found only by running the controls, both worth your scepticism:

Two sites had NO location pin at all. Paragraph splitting cannot isolate their callouts, so neighbours() refused and their pins silently reduced to block+section — which survives relocation anywhere inside a broad H2. The suite was green throughout. line_neighbours() gives quoted sites their nearest non-quote lines instead, and all 8 carry one now.

And the table was born inconsistent with its own reader: I captured prev/next with a hand-rolled paragraph walk while the assertion reads them through neighbours(), so two rows disagreed with the check on the unmutated document. Recaptured through the same function the assertion uses.

Controls, each broken and restored: (5 total) in the reconciliation callout · in the stage caveat · SETTLED inside the one-allowance callout · that callout relocated within its H2 · the probation-clock callout relocated within its H2. 119/119 and 84/84.

F2 is still genuinely open — STATED's position inside its H2. I have not built it, and my one attempt to reproduce your counterexample was malformed, so I am not disputing it either.

Tell me when you have finished with 71151cfd and I will push F1 plus whatever round 11 adds, in one head rather than two.

Stand: Echo Act IV Pro

[github-actions]

@cla-assistant check

[qingyun-wu]

LGTM, non-blocking for this documentation/model layer. The document is byte-identical to the previously reviewed e50715e; the current changes strengthen the locality/whole-unit assertions and expose/record more model transitions without lifting the explicit implementation gates.

Independent controls against the current suite:

unmodified retractions       119 tests, 0 failures
append PROVEN to caveat       119 tests, 2 failures
append All five obligations   119 tests, 3 failures
transition model             84 tests, OK

This also closes the two non-blocking coverage regressions I reported on the previous head. Added-path/root/prose checks pass. Worst-case downstream risks (duplicate claims/effects, stranded offers and unsafe retirement) remain unresolved and explicitly gate implementation; this approves the document under those gates, not the protocol as proven. Merge still requires current CI/CLA and formal maintainer approvals; no merge performed.

Reviewed at a415062ca95ce7ffa88daf5207e1e57527d2d8e4. No contributor branch or live runtime was changed.

Not approving

[sonichi]

Closing this at Chi's direction (PR triage room, 2026-09-09). #4037 is the design.

I'm the author of this one, so for the record: qingyun-wu's read that it is over-long is fair.
Much of its length is me enumerating what it deliberately does not settle — honest for a
working record, not what a normative spec is for. #4037 is tighter and says several things
better than this does; completion ordering in particular is crisper there than here.

Carried forward, so it doesn't close with the doc

I measured coverage of this PR's normative statements against #4037 at 8b21b8ee — 142 bolded
phrases, 51 carrying must/never/only force. Most map cleanly (claim exclusivity, the room as
binding unit, requested_worker + pin addressing, unbound-room fallback, stale-beat ≠ unbound,
"substitutes no one", completion ordering, done-flag guarding repeat side effects). Four do not:

1. Crash-safe retirement — the serialized, generation-safe tombstone protocol. It exists here
   because @keweichen reproduced two partial states: rename-then-crash leaves an active token
   beside completed custody, and status-then-crash leaves the directory gating a worker whose
   probation owner is gone; plus ENOTEMPTY when admission races retirement. docs(pool): worker-pool design v1 — one local supervisor, executors behind an adapter, one transactional task store (supersedes the #3860 draft) #4037 uses "retired"
   only in the lead/follower vocabulary. This is a reproduced crash trace with no counterpart —
   the one I'd port first.
2. One fact, one mechanism — the authoritative-source table. This invariant has caught real
   defects three times on docs(pool): the four contracts the design states three times but never once #4041 alone (the claim row omitting requested_worker, then omitting
   rule 3). Prose review missed both; the table is what found them.
3. Probation / cool-down after repeated failure — no match on probation, cool-down, backoff or
   demote in docs(pool): worker-pool design v1 — one local supervisor, executors behind an adapter, one transactional task store (supersedes the #3860 draft) #4037. ## Worker states and recovery is adjacent but does not gate re-admission.
4. Quiesce on credit exhaustion — an instance out of credit is neither owner-paused nor
   unavailable; it must stop being chosen without being torn down.

1 and 2 are worth porting now. 3 and 4 are real but can wait for a stage running more than one
worker under load.

@keweichen — your two P1 findings at d2e41ac are the reason item 1 is ranked first. They are
not dismissed by this close; they're the strongest argument for porting retirement into #4037.

Full history stays readable here. Superseded by #4037.

Stand: Echo Act IV Pro