[#441] feat: Codex workflow orchestration — tier-1 in-harness fan-out for pair-loop

[rucka]

PR Information

PR Title: [#441] feat: Codex workflow orchestration — tier-1 in-harness fan-out for pair-loop
Story/Epic: #441 · Epic #212 — Supervised automation
Type: Feature
Priority: High
Assignee: rucka
Labels: risk:red (cost: yellow)

Summary

What Changed

pair-loop gains a second tier-1 fan-out realization. It probes its session for a fan-out primitive, binds the first realization the surface map confirms, announces it, and — where the binding dispatches by spawn/wait — drives the same implement → PR → review ↔ fix lane through the harness's own subagent tools instead of degrading to one card per invocation.

Everything in that lane that is a rule rather than a judgement lives in one tested module, packages/knowledge-hub/src/tools/codex-fanout.ts, shipped as the generated KB asset .pair/knowledge/assets/codex-fanout.cjs and invoked as a 7-command CLI (bind, cap, packet, collect, converge, audit, resume). Only the harness tool calls stay with the model. Same pattern as the coverage ratchet (ADL 2026-07-13-gate-tooling-code-in-tested-modules + ADR-023) — not a new one, and not a ported workflow: Codex has no workflow runtime, its orchestrator is the model.

The orchestrator being a model is the design constraint the whole module answers to. Every request it composes and every audit line it writes is something a killed or careless run can get wrong, so each one fails closed: an unknown key is rejected rather than dropped, a missing required field is refused at write time rather than defaulted, and every ambiguous read resolves to "more work owed", never to "done".

Why This Change

Before this branch, a Codex session ran the delivery process one card per invocation while a Claude Code session fanned out — the same policy file, two different throughputs, and the difference documented as a permanent property of the harness. It is not: the probe found two multi-agent toolsets exposed. ADR-021 gains §7 and the ADL that asserted otherwise is corrected in place.

Story Context

User Story: As the delivery process, I want the unattended loop to run context-safely in whichever harness the session is in, so that the same cards reach the same gate with the same audit regardless of the product driving them.
Acceptance Criteria: AC1–AC14 (story body). AC11 and AC13 are guarantees of absence and are asserted mechanically, not claimed — see Reviewer Guide.

Changes Made

Implementation Details

• The surface map is data, and it holds EVERY in-harness realization. Claude Code's Workflow and both Codex toolsets are entries in HARNESS_SURFACE_MAP, differing only in dispatch shape: delegated-run (one call hands the whole run to a runtime that fans out itself — no cap, no wait bound for this orchestrator) vs spawn-wait (this orchestrator starts and awaits each subagent, so it owns both). bind returns the shape; the skill branches on it, never on a product name. A vendor rename is an edit to that array and nothing else — asserted by a containment test.
• The probe is the only admissible evidence. ProbeObservation has fields for exposed tool names, a reported namespace, a concurrency ceiling and a wait timeout — and no field for a product name or a version, so the shape is the rule. An unrecognised probe reads as absent and the cascade degrades: external driver, else one card + continue-token. Never an in-context multi-card loop.
• Every wait is bounded, on every path. bind resolves the bound in three steps and always terminates in a number for a spawn-wait binding: the session's reported value, else a config key the entry names, else the entry's declared fallback — with waitTimeoutSource naming which, and the announcement carrying it into the audit. The generation a bare Codex session binds has no wait-timeout config key at all, so "read the harness's configured maximum" resolves to nothing there; leaving the caller with null met AC10 on paper and not on the common path. Verified against codex-cli 0.150.1 per entry, in verifiedAgainst.
• Context packets, with two pre-spawn rejections. packet builds one card's packet — role text in the request (no harness profile required), the card, its worktree, the skill, the return schema, and the findings the role must act on. A review packet carrying anything under the project's resolved working area is rejected before any spawn — in an attachment or in the card's own title/notes, because the reviewer receives the card verbatim and a pointer in prose is as harmful as one in a path. A fix packet with no findings is rejected too.
• One result contract, two realizations. collect validates every return against the built-in phase contract, plus the project's generated review contract when it is on disk and fresh. The override tightens only. A conformance guard slices the workflow's own STEP_SCHEMA/PR_SCHEMA/LOOSE_REVIEW_SCHEMA/FIX_SCHEMA out of .claude/workflows/pair-implement-batch.js and asserts deep equality, so a divergence fails on whichever side moves first.
• A fix is owed by findings, never by a phase list — and the audit has to say so. converge partitions a review's findings, converges on zero actionable ones (no fixer is spawned on an approved PR), dispatches one fix round followed by a re-review, and escalates at the cap. OWED_PHASES excludes fix, so a resumed card re-enters at review. The decision is carried on the review's audit record as action, and both halves are mechanical: audit refuses a completed review record whose action is not converged/fix/escalate, and the resume reconstruction closes the cycle on converged alone.
• Ceilings compose, this module adds none. cap returns min(dependency, policy, harness) and which of the three bound it, and it owns no policy knob — asserted. When the probe reports no harness ceiling, bind hands back the concurrencyKey the entry declares so the caller can read it rather than invent a number; a key it cannot use halts the fan-out instead of guessing. A ceiling nobody observed is never silently a ceiling of 1. cap called without a ceilings object answers with a command-level error naming the fields it needs, never a raw TypeError.
• Audit and resume, both stamped with the invocation. audit appends and reads back; an unwritable audit HALTs the run. Every record — card or run — must carry a non-empty run, refused at write time and again on a named resume, because a halt is scoped to the invocation that recorded it and an unstamped line cannot be placed: inferring the boundary from a later caller is how one run's escalation is read as another's history. resume reconstructs per-card state from the audit alone and retires a halt on a later success of the same phase. Records are kind:'card' (the default, naming a card) or kind:'run' (the realization announcement — no card id to invent).
• Unknown keys are rejected at every level a model composes: the request root, packet, card, probe and ceilings. A dropped key reinstates exactly the default it was passed to override — a dropped harnessCeiling dispatches the policy's parallelism into a harness that allows less, a dropped workingPath hands the author's checkpoint to the independent reviewer.

Files Changed

• Added: packages/knowledge-hub/src/tools/codex-fanout.ts + .test.ts (126), build-codex-asset.ts, build-kb-asset.ts (extracted generic asset builder), src/conformance/codex-fanout-asset.test.ts (23), src/conformance/codex-realization.test.ts (18), .pair/knowledge/assets/codex-fanout.cjs (+ dataset mirror), .../skill-conventions/harness-realization.md (+ mirror)
• Modified: .claude/skills/pair-loop/SKILL.md (+ dataset source, 0.1.0 → 0.2.0), build-ratchet-asset.ts (now delegates; output byte-identical, own guard), adr-021-fan-out-three-realizations.md (§7, §8, tier row, 5 trade-offs), decision-log/2026-07-11-agent-execution-layer.md (factual correction), tech/architecture.md (## Unattended Fan-Out), collaborative-workflow.context.md (6 terms), skill-conventions/README.md + graceful-degradation.md + resolution-cascade.md + llms.txt (registration), docs/integrations/codex.mdx, docs/reference/batch-engine.mdx, docs/tutorials/unattended-delivery.mdx
• Deleted / Renamed: none

Testing

Test Coverage

• Unit tests: codex-fanout.test.ts — 126 cases against mocked harness primitives and an in-memory filesystem double. CI needs no Codex binary and no network: the far side of this integration is observably in motion, and a suite that depended on the vendor would go red for reasons that are not this repo's.
• Conformance: codex-realization.test.ts (18) — containment, zero-merit-logic, regression-by-absence. codex-fanout-asset.test.ts (23) — asset drift, CLI smoke against the shipped artifact, result-contract parity with the workflow. The three suites are 167 cases at this head; every review finding across four rounds carries a case, and the exploit reproductions run against the shipped asset rather than the source.
• E2E: the website suite (39/39), unrelated to this change but part of the gate set.
• Manual: a real Codex dogfood run over this repo's own backlog is a DoD line and is NOT done — it needs a Codex session no session so far has had. Flagged here rather than silently checked.

Test Results

pnpm quality-gate                          ✅ exit 0  (ts:check, test, lint, workflows:test,
                                                      format:check, gate:composition, hygiene,
                                                      smoke-modes, docs:staleness,
                                                      skills:conformance, dup:check)
./scripts/smoke-tests/run-all.sh --cleanup ✅ exit 0  (all scenarios)
pnpm --filter @pair/website e2e            ✅ 39/39

Pre-merge tiering: disabled ⇒ the full suite runs regardless of tier, which is a superset of the 🔴 set this story's classification requires.

Testing Strategy

• Happy path: a probed session binds tier 1 and announces the primitive and its wait bound; a batch is capped, dispatched, collected, converged, audited and resumable.
• Edge cases: a probe that misses; a v1 session reporting a v2 namespace; a handle renamed on the vendor's side; a ceiling of 0; a truncated last audit line; a card halted by an older run; a review record with no action; an audit line with no run stamp; a working-area pointer smuggled into card.title/card.notes; cap invoked with no ceilings at all; a misspelled key in any of the five request objects.
• Error handling: every dispatch outcome is one of six declared terminal outcomes. An absent, unparseable or schema-invalid return is a failed phase — a missing result is never read as success, which is the one reading that would let an unattended run report work it never did.

Quality Assurance

Review Areas

• The blindness check (assertBlind + assertBlindCardText + blindDenyPrefixes) — it is the AC5 guarantee, and it takes the project's resolved working_path as an input. A parent-relative attachment is rejected outright rather than prefix-matched, and both prose channels of the card are scanned, not attachments alone.
• collect's override semantics — the party composing that JSON is a model, so the override tightens and never replaces.
• The run stamp on every audit record — the invocation boundary is the only thing that keeps one run's halt out of the next run's reconstruction, so it is required on write and on a named resume rather than inferred from the caller.
• converge + the review audit record — the loop bound is the loop's own; it decides nothing about what to work on. Worth checking as a pair: converge returning fix and the audit line recording it are two separate acts by the same non-deterministic party, and the second is now refused when it omits the decision.
• The wait-bound resolution — the one place the module supplies a value nobody probed. It is data on the map entry, not a number chosen at run time, and the binding reports its source; see ADR-021's trade-offs for why fail-closed degradation was not the alternative taken.
• The request shape — unknown keys are rejected at the root, the packet, the card, the probe and the ceilings, because a silently-dropped key reinstates exactly the default it was passed to override.

Documentation

• Technical decisions: ADR-021 §7 (a realization is probed; both tier-1 realizations are entries in one map differing by dispatch shape) and §8 (four cross-realization properties, including: where the orchestrator is a model, its omission must fail closed mechanically rather than in prose). Trade-offs record that tier 1's wait bound is not always configurable and that the map therefore carries a declared fallback. The 2026-07-11-agent-execution-layer ADL's "Codex does not have subagent primitives" clause is struck and corrected with the probe evidence.
• KB: a new generic convention, skill-conventions/harness-realization.md — probe → bind → announce → degrade; the surface map as data; the map holds every realization; dispatch shapes; the bind returns a bound the caller can always apply (three-row resolution, plus: bounding keys belong to the entry that owns them, and Verified against covers them too); ceilings compose.
• Website: a Codex unattended-delivery section, the first generation's lack of a wait-timeout setting stated, and two now-inaccurate "everything else degrades to one card" claims corrected.
• Ubiquitous language: six terms registered in collaborative-workflow.context.md.

Risk Assessment

Risk	Impact	Probability	Mitigation
The vendor renames or withdraws a toolset	Med	High (already observed: a third mechanism was withdrawn before this landed)	The surface map is data with a verifiedAgainst note per entry; a rename is a one-line edit, and a probe miss fail-closes to a lower tier
The two tier-1 realizations drift	High	Med	Result-contract parity asserted against the workflow's own constants; ADR-021 §8 fixes the four lane properties both must hold
A model-driven orchestrator is less deterministic than a JS one	Med	—	Every rule is in the tested module; every request and every audit record fails closed on an omission; every dispatch is schema-validated and every outcome audited rather than trusted. Narrowed, not closed — stated in ADR-021's trade-offs
The declared wait fallback is wrong for a slow phase	Low	Med	A timeout is a declared terminal outcome: the phase collects timed-out, the card halts for that run only, and the next run re-drives it. The alternative — an unbounded wait — hangs the unattended run outright

Reviewer Guide

Review Focus Areas

1. AC11 — Claude Code behaviour is unchanged. A guarantee of absence, asserted rather than claimed: git diff --stat origin/main...HEAD shows no file under .claude/workflows/ or .claude/agents/, and codex-realization.test.ts asserts a Claude session binds claude-code-workflow / tier 1 / delegated-run and that the workflow's argument list ({ policyText, root, overrides, predicateOverride, startIteration, tagProjectionFamily }) is untouched.
2. AC13 — distribution is untouched. No .codex/ target exists; asserted against apps/pair-cli/config.json's real registry set, which is also asserted unchanged.
3. AC10 — every wait is bounded. Check it on the DEFAULT session, not the configured one: bind on a bare spawn_agent/wait_agent probe must return a positive waitTimeoutMs.
4. Zero merit logic. The new skill section must not smuggle in a selection or classification criterion. Asserted by pattern over the section's own text, and by the module carrying no policy constant.

Testing the Changes

git checkout feature/US-441-codex-workflow-orchestration
pnpm install
pnpm --filter @pair/knowledge-hub exec vitest run src/tools/codex-fanout.test.ts src/conformance/codex-realization.test.ts src/conformance/codex-fanout-asset.test.ts

# the shipped asset, exactly as the skill invokes it
echo '{"probe":{"tools":["Workflow","Task","Read","Bash"]}}' | node .pair/knowledge/assets/codex-fanout.cjs bind
echo '{"probe":{"tools":["spawn_agent","wait_agent"]}}'      | node .pair/knowledge/assets/codex-fanout.cjs bind   # waitTimeoutMs is a number, source realization-default

# each fail-closed rule, one line each — all exit 1
echo '{"workingPath":".pair/scratch","packet":{"phase":"review","card":{"id":"441","title":"t","branch":"b"},"attachments":[".pair/scratch/checkpoints/441.md"]}}' \
  | node .pair/knowledge/assets/codex-fanout.cjs packet   # the reviewer never gets the author's checkpoint
echo '{"path":"/tmp/a.jsonl","records":[{"kind":"card","id":"441","run":"r1","phase":"review","outcome":"completed","round":1}]}' \
  | node .pair/knowledge/assets/codex-fanout.cjs audit     # a completed review must say what converge decided
echo '{"ceilings":{"dependencyAllowed":5,"policyMax":5,"harnessCieling":2}}' \
  | node .pair/knowledge/assets/codex-fanout.cjs cap        # a misspelled ceiling is rejected, not dropped
echo '{}' | node .pair/knowledge/assets/codex-fanout.cjs cap                        # names the fields it needs, never a TypeError
echo '{"packet":{"phase":"review","card":{"id":"441","title":"see .pair/working/checkpoints/441.md","branch":"b"}}}' \
  | node .pair/knowledge/assets/codex-fanout.cjs packet   # the pointer is refused in card prose too

# and the read side of the same rule — redispatch is ["review"], never []
node -e 'const a=[`{"run":"r1","id":"441","phase":"implement","outcome":"completed"}`,`{"run":"r1","id":"441","phase":"pr","outcome":"completed","prNumber":9}`,`{"run":"r1","id":"441","phase":"review","outcome":"completed","round":1}`].join("\n");console.log(require("child_process").execSync("node .pair/knowledge/assets/codex-fanout.cjs resume",{input:JSON.stringify({audit:a,run:"r1",id:"441"})}).toString())'

Dependencies & Related Work

• Follows: pair-cli execution adapter: run a card on any engine (pi, opencode, claude -p) — portable fan-out, amends ADR-017 #451 (pair run, the tier-2 external driver) — it wrote ADR-021's three tiers and already reserved the tier-1 slot this story fills. This branch amends that ADR; it does not write a second, divergent three-tier record.
• Related: Generalized implement-batch shipped in the dataset: N mutex-safe cards to review-approved PRs, never merges #219 (implement-batch, the per-card pipeline this skill composes and never re-implements), ADR-017 §3 (context isolation, the invariant every tier preserves).
• Follow-up work: the manual Codex dogfood run (a DoD line, see Testing). No technical debt created; no changeset added, per ADL 2026-08-20-a-changeset-is-added-when-a-release-is-scheduled.

---

_{This description reflects the current head commit, not a round-by-round history.}

[rucka]

Verdict

risk:red · cost:yellow — CHANGES-REQUESTED — the deterministic module is well-built and its guards are real, but five AC-level holes reproduce against the shipped asset: the reviewer-blindness rejection (AC5) is bypassable two ways, collect's caller-supplied schema nullifies fail-closed validation (AC6), a halted card is frozen forever across runs (AC9), and the Codex branch has no verdict-driven review↔fix convergence (lane parity).

Open findings: 10. 5 Major, 5 Minor. One Question is non-actionable (human merge-gate item).

PR: #469 · Author: rucka · Reviewer: independent review agent · Date: 2026-08-28 · Story: US-441 · Type: feature

Classification matrix — per dimension

Dimension	Tier	Source	Note
Service/domain criticality	green	Criticality Table	packages/knowledge-hub + apps/website, both Low
Change/diff risk	yellow	diff footprint	+3193/−67 across KB convention, dataset skill, adoption, docs; additive, no migration
Business impact	red	subdomain class	Collaborative Workflow is Core — this executes delivery orchestration
Security relevance	red	path heuristic	unattended fan-out with repo write access; confirmed by two blindness bypasses below
Coupling balance	yellow	story matrix	contract-strength integration toward an observed-volatile vendor surface; ACL present and genuinely contained

Tier = max(assessed) = risk:red, confirmed, not raised. Cost = yellow, confirmed (bounded LLM-call signal, every ceiling declared).

Assessments

Security — Input validation

Verdict: red — the packet allow-list is the security control of this diff and it is bypassable by two ordinary path spellings.

Details

• normalizeAttachment rejects absolute paths but allows ..; assertBlind then compares a prefix, so ../../pair/.pair/working/checkpoints/441.md never matches .pair/working/. Reproduced against the shipped asset (Major 2).
• BLIND_DENY_PREFIXES is a hard-coded literal while working_path is a documented project override (working-area.md §Overriding the Path). Reproduced (Major 1).
• collect accepts a caller-supplied schema that replaces the contract (Major 3).
• Card fields are shape-checked (assertSingleCard) but not content-validated; unlike pair-implement-batch.js, which validates card fields by CONTENT because they reach shell commands. Here they reach worktree string assembly and a spawn prompt only, so it stays a Question rather than a finding.

Security — Output handling

Verdict: green — outputs are JSON.stringify of own structures; nothing is interpolated into a shell.

Security — Authentication

Verdict: not applicable — no auth surface touched.

Security — Authorization

Verdict: yellow — the only authorization-shaped control in the diff is the reviewer allow-list, and it is the subject of Major 1 and Major 2. Merge authority correctly stays with ## Auto-Advance (BR6 respected).

Security — Introduced vulnerabilities

Verdict: yellow — 0 conventional OWASP findings; 2 introduced control-bypasses in the story's own declared invariant.

Details

Severity	Category	File:location	Introduced / pre-existing	Recommendation
P1	Broken access control (analogue)	codex-fanout.ts:451	introduced	resolve the deny prefix from working_path
P1	Path traversal	codex-fanout.ts:463-488	introduced	reject .. segments in attachments

Cost

Verdict: cost:yellow — confirmed. Each iteration fans out N agent runs, but every ceiling is hard and declared (policyMax, harness ceiling, cap arithmetic, stop predicate). effectiveParallelism adds no ceiling of its own — verified.

Architecture (Coupling)

Verdict: yellow — the anti-corruption layer is real, not claimed. codex-realization.test.ts slices the module below the surface-map section and asserts zero vendor tokens in the logic, and asserts none in either skill copy. A vendor rename genuinely is a data edit.

Bug fix — Red test before fix

Verdict: not applicable — not a bug fix. The PR states the test-after ordering explicitly rather than hiding it; /pair-process-implement permits it for new features.

Details

Findings by severity

Critical (must fix before merge)

• none.

Major (must fix before merge)

• packages/knowledge-hub/src/tools/codex-fanout.ts:451 — BLIND_DENY_PREFIXES = ['.pair/working/'] is a literal, but working_path is a supported project override (.pair/knowledge/guidelines/collaboration/working-area.md:54-58, example .pair/scratch). In a project that sets it, the AC5 pre-spawn rejection silently no-ops. Verified against the shipped asset: echo '{"packet":{"phase":"review","card":{"id":"441","title":"t","branch":"b"},"attachments":[".pair/scratch/checkpoints/441.md"]}}' | node .pair/knowledge/assets/codex-fanout.cjs packet → exit 0, packet built with the author's checkpoint attached and blind:true. The reviewer subagent then reviews the author's own account of the work — precisely the risk row the story lists as "Reviewer blindness silently lost in the port". Fix: take the resolved working path as an input to buildPacket/assertBlind (default .pair/working/), and have the skill's 1b.3 pass what Step 0 resolved.
• packages/knowledge-hub/src/tools/codex-fanout.ts:463-488 — normalizeAttachment rejects absolute paths but permits .., and assertBlind only prefix-matches, so a parent-relative spelling of the same file passes. Verified: attachment ../../pair/.pair/working/checkpoints/441.md → exit 0, accepted into a blind:true review packet (while the sibling ./.pair/working/../working/handoff.md in the same call was rejected — so the guard looks alive while the traversal spelling walks past it). This is not contrived: worktrees live at ../pair-worktrees/<id>, so the main checkout's working area genuinely IS ../../<repo>/.pair/working/... from a dispatched subagent's cwd. Fix: reject any attachment whose normalized form starts with .. — a packet references paths inside the project, and the type already says so.
• packages/knowledge-hub/src/tools/codex-fanout.ts:644-667 — collect's schemaOverride replaces the phase contract instead of tightening it, and the party composing that JSON request is the model. Verified: {"phase":"review","result":{"status":"ok","value":{}},"schema":{"type":"object"}} → {"outcome":"completed","advances":true} for an empty review return. A model that truncates or mis-copies the schema turns AC6's fail-closed guarantee into a no-op at the exact seam where the untrusted actor sits, and under ## Auto-Advance that advances a card on a review that returned nothing. Fix: validate against the built-in contract ALWAYS, and let an override only add constraints (validate against both), or drop the parameter and resolve the enum-locked contract from disk (see Minor 2).
• packages/knowledge-hub/src/tools/codex-fanout.ts:742-756, 800-818 — state.halted is set by any non-completed outcome and is never cleared, while reconstructState reads the whole append-only audit with no run/iteration scoping (AuditRecord.iteration is parsed and then ignored). Verified against the shipped asset with a two-line audit — implement timed-out at iteration 1, implement completed at iteration 2 — resume returns {"redispatch":[],"halted":true,"note":"halted by a previous iteration: implement ended timed-out — not re-driven"}. So: a card whose review times out on Monday is refused by every future pair-loop invocation, forever, because the audit is one persistent project-relative file (automation-policy.md: absent section ⇒ automation/loop-audit.md under working_path). Only hand-editing an append-only audit unblocks it. That is stricter than the Claude lane, whose exclusion is explicitly scoped to "every later iteration in the same run", and it contradicts AC9 ("re-dispatches only what is unfinished"). Fix: clear halted when a later record for the same phase is completed, and scope halt reconstruction to the current run (carry the run/iteration boundary into resume).
• .claude/skills/pair-loop/SKILL.md:78-84 (Step 1b.3) + codex-fanout.ts:800-818 — the Codex branch has no review↔fix convergence: resumePlan returns ["implement","pr","review","fix"] for a fresh card (verified), and 1b.3 says to dispatch "each phase it still owes (implement → pr → review → fix)" with no branch on the review's verdict or findings. Two concrete wrong outcomes. (a) Review returns {"verdict":"APPROVED","findings":[]} → collect says completed/advances → a fixer subagent is nevertheless spawned on an approved PR with nothing to fix. (b) Review returns CHANGES-REQUESTED with 5 actionable findings → fix runs once → {"fixed":true} → all four phases are recorded complete and the card is treated as converged, with the fixes never re-reviewed; under ## Auto-Advance that PR merges. The Claude realization it claims parity with does the opposite: MAX_FIX_ROUNDS = PIPELINE.maxFixRounds (default 3, pair-implement-batch.js:442,779), a severity floor, an actionable-finding count, and re-review each round until zero actionable findings or escalation. ADR-021's own new consequence bullet ("a given card comes out of both with the same outcome") is not met. Fix: make fix conditional on the review's returned actionable findings, re-dispatch review after each fix, cap the rounds, and escalate at the cap — stated in 1b.3 and reflected in resumePlan so a resumed card does not owe a phantom fix.

Minor (must fix before merge — same bar as Major, just lower impact)

• packages/knowledge-hub/src/tools/codex-fanout.ts:190-199 — matchRealization applies probe.namespace to every map entry, including CODEX_MULTI_AGENT_V1, which has no namespaceKey (its handles are un-namespaced by construction). A session that exposes the default-on v1 tools while reporting the configured v2 tool_namespace therefore matches nothing. Verified: {"probe":{"tools":["spawn_agent","wait_agent"],"namespace":"agents"}} → {"tier":3,"realization":"degraded-one-card"} — a false-negative degradation that silently costs the whole tier-1 fan-out on a session that has it. Fix: apply the override only to entries that declare namespaceKey.
• packages/knowledge-hub/src/tools/codex-fanout.ts:357-378 + SKILL.md 1b.3 step 4 — the review phase is validated against LOOSE_REVIEW_SCHEMA only. The Claude lane treats that as its fallback: phase-0 ensure-contract.mjs derives an enum-locked schema plus vocabulary/severityRanks from the review template and uses those for convergence. The Codex path has no equivalent and the skill never passes one, so {"verdict":"looks good to me"} collects as completed/advances (verified) where the Claude lane with a fresh contract rejects it. AC6 says "the same JSON-Schema result contract the Claude workflow uses"; today it is the same skeleton, not the same contract. Fix: have 1b.3 resolve the generated contract when present (it is the same artifact ensure-contract.mjs writes) and pass it as the phase schema, falling back to the skeleton.
• packages/knowledge-hub/src/tools/codex-fanout.ts:644-655 — an unrecognised phase reaches PHASE_CONTRACTS[phase] undefined and surfaces as {"error":"Cannot read properties of undefined (reading 'schema')"} (verified with phase:"deploy"). Exit code 1 keeps it fail-closed, but AC10 asks for a named outcome and the taxonomy has one. Fix: guard with isPhase and return failed-validation with the same "an outcome this file cannot name is never read as success" wording used for statuses.
• apps/website/content/docs/integrations/codex.mdx:82 — "Enable the second generation and you additionally get a harness-side concurrency ceiling the loop will respect" is inaccurate: the surface map declares agents.max_concurrent_threads_per_session as v1's bounding.concurrencyKey too, so a default-on v1 session also contributes a harness ceiling. A reader enables a default-off feature for a property they already had. Fix: say the second generation makes the ceiling and the timeouts configurable, not that it is what provides them.
• .pair/knowledge/guidelines/technical-standards/ai-development/skill-conventions/harness-realization.md:59 — the convention prescribes a fail-closed shape the implementation cannot express: "An entry nobody could verify ships with no handles", while RealizationHandles.spawn/wait are required non-optional strings (codex-fanout.ts:48-57). The next maintainer following the convention on this repo's only implementation cannot do what it says. Fix: either make the handles optional and have matchRealization treat an entry without them as unbindable, or state the fail-closed shape as "an unverifiable entry is not added to the map".

Questions (informational, never blocking)

• DoD — Manual dogfood — the unchecked box ("one Codex run over a narrow perimeter, leaving a readable audit") is the only evidence that the model-driven half of this realization sequences correctly; every automated guard covers the deterministic half. Major 5 is exactly the class of defect a dogfood run surfaces and the suite structurally cannot. Non-actionable in code — it needs a live Codex session — so it is carried to the human merge gate, but it should be weighed against Major 5 rather than ticked.

Positive feedback

• The result-contract parity guard is real, not decorative: it slices the workflow's own STEP_SCHEMA…FIX_SCHEMA block out of source, throws loudly if the markers move, and the four schemas are byte-equivalent to the module's copies. Verified by reading both sides.
• The containment guard genuinely enforces the anti-corruption claim — vendor tokens are asserted absent from the module below the map section and from both skill copies. The exclusion of bare spawn/wait, with its stated reason, is the right call.
• build-kb-asset.ts is a correct generalization: same transpile options, same header derivation, same catch {} re-fill, and the ratchet's output is byte-identical (its own drift test covers it).
• Fail-loud audit via append-and-read-back rather than a returned flag is the right shape for AC8.
• Both factual corrections (ADL clause struck in place with the probe evidence, ADR-021 amended with §7 rather than duplicated) do exactly what AC12 requires of whichever story lands second.

Functionality & requirements (AC coverage)

AC	Verdict	Note
AC1 probe/announce/data	partial	probe + announcement correct; namespace override misapplied to v1 (Minor 1)
AC2 degrade in order	met	tier 2 when externalDriverAvailable, else tier 3; reason carried into the announcement
AC3 fresh subagent, one packet	met (shape)	one card enforced; compact result only
AC4 role text in the request	met	ROLE_INSTRUCTIONS travel in the packet; no profile dependency
AC5 reviewer blindness	not met	two reproduced bypasses (Major 1, Major 2)
AC6 one contract, fail-closed	not met	caller-supplied schema replaces the contract (Major 3); loose skeleton only (Minor 2)
AC7 min of three ceilings	met	verified incl. 0 and the binding-limit line
AC8 audit on disk, fail loud	met	append + read-back + throw
AC9 resume only the unfinished	not met	sticky cross-run halt (Major 4); no second PR — met
AC10 terminal outcomes	mostly met	taxonomy + fail-closed statuses; unknown phase escapes as a TypeError (Minor 3)
AC11 Claude unchanged	met	no file under .claude/workflows/ or .claude/agents/ in the diff
AC12 one record, amended	met	ADR-021 §7 + ADL correction
AC13 distribution untouched	met	asserted against the real registry set
AC14 mocked-primitive coverage	partial	the listed cases are covered; the three reproduced defects have no test (traversal, working_path, halt-then-complete)

Testing & quality gates

• Coverage is strong on the happy paths and on every case AC14 enumerates; the gaps are the ones above — no test spells an attachment with .., none sets a non-default working path, none feeds an audit where a halted phase later completes, none exercises a v1 session reporting a namespace.
• Quality gates: not re-run here (no node_modules in the review worktree). All findings were reproduced by executing the shipped .pair/knowledge/assets/codex-fanout.cjs on bare node, which is the artifact the skill actually invokes.

Adoption compliance

• Degradation level: 1 — every referenced adoption file exists and was read.
• No new dependency; the module imports node builtins only, as build-kb-asset.ts requires.
• Patterns match architecture.md (the new Unattended Fan-Out section is consistent with what ships) and ADR-023's generated-asset pattern.
• ADRs present: ADR-021 §7 amendment + ADL correction. No missing decision record.

[rucka]

Verdict

risk:red · cost:green — APPROVED

Open findings: 0. All Major/Minor findings from prior rounds are resolved and verified on the current head cf967d999c3f1d90a32560013d22ce916214456b.

PR: #469 · Story: #441 · Author/reviewer: rucka · Date: 2026-08-30

risk:red still requires a non-author human approval on this exact head before merge; this self-review cannot satisfy that authorization requirement.

Classification

Dimension	Assessment	Result
Criticality	delivery orchestration and audit trail	yellow
Change risk	fan-out lifecycle, resume safety, generated distributed asset	red
Business impact	repository-wide autonomous-workflow control	yellow
Security	command policy remains deny-first; no auth/trust-boundary change	green
Coupling	existing knowledge-hub tool plus intentionally synchronized asset/mirror	balanced
Cost	no billable/external-resource surface	green

Existing labels remain correct: risk:red, cost:green.

Findings

Critical

None.

Major

None.

Minor — resolved

Finding	Resolution	Verification
Audit history could be associated with an unstamped run	Non-empty run is now required for audit records and named resume	TDD RED → GREEN
Reviewer prompt could see only attachments	Deny-first blindness now also rejects card title and notes	TDD RED → GREEN
Binding omitted the selected harness concurrency key	Binding returns concurrencyKey; loop guidance halts fan-out when the ceiling cannot be resolved	TDD RED → GREEN
ADR property count contradicted its list	Corrected three/four mismatch	Documentation review
Missing cap produced a raw TypeError	Ceiling validation returns a normalized domain error	TDD RED → GREEN

Questions / residual scope

The story's manual Codex dogfood DoD remains unexecuted because it would act on a real eligible card. It is explicitly recorded as external side-effect scope, not misclassified as a Major/Minor.

Review evidence

Area	Result
Functionality / AC	Covered, including audit identity, resume safety, reviewer blindness, concurrency resolution, and error normalization
Test-first remediation	Six focused red tests failed before source changes and pass after them
Target tests	@pair/knowledge-hub: 51 files, 5,064 tests passed
Generated asset	codex:asset regenerated; source and packaged asset/mirror verified together
Quality gate	pnpm quality-gate passed on exact head
CI	build, preview, secret scan, smoke: passed
Security	Green — deny-first command policy preserved; no new authorization or vulnerability finding
Adoption	Level 1 conformant; ADR aligns with implementation; no new dependency or migration
Debt	None introduced
Documentation	Loop skill, packaged mirror, Codex integration docs, and ADR updated consistently
Performance / deploy	No runtime hot path, migration, or deployment impact

Why prior reviews kept finding issues

Earlier fixes closed only the directly reported representation: attachment text but not card title/notes; configuration map but not returned binding; audit record prose but not resume identity; source but not every distributed representation. The new tests cover these sibling boundaries and the shipped asset, so the review evaluates the whole contract rather than a single path.

[rucka]

Synthesis of the whole review↔fix cycle, replacing the round-4 escalation flush (now minimized, per the convention that comment itself stated). Round-by-round detail stays in the untracked working log .pair/working/reviews/441.md in the persistent authoring worktree ../pair-worktrees/441.

Convergence — round 4 (commit cf967d99)

The escalation raised 5 actionable findings (2 Major, 3 Minor). All five were remediated test-first and verified by an independent re-review on this exact head (review — APPROVED, 0 open Critical/Major/Minor).

Per finding:

• [Major] The run stamp was mandated in prose only — its omission failed OPEN (codex-fanout.ts:1324-1345 assertAuditable, :1531-1537 canHalt) → assertAuditable now refuses any record whose run is not a non-empty string, for kind:'card' and kind:'run' alike, and parseAudit/runResume refuse an unstamped audit on a named resume rather than reconstructing it as another run's history. The escalate-without-stamps → re-invoke-with-run-id → silent auto-advance sequence is closed at both the write and the read end — packages/knowledge-hub/src/tools/codex-fanout.ts, codex-fanout.test.ts, .pair/knowledge/assets/codex-fanout.cjs (+ dataset mirror)
• [Major] AC5 blindness scanned attachments only, so a working-area pointer in card prose reached the blind packet verbatim (:735-756 assertBlind, :845-869 buildPacket) → new assertBlindCardText runs the same blindDenyPrefixes scan over card.title and card.notes whenever contract.blind, rejecting before spawn with the existing message shape — codex-fanout.ts, codex-fanout.test.ts, the asset + mirror
• [Minor] The concurrency ceiling had the wait bound's shape but not its obtainability (:196-236 bounding, :303-317 Binding, pair-loop 1b.2, codex.mdx:78) → Binding now returns concurrencyKey alongside waitTimeoutKeys; the skill reads it when the session reports no ceiling and halts the fan-out rather than dispatching an unbounded batch; the overstated codex.mdx sentence corrected — codex-fanout.ts, .claude/skills/pair-loop/SKILL.md (+ dataset source), apps/website/content/docs/integrations/codex.mdx
• [Minor] ADR-021 §8 said "Three properties" over a four-bullet list → corrected to four, so the fourth (model-omission-fails-closed) reads as part of the fixed cross-realization set a third realization must hold — .pair/adoption/tech/adr/adr-021-fan-out-three-realizations.md
• [Minor] cap with no ceilings surfaced a raw TypeError (:1704) → checkedCeilings rejects a missing/non-object ceilings in the module's house style, naming the fields it needs — codex-fanout.ts, the asset + mirror

Not changed (escalated to the human): the story's manual Codex dogfood DoD line — it would drive a real eligible card and create external delivery side effects, so it is a scope decision at the merge gate, not a defect. Recorded as Questions / residual scope in the review, and flagged in the PR body's Testing section rather than silently checked.

Rounds, for the record

round	commit	verdict	outcome
1	ca8600e7	fix	5 Major + 5 Minor — blindness workingPath input, attachment .. rejection, tightening-only schema override, run-scoped halts, converge + fix-round cap
2	c5eb965d	fix	2 Major + 3 Minor — dispatch-shape realization map (no product-name special case), root/packet/card key handling incl. findings, probed wait bound, AuditRecord.kind
3	3ba02891	fix	2 Major + 1 Minor — the review action stamp, v1's declared wait-bound fallback + waitTimeoutSource, unknown probe/ceilings keys
4	cf967d99	converged	the 5 findings above; independent re-review on this head returns 0 open Critical/Major/Minor

Verification on the current head cf967d99

Gate	Result
pnpm quality-gate	PASS (exit 0) — re-run at publish time
CI: build, smoke, secret-scan, preview	PASS
pair-review status on head	success — APPROVED — 0 open Major/Minor findings
Codex suites	167 passing (codex-fanout.test.ts 126, codex-fanout-asset.test.ts 23, codex-realization.test.ts 18)
@pair/knowledge-hub	5,064 passing

The PR description has been refreshed to describe this head (the round-4 semantics above, and the corrected counts) rather than the round-3 one.

PR state — pr-state:to-be-reviewed is correct, not stale

Computed with the shipped, provider-agnostic evaluator rather than asserted:

resolve_tier "risk:red,pr-state:to-be-reviewed"                 → red
resolve_pr_state pass approved red 0                            → to-be-reviewed
  (pr-state: tier 'red' requires explicit human approval)
merge_allowed to-be-reviewed                                    → NO

Green gates + an APPROVED review at 🔴 with no explicit human approval on the current head synthesizes to to-be-reviewed — the fifth row of the synthesis table in pr-states.md. The label therefore stays as it is: the review is converged, the human approval is what remains. This repo is single-maintainer and Review enforcement is disabled, so pair-review here is advisory and the 🔴 approval rule is a convention, not a branch-protection block — which makes this line the operative one rather than a formality.

Nothing has been merged. The merge is a human act.