Skip to content

[US-479] refactor: delivery workflow TO BE — coordinator + phase skills, one squash with the 2026-09-08 hardening - #480

Draft
rucka wants to merge 43 commits into
mainfrom
feature/US-479-delivery-workflow-to-be
Draft

[US-479] refactor: delivery workflow TO BE — coordinator + phase skills, one squash with the 2026-09-08 hardening#480
rucka wants to merge 43 commits into
mainfrom
feature/US-479-delivery-workflow-to-be

Conversation

@rucka

@rucka rucka commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

PR Information

PR Title: [US-479] refactor: delivery workflow TO BE — coordinator + phase skills, one squash with the 2026-09-08 hardening
Story/Epic: #479 (epic #212)
Type: Refactor
Priority: High
Assignee: @gcarucci
Labels: user story

Summary

What Changed

The branch starts at 7b559003 (the squashed 2026-09-08 hardening, removed from main on 2026-09-08 so that this PR's squash-merge brings it back together with the TO BE work). Commits land in this order, engine usable and tests green after each:

  • 05864b162 pair-loop.js halts on every status ≠ ready-for-merge; implement-batch CONTRACT lists every emitted status
  • 154e9578a custody/history machinery removed; rebase ⇒ stale
  • 236cf29b8 60429507 fase D: planner D0, skills pair-workflow-red-spec / red-verify / green-fix / p3-verify, seal + blob-check as node scripts, one handoff JSON per phase
  • 3e1886ea2 fase C: pair-workflow-review-phase, declared review set per risk tier
  • 4e2e9d9e8 fasi A/B → pair-workflow-implement-phase / pr-phase
  • 57f4cbb5a changelog-comments → ADR; workflowVersion; zero prompt / zero shell in pair-implement-batch.js
  • 6–7 — custody scripts ship INSIDE the skills that run them (contract-phase/scripts/ensure-contract.mjs, red-seal/scripts/red-snapshot.mjs, byte-identical copy in p3-verify/scripts/, drift-guarded by test) so a skill is portable as one folder on any harness; skill examples neutralized

Why This Change

pair-implement-batch.js (2222 lines) carries phase policy in free-form prompts, orchestrator-side semantics, and a rebase-repair layer (custody/history) that a stale-head rule makes unnecessary. Reference design: .pair/working/reports/delivery-workflow-to-be.md, with the corrections recorded in #479.

Story Context

User Story: see #479
Acceptance Criteria: to be fixed during refinement of #479

Changes Made

Files Changed

  • Modified: .claude/workflows/pair-implement-batch.js, pair-implement-batch.test.mjs, pair-loop.js, pair-loop.test.mjs, .claude/agents/*, dataset mirrors, batch-engine.mdx
  • Added: .claude/skills/pair-workflow-*/ (11, three of them with a scripts/ folder), .claude/agents/pair-remediation-planner.md, ADR-024
  • Deleted: .claude/agents/pair-custody-verifier.md, pair-red-domain-mapper.md
  • Moved: ensure-contract.mjspair-workflow-contract-phase/scripts/

Testing

Test Coverage

  • Unit Tests: workflow dry-run harness (pnpm workflows:test, 378 tests incl. the two US-479 acceptance criteria), conformance/mirror suites (4905), website (177)
  • Integration Tests: red-snapshot.test.mjs — 12 scenarios on throwaway git repositories (seal idempotency, blob identity, unlisted test, out-of-scope, behavioral module add, rebase ⇒ snapshot-missing, CLI exit codes)
  • Manual Testing: one story to ready-for-merge on the new engine (canary) — proposed: this PR's own final review via pair-implement-batch in prNumber mode

Quality Assurance

Code Quality Checklist

  • Code follows established style guides and conventions
  • Error handling implemented for edge cases
  • No debugging code or console logs left behind

Review Areas

  • Business Logic: every emitted status handled by pair-loop
  • Code Structure: no prompt / shell / severity rule left in the .js
  • Testing: each skill testable from a manifest fixture without the orchestrator

🤖 Generated with Claude Code

Review classification

risk:red · cost:green — review matrix (2026-09-09)

Classification matrix — per dimension
Dimension Tier Source Note
Service/domain criticality green tech/risk-matrix.md All touched deployables are listed Low.
Change/diff risk red PR diff Workflow/phase contract refactor across 90 files.
Business impact green Generic development-tooling subdomain Internal delivery tooling.
Security relevance yellow assess-security review Local workflow input reaches shell commands; validation is security-adjacent and covered by tests.
Coupling balance green review Typed phase boundaries; mirror and script-identity tests cover the distribution boundary.

Cost: green — no cost surface touched. Review classification has no refinement floor: #479 has not been refined.

…y, domain map, preflight, P3 carry-forward

Squash of the fifteen direct-to-main hardening commits of the pair-implement-batch engine
(2026-09-01 … 2026-09-08: front-load review contract coverage; prove external boundaries; the review
severity floor defaults to Minor; prove claims and collisions; preflight review fixes; lock red contracts
before fixes; seal RED contracts in Git; bind RED fixtures to tests; escalate history before seal;
harden sealed fix loop; carry P3 evidence forward; guard sealed custody; repair one RED contract;
map RED domains; retain grammar tokens), kept as the shipped state of fact: engine, tests, the
pair-red-*/pair-fix-*/pair-custody-verifier agents, their dataset mirrors, batch-engine.mdx and
apps/pair-cli/config.json.

The adoption records the 2026-09-03…09-08 commits carried (11 decision-log entries, four Review
Convergence bullets in way-of-working.md) are NOT part of this squash: the maintainer declared that
process no longer valid on 2026-09-08 and they never enter this history; the engine keeps
DEFAULT_SEVERITY_FLOOR with its rationale in the code comment. The three records that remain valid
(2026-08-31-review-baseline-and-provisioned-artifact-contract, 2026-09-01-review-contract-inventory-
prevents-serial-findings, 2026-09-01-external-boundary-proof-prevents-false-equivalence) and their
three Review Convergence bullets ARE part of it. The code stays as the product's current behaviour.

Refs: #219

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@rucka rucka added the user story Work item representing a user story label Sep 8, 2026
…ract names every status it emits

pair-loop matched halted cards on `escalate` / `failed*`. The engine also returns
`seal-invalidated` and `stale-history-decision`: those cards were neither halted nor
parked, so they were re-selected and re-driven on every iteration up to max-iterations.
Rule inverted to a deny-list of one — only `ready-for-merge` may advance — and the
audit-resume prompt reads the same rule. implement-batch's CONTRACT block now lists
every status the code emits and states that a caller must halt on any status it does
not recognise. Dataset mirrors synced.

Test: pair-loop.test.mjs "ANY non-ready status halts the card" (red before, green after).

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ever repaired

Removed from pair-implement-batch: the custody probe (pair-custody-verifier agent, Custody
phase, CUSTODY_SCHEMA), card-level `custodyReset` and `historyDecision`, the `supersedes`
seal field, `applyHistoryDecision` / `textualHistorySubjects`, the `history-subject` finding
kind, and statuses failed-custody / failed-custody-reset / seal-invalidated /
stale-history-decision. −246 lines of engine, 14 tests retired, 3 added.

Rule that replaces it (TO BE §3.4/§7.5): an in-flight attempt whose base moved fails closed
where it is measured — sealer refuses a HEAD ≠ base, preflight refuses a non-ancestor
snapshot — and a resumed run starts a fresh review on the current head; older snapshots
are historical evidence. A history-only finding stays a typed human escalation
(`humanDecisionKind: history-rewrite`) taken before RED/seal/GREEN; nothing in the engine
can accept or waive it.

pair-reviewer.md: history clause reduced to the escalation. batch-engine.mdx: custody agent
row and the two custody sections replaced by "History rewrites and rebases". Dataset mirrors
synced.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
rucka and others added 2 commits September 8, 2026 22:56
…GREEN/P3 dispatched by name; Git custody in red-snapshot.mjs

Six skills in a new `workflow/` category (installed as pair-workflow-*): remediation-plan (D0),
red-spec (D1, domain map folded in), red-verify (D2), red-seal (D3), green-fix (D4), p3-verify
(D5). Each declares arguments, one mutation boundary and a handoff JSON under
.pair/working/runs/<runId>/<story>/. pair-implement-batch.js now names the skill and passes
typed args ($run $story $pr $phase $base $worktree $branch + phase JSON); the fase-D prompt
constants, the mapper agent and its schema are gone. One frozen plan per round; groups run
sequentially, each on the previous group's verified head; a plan that drops/duplicates/invents
a finding index is failed-plan. `args.runId`, `models.planner` (replaces redMapper),
`pipeline.skills.{remediationPlan,redSpec,redVerify,redSeal,greenFix,p3Verify}`.

red-snapshot.mjs: deterministic `seal` (HEAD == base, sha256 per artifact, tree dirty only at
the contract, one --no-verify commit with the Pair-RED-Snapshot trailer, idempotent) and
`verify` (one snapshot by trailer, parent == base, tree == manifest+artifacts, sealed blobs
byte-identical at HEAD, no unlisted test change, fixScope respected, no module added under a
behavioral scope). 12 tests on throwaway repos. Sealer (sonnet) and P3 agents run it.

Conformance checker learns the `workflow` category (four-part breakdown); KB/docs counts 44 → 50;
skills-catalog, skills-guide, next catalog gain a Workflow section; batch-engine.mdx rewritten
for the phase flow. ADR-024 records the decisions (storage authority, repair budget 1, local
snapshot commit, rebase never repaired, agent budget as AC). Dataset mirrors synced.

Tests: 375 workflow (dry-run harness + red-snapshot), 4900 knowledge-hub.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…k tier) and cycle-comments (probe / flush / synthesize)

pair-workflow-review-phase owns the independent review: it reads the PR's risk:* label and
declares its passes before running (general via the configured review skill on green; +
security and boundary lenses on yellow; + architecture and an adversarial second pass on red),
unions and deduplicates to one finding set with the template vocabulary, and carries the rules
the engine used to spell in a 5 KB prompt — pacing, blindness, finding shape and acceptance
plan, evidence ledger / boundary proof / lossless diagnostics, no-new-issues, nonActionable +
disposition, history-rewrite escalation, TEXT SHAPE — plus the three modes (first posts with the
hidden marker; re-review inspects only the fix delta, silent; fresh is a full silent pass).
pair-workflow-cycle-comments owns the PR-comment policy: probe (log present? marker on the PR?),
flush (escalation, superseding any prior flush, log kept), synthesize (ONE remediation table
across every run, intermediates minimized, log deleted).

pair-implement-batch.js dispatches both by configured name with typed args ($mode, $marker,
$template, $severities, $verdicts, $reviewSkill, $writeIssue, $priorFindings, $priorHead,
$hasLog, $accepted); the reviewer prompt, probe prompt, both flush prompts, the synthesis prompt,
CONTRACT_INVENTORY, revWtClause and the flush helpers are gone from the workflow. pair-reviewer
agent thinned to a skill executor. Counts 50 → 52; catalogs, batch-engine.mdx and ADR-024 updated.
Dataset mirrors synced.

Tests: 375 workflow, 4902 knowledge-hub, 177 website.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…phase, pr-phase; the workflow dispatches no free-form prompt

pair-workflow-contract-phase (ensure-contract.mjs check/write, severityRanks from meaning not
order), pair-workflow-implement-phase (worktree isolation incl. stacked stories, implement process
test-first, finite-state / evidence / collision / lossless / boundary disciplines, tier gate,
decisions, checkpoint) and pair-workflow-pr-phase (resume from checkpoint, publish through the
configured publish-pr skill, TEXT SHAPE, review-dispatch-required signal). The coordinator passes
$run $story $branch $worktree $base $stacked + the configured skill names + $notes; wtClause,
revWtClauseBase, TEXT_SHAPE, FINITE_STATE_COMPLETENESS and their sub-constants are gone. One run
directory per story for every phase (args.runId, default story-<id>); every invocation names its
story ("for story #<id>"). p3-verify owns its detached-worktree isolation. pair-implementer and
pair-contract-generator thinned to skill executors. Counts 52 → 55; catalogs, batch-engine.mdx,
ADR-024 updated. Dataset mirrors synced.

Tests: 375 workflow, 4905 knowledge-hub, 177 website.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…istory — workflowVersion, zero-prompt / zero-shell AC

pair-implement-batch.js: 56 changelog-style comment blocks (issue numbers, "measured", round-by-
round defect narration — 614 lines) compacted into behavioural comments; the defect history is
recorded once as an appendix of ADR-024. 1929 → 1671 lines (2222 at the start of US-479).
WORKFLOW_VERSION is returned with every result and handed to every phase skill as
$workflowVersion, so each handoff records the coordinator that produced it.

Acceptance criteria as tests: every agent dispatch is a skill invocation
(`Invoke **<configured skill>** for story #<id> with <typed args>`), no dispatched prompt carries
a git/gh/node command, none of the retired prompt rules survive in code, and the version reaches
every skill on every run.

Tests: 378 workflow, 4905 knowledge-hub, 177 website.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…skills run on every harness

red-snapshot.mjs and ensure-contract.mjs move from .claude/workflows/pair-contracts/ (a registry
that installs only in Claude Code) to .pair/knowledge/assets/, the channel every harness installs
(same as coverage-ratchet.cjs, ADR-023). red-seal, p3-verify and contract-phase invoke
`node .pair/knowledge/assets/<script>`; the dry-run tests and the contract cache's .gitignore stay
under pair-contracts/. Skill description examples no longer name this repo's PR (#480#42).
Dataset mirrors synced; ADR-024 and batch-engine.mdx updated.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ensure-contract.mjs ships in contract-phase/scripts/; red-snapshot.mjs ships in red-seal/scripts/
(seal) and, byte-identical, in p3-verify/scripts/ (verify) — red-snapshot.test.mjs fails if the
two copies (installed and dataset) drift. Each SKILL.md resolves its script from its own
directory ($SKILL_DIR/scripts/…), never from a repository path, so a skill is portable as one
folder on any harness. .pair/knowledge/assets/ no longer carries them. eslint ignores
dataset/.skills/**/scripts/*.mjs on the same one-artifact rule as the workflows. Dry-run tests
stay in .claude/workflows/pair-contracts/ and import the skill-local scripts.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@rucka rucka added the risk:red Classification: high risk tier label Sep 9, 2026

@rucka rucka left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

risk:red · cost:greenCHANGES-REQUESTED — la trasformazione tecnica e i gate sono verdi, ma la story non ha AC verificabili e manca il canary end-to-end richiesto.

Open findings: 2.

PR: #480 · Author: rucka · Reviewer: rucka (self-review) · Date: 2026-09-09 · Story: US-479 · Type: refactor

Classification matrix — per dimension
Dimension Tier Source Note
Service/domain criticality green risk matrix Touched deployables listed Low.
Change/diff risk red 90-file diff Workflow/phase contract refactor.
Business impact green subdomain Generic development tooling.
Security relevance yellow security review Local config reaches shell boundaries; validation is security-adjacent.
Coupling balance green coupling review Typed phase contracts and mirror/identity tests keep boundaries balanced.

Tier is risk:red; cost is cost:green. Refinement floor unavailable because #479 is not refined.

Assessments

Security — Input validation

Verdict: yellow — workflow card/pipeline inputs reach shell-running agents; reject-path validation is present and tested.

Security — Output handling

Verdict: green — no web/API output surface added.

Security — Authentication

Verdict: green — no authentication surface changed.

Security — Authorization

Verdict: green — no authorization surface changed.

Security — Introduced vulnerabilities

Verdict: green — 0 introduced, 0 pre-existing findings.

Cost

Verdict: cost:green — no paid-provider, IaC, queue, scheduler, media, or LLM cost surface added.

Architecture (Coupling)

Verdict: green — phase handoffs are typed and skill-local scripts are identity-checked across their required copies.

Bug fix — Red test before fix

Verdict: not applicable — not a bug fix.

Details

Findings by severity

Critical

  • None.

Major

  • [Story #479 — Story Context] Acceptance criteria are explicitly “to be fixed during refinement”. Refine the story with measurable AC before merge; the review cannot validate the claimed workflow contract against an unrefined story.
  • [PR #480 — Testing / Story #479 DoD] The required end-to-end canary is still unchecked. Run one story through the new engine to ready-for-merge and attach durable evidence (run/phase handoffs, one first review, final synthesis).

Minor

  • None.

Questions

  • None.
Positive feedback
  • ADR-024 records the new phase-skill architecture.
  • Script copies and dataset mirrors are guarded; red-snapshot copies are byte-identical.
  • No new dependencies; CI and local full quality gate pass on da1eb9a.
Functionality & requirements
  • FAIL — story AC absent; required canary not executed.
  • The static workflow contract, phase dispatch, and mirror behavior are covered by the automated suite.
Testing & quality gates
  • PASS — pnpm quality-gate passed locally on PR head da1eb9a; published format, secret-scan, build, preview, and smoke checks are successful.
Adoption compliance
  • Level 1 — tech stack, architecture/ADR, security, coding standards, and infrastructure conformant. No unlisted dependency; ADR-024 is present.
Tech debt
  • 0 introduced items flagged.
Documentation
  • PASS — workflow documentation, skills catalog, and dataset mirrors updated.
Performance & deployment
  • No runtime hot-path or deployment change; rollback is the normal PR revert/squash workflow.

@rucka rucka added the pr-state:not-approved PR state: changes requested label Sep 9, 2026
@rucka

rucka commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Review finding 1 (Major — story AC) — resolved. #479 refined via /pair-process-refine-story + /pair-process-plan-tasks (non-interactive, assumptions recorded): 10 Given-When-Then AC (1–9 mapped to the automated suites, 10 = end-to-end canary), business rules, edge cases, classification risk:red · cost:green, DoD, sizing 8 pts, dependencies, validation strategy, technical analysis, task breakdown T-1…T-9 with dependency graph and AC-coverage table. Board: Refined, assignee set, risk:red label.

Finding 2 (canary) — in progress: pair-implement-batch running on #321 (risk:green, no prior branch) with runId: canary-479, maxParallelism: 1, from this branch's checkout. Evidence (run result, per-phase handoffs, agent count) is kept under .pair/working/canary/us-479/ (untracked working area) and summarized here.

…typed refusals are answers, handoffs in the main checkout

Canary run 1 (story #321, PR #481, runId canary-479): fases 0/A/B/C and the planner worked; the
RED author refused group r1-g1 as `split-required` — a guard-strength finding (positional-blind
assertion) with production already correct has no RED against unfixed production — and the engine
re-dispatched the identical prompt once before ending the card as failed-red-contract (8 agents).

- Third group mode `test`: allowedPaths [], RED proven against an injected regression, no GREEN,
  P3 verifies the sealed head; red-snapshot.mjs treats any production change under a test scope
  as `test-mode-production-change` (script test added; both copies synced).
- A typed refusal (`stale`, `split-required`; planner `stale`) is routed by status and never
  retried with the same prompt; the result carries `redRefusal` and `splitReason`.
- Handoff directory rule: `.pair/working/runs/<run>/<story>/` in the MAIN checkout the coordinator
  was started in, never a story/review worktree (the review's detached worktree took its handoff
  with it) — stated in every skill's $run row and in the dispatch text.

ADR-024 amended (§8–10); batch-engine.mdx and the remediation-plan description updated.

Tests: 382 workflow (+2 coordinator, +1 script), 4905 knowledge-hub, 177 website.

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@rucka

rucka commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Canary run 1 (canary-479, #321 → PR #481) — fases 0/A/B/C and the planner worked (implementation committed test-first, PR opened with labels + pr-state, first review posted with the hidden marker, 3-group plan); the card ended failed-red-contract after 8 agents (≈580k subagent tokens, 46 min). Root cause: a guard-strength finding (positional-blind assertion, production already correct) has no RED against unfixed production, the RED author refused split-required, and the engine re-dispatched the same prompt once. Evidence for run 1 is under .pair/working/canary/us-479/run-1/ (untracked working area).

Fixed in d230cb7c (c8): test group mode (RED proven against an injected regression, no GREEN, any production change is a breach), typed refusals routed by status and never retried identically, handoff directory pinned to the main checkout. ADR-024 §8–10.

Canary run 2 (canary-479-run2, resume on PR #481) is running.

@rucka

rucka commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Canary run 2 (canary-479-run2, resume on PR #481) — 8 agents, ≈470k subagent tokens, 32 min, failed-red-contract.

What c8 fixed is confirmed live: the probe recognised the existing first review (silent fresh re-review), the planner emitted 4 groups including a test-mode group for the guard-strength finding, every handoff landed in the main checkout (review-phase, remediation-plan, red-spec, red-contract, red-verify), and the typed refusal path was not needed.

Where it stopped: group r1-g1 (behavioral, the $tool token-table finding). The RED author produced a 10-row contract; the independent verifier rejected 2 rows as non-discriminating; the one bounded repair produced 12 rows; the verifier rejected the headline row A5 (keyword co-occurrence over joined prose instead of an assertion bound to the item that states validity). Terminal per ADR-024 §6 (repair budget 1).

Reading: the engine behaved as designed; #321 is a poor canary for D1–D5 — every finding is a regex guard over SKILL.md prose, where a "discriminating RED" is genuinely hard and the verifier is right to refuse. Seal, GREEN and P3 have still not run live. Evidence: .pair/working/canary/us-479/run-2/.

…low; way-of-working Review Convergence note

Refs: #479

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…g — under pnpm the installer resolved the bare target from it and landed in the monorepo

- caught by the pre-push quality gate (pnpm sets INIT_CWD); the suite now runs identically under node --test and pnpm workflows:test
- Task: T-17 — Prove the real engine boundaries before spending on live canaries

Refs: #479
…ory — scripts and suites scrub the GIT_* family; decoy-repo regression

- root cause of the 2026-09-09 canary incident class: the pre-push hook exports GIT_DIR/GIT_WORK_TREE to everything it runs; `git init`/`commit` spawned in temp fixtures then acted on the REAL repository (core.bare → true, fixture commits and a fixture worktree/branch on a story branch). Reproduced from the story worktree on this branch (reflog 20:45:01–03), repaired by hand (core.bare=false, fixture worktree/branch removed), remote untouched
- red-snapshot.mjs and cycle-state.mjs spawn git with a scrubbed env; every dry-run suite deletes the family at import; engine-boundaries runs the seal CLI and the whole custody suite under a hostile GIT_DIR pointed at a decoy repo and asserts the decoy's head, config, branches and worktrees unchanged
- Task: T-17 — Prove the real engine boundaries before spending on live canaries

Refs: #479
T added 10 commits September 9, 2026 20:54
…Workflow harness refused the script (canary run 11 launch); the NUL delimiter is spelled as an escape, guarded by test

Refs: #479
… in the Workflow sandbox and aborted canary run 11 at launch; wall time is read from the harness summary, guarded by test

Refs: #479
…rns to implement on the same seal once — never to the verifier; the seal manifest is removed in the first GREEN commit; no hook bypass; initial fixScope covers the story surface

- canary run 11 (engine 3.0.0): the first live GREEN published with gatesPassed:false — the sealed manifest .pair/red-snapshots/pr-0-a0.json failed the prettier gate and the implementer pushed with --no-verify; the coordinator failed closed (failed-implement) while cycle-state named verify next — the two disagreed
- cycle-state: implement-phase ok ⇔ gatesPassed true; a red gate is one bounded retry (greenRetries) on the same seal, then failed-implement; coordinator follows that next, binds the PR, never dispatches the verifier on a red gate; $attempt on implement
- implement-phase: remove the transient seal manifest in the first commit above the snapshot; the repository's own pre-push gate counts; --no-verify forbidden; decisions go to the decision log the contract scope covers
- red-spec: initial fixScope.allowedPaths = the story's implementation surface incl. .pair/adoption/decision-log/
- WORKFLOW_VERSION 3.0.1 (compatible major) — the canary continues on it as a declared successor

Refs: #479
… revises it — a0-rev<m> is a phase (prepare revision → validate + successor seal → implement again → re-review); the initial chain's seal identity is pr=0

- canary run 11 (3.0.1): the first verification found a Minor contract-gap on the initial contract (groupId a0) and asked for prepare/a0-rev2; cycle-state and the coordinator admitted revisions only for remediation groups → failed-resume (fail-closed, no judgment lost)
- cycle-state: a0(-rev<m>) phases; sealed a0 revision → implement (attempt counted per phase); implement ok after a review → re-review of prior findings + delta, never a second first review; coordinator PHASE_RE aligned; dry-run + cycle-state tests for the whole path
- skills: SEAL_PR = 0 for the initial chain and its revisions (Question r0-3 of the live review), $pr for remediation groups; implement-phase accepts a0-rev<m> and removes pr-0 manifests
- WORKFLOW_VERSION 3.0.2 (compatible major): the canary continues from r0-review-phase as a declared successor

Refs: #479
…s it on every next, the coordinator refuses a verification or a fix without it (canary run 11 finding r1-5: markers read PR#null on a fresh-path resume)

Refs: #479
…self-redirect is failed-resume; the inputs digest is keyed by engine major

- canary run 11 (3.0.3): the verifier for r2 returned a redirect to itself; VERIFY_SCHEMA required the verdict fields, the harness rejected the structured output and re-prompted a finished agent → 6 stalls, card died (3.75 h)
- schemas require status only (evidence checked by the coordinator's own predicates); a stage redirecting to its own step is refused with an explicit reason; skills state 'when next names THIS dispatch you ARE the step'
- effectiveInputs keyed by MAJOR: a patch successor no longer forces a re-verification (two were spent on run 11)

Refs: #479
…clared structured-output fields and a fresh-path resume lost its PR (canary run 11, 3.0.4); every next field the coordinator reads is now guarded against the schema

Refs: #479
…the cycle state — every verify next carries priorFindings (id + latest severity); a resumed run judges transitions and severity changes without re-inventing them (canary run 11, r1-5 arrived resolved)

Refs: #479
…ha256 of the card body), compared only when both sides are canonical; the skills record it from the same command (canary run 11: a free-text acHash vs a sha256 flagged 'inputs changed' on every resume and ping-ponged verify ↔ prepare into failed-resume)

Refs: #479
…xScope (sealer refuses fixScope-narrowed, verify-chain reports successor-narrows-scope); cycle-state publish --pr stamps the bound PR into every handoff

- red-snapshot.mjs: predecessorPhase / scopeNarrowing / findSnapshotByPhase; seal reads the predecessor manifest from Git history; chain breach on a forged narrowing successor
- cycle-state.mjs: publish --pr (positive int; pr-mismatch vs the draft or an earlier handoff of the run)
- skills: red-spec/red-verify state the inheritance rule; every stage passes --pr on publish
- tests: red-snapshot (narrowed refused, widened sealed + chain verified, predecessor missing, forged successor breach), cycle-state (publish --pr, mismatch, CLI, resolve reads it); 214/214
- ADR-024 amendment (b) and batch-engine reference updated
- canary run 11: a0-rev2 narrowed a0 to one file → decision log and convention page had no home (r0-4, r3-10, cg-1, cg-2); a revision handoff carried pr=null
- Task: T-8 — live canary follow-up

Refs: #479
@rucka

rucka commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Canary US-479 — run 11 on the versioned engine (3.0.0 → 3.0.8)

Story #482 · PR #483 · runId canary-479-v3 · maxParallelism 1 · policy maxFixRounds 3 / redRepairs 1 / greenRetries 1 / reviewers 1 / floor Minor
Legacy archived: archive/canary-479-legacy = c2e55274 (first review of #483 by the 2.0.0 engine preserved in run-11/legacy-first-review-483.json, marked historical in place). Branch #482 reset to the engine head with --force-with-lease after a matching read-only preflight (issue comment 5606956980).
Engine under test: pair-implement-batch.js 3.0.0 at 421441fa, patched live to 3.0.7 (seven pinned successors below); 3.0.8 = the two improvements the run surfaced, committed at 5b10f770 on #480.

Machine-readable evidence (durable, outside the local working directory): https://gist.github.com/rucka/647d7cf0a7cee450d3358a9187945706 — the 17 handoffs of canary-479-v3/482 (seq 1–15), plan.json (every run, defect, successor, observation), PR-comment snapshots before/after the unchanged re-run, the fixed-trace benchmark, the preserved legacy first review and the preflight report.

Path proven (evidence: .pair/working/runs/canary-479-v3/482/, 17 handoffs, seq 1–15)

Stage Result
a0 prepare (initial, 22 rows after one repair) rejected once (4 gaps) → repaired → validated + sealed a3772abe (pr=0)
interruption after the seal → resume prepare entry dispatch → redirect next=implement/a0 — no fresh contract, no re-seal
a0 implement attempt 1 red gate → attempt 2 GREEN a5f4da50, PR #483 reopened and updated
r0 first review upserted in place (comment 5599899909): r0-1 blocking contract-gap, r0-2 minor, r0-3/r0-4 questions
r1, r2 re-reviews r1-5 resolved; r2 added r2-6 (blocking contract-gap), r2-7, r2-8
a0-rev2 prepare (revision, 27→29 rows, ids stable) rejected once (2 gaps) → repaired → validated + successor seal 2c5bba41 on a5f4da50
a0-rev2 implement GREEN 9735d2da, PR head updated, 2 minor contractGaps reported (not patched around)
r3 re-review custody verify-chain verified (a0 → a0-rev2), gate 22/22, 119 sealed rows pass; r0-1, r2-6 resolved; new blocking r3-9 (regression introduced by the rev2 delta, alias-directory class uncovered)
budget round 3 of 3 spent → escalate, escalation comment 5614694532 upserted; first review untouched, no synthesis, no merge
unchanged re-run 1 identity dispatch → same escalate, 0 new handoffs, PR comments byte-identical

Terminal status is escalate, not ready-for-merge: a late real finding stayed blocking and the engine asked the human (extend the budget for a0-rev3, or merge carrying r3-9). No floor, budget, independence or gate was lowered.

Engine defects found by the canary and fixed as pinned successors (all on #480)

Version Defect
3.0.1 sealed manifest failed prettier → implementer used --no-verify; coordinator/script disagreement on the manifest
3.0.2 a0-rev<m> not a legal phase
3.0.3 PR#null markers when the PR was created mid-cycle
3.0.4 self-redirect loop; strict VERIFY schema dropped fields
3.0.5 pr stripped from next by the schema → verify without a PR
3.0.6 finding memory empty on resume (priorFindings missing)
3.0.7 acHash computed two ways → perpetual inputs-changed ping-pong (ac-hash command, canonical compare)
3.0.8 revision/repair must inherit fixScope (sealer fixScope-narrowed, chain successor-narrows-scope); publish --pr stamps the bound PR into the envelope

Cost (harness usage summaries; wall time includes retries and the overnight stall)

Engine Run Id Agents Subagent tokens Minutes
3.0.0 wf_84ca8007-ee4 interrupted deliberately after the a0 seal (TaskStop) 5
3.0.0 resume wf_dbd17b82-001 3 273,360 22.5
3.0.1 successor wf_2d9bcd65-654 4 280,460 22.4
3.0.2 successor wf_5b94cff3-b96 (stopped: a0-rev2 not a phase)
3.0.3 successor wf_7c54c414-a5c (network down overnight: 6×180s stall) 3 222,442 225.4
3.0.4 successor wf_547800f1-2c0 2 56,413 1.8
3.0.5 successor wf_1e595aec-fef 3 143,561 12.5
3.0.6 successor wf_81fa3204-9ea 4 115,293 2.7
3.0.7 successor wf_64be5e66-021 → escalate 8 723,206 61.1
3.0.7 unchanged re-run wf_8aabc82f-c65 → escalate (identity only) 2 50,625 1.2
total measured 34 (+ unmeasured) 1,865,360 349.6 (124.1 without the stall)

Fixed-trace dispatch counts 2.0.0 → 3.0.0 (dry run, run-11/benchmark/): cold 5→5, one fix round 13→9, interrupted resume 11→4, completed resume 3→2.

Test outcomes

Contract, coordinator, comment, boundary and loop suites: 214/214 at 3.0.8 (node --test .claude/workflows/...). Full pnpm quality-gate green standalone; the pre-push hook run failed twice on an unrelated load-dependent timeout (apps/pair-cli/src/cli.test.ts › package command is registered, 5000 ms) and passed on the third attempt with no change — same flake class as r2-7, no bypass used.

Compatibility declared

Handoff schemaVersion 2; compatibility by workflow major (3.x resumes 3.x); a 2.0.0 run directory is reported as legacyRuns, never reused; retired dispatch keys are rejected at parse time with a migration message.

Open for the human (merge gate of #483)

r3-9 (blocking regression, needs a0-rev3 or an explicit carry), questions r0-3 (seal identity pr=0 vs dispatched PR), r0-4 / r3-10 (decision-log and convention page outside the sealed scope — addressed structurally by 3.0.8 scope inheritance), r2-8 (no CI on stacked PRs); carried r0-2, r2-7.


Addendum — run 12 (human decision: extend the budget for a0-rev3) and engine 3.0.9

Decision recorded on #483 (comment 5615141891): option 1, one extra fix round. Engine 3.0.9 (commit e5211994) = the T-9 fixes (t9-1 chain across seal identities, t9-2 reviewer count honoured + partial never done, t9-3 readiness needs a 40-hex remoteHead, t9-4 approved-test-failing per group, t9-5 typed failed-resume for a contract-less next, t9-8 doc) + budget out of the effective-inputs digest + two defects run 12 surfaced (a repair/revision result is a delta; behavioral forbids new code modules only, so a decision-log entry is legal).

Stage Result
resume first launch (budget 4) redirected to a migration re-review r4 — the 3.0.9 digest formula invalidates pre-3.0.9 review digests once; stopped before any handoff and relaunched with budget 5 (round 4 = migration re-review, round 5 = the granted round)
r4 re-review (9735d2da) r3-9 still open; r0-3 resolved by the 3.0.9 chain change; no comment posted
a0-rev3 prepare (32→34 rows, R30–R34) scope inherited from a0-rev2 and widened with decision-log/ (3.0.8 rule); rejected once (half the cross-product uncovered) → repaired → validated + successor seal e0475a76
coordinator refused the repair result (delta rows cover AC-2 absent from the delta inventory) → harness retry → the retried author resolved the state and redirected to validate (self-heal); fixed in 3.0.9
a0-rev3 implement GREEN 7ab4240b; the ADL written in decision-log/ had to be reverted (4b1192f0): behavioral breached on any new file — reported as CG-3, fixed in 3.0.9
r5 re-review (4b1192f0) custody verify-chain verified (a0 → a0-rev2 → a0-rev3, ONE call); r3-9 resolved; new blocking r5-11 (Major contract-gap: the gate demands an installed twin pair update never creates for a directory symlink — sealed rows R33/R34 rest on a false premise) → round 5/5 → escalate, escalation comment updated in place (no duplicate)

Terminal status escalate again; the human chooses on #483: grant a0-rev4 (rewrite R30/R31/R33/R34 around the real installer; refuse directory symlinks under scripts/ as unsupported) or merge as is carrying r5-11 (latent: the corpus has zero symlinks).

Cost, run 12 (harness): 10 agents, 878,605 subagent tokens, 81.1 min, dispatches 10 / retries 1 / redirects 2; the stopped first launch: 3 agents, usage not summarised. Cumulative: 44+ agents, 2,743,965 measured tokens, 430.7 min (205 without the overnight stall).

T-9: independent review of 5b10f770 = CHANGES-REQUESTED (#480 (comment)); t9-1..t9-5, t9-8 fixed in 3.0.9; t9-6 (AC-10 reached escalate, not ready-for-merge; no live green-fix/pr=N seal/synthesis) and t9-7 (AC-14 has no matched 2.0.0 baseline pair) are maintainer decisions. A re-review of the 3.0.9 head follows.

Evidence gist updated: the a0-rev3 handoffs, r4/r5 reviews, the T-9 report and the plan.


Addendum — run 13 (a0-rev4, budget 6) and engine 3.0.10

Run 13 (wf_02fd2284-80f, engine 3.0.9): resume went straight to prepare/a0-rev4 (digest stable across the budget change — no migration re-review); the revision (39 → 41 rows: installer-derived expected tree, directory symlink under scripts/ refused as an unsupported layout) was rejected twice by the independent validator on real, reproduced gaps of the same class (entryIsCopyable as sole oracle; then only one of the two installer .md rewriters covered — RV-12 on R40). RED repair budget 1 spent → failed-contract, nothing sealed, head stays 4b1192f0. The delta result was accepted first time (3.0.9 fix), 0 retries. Cost: 6 agents, 578,563 tokens, 45.6 min. Status posted on #483 (comment 5617956668) with the two options for the human.

Engine 3.0.10 (commit 8488e1cc) — from the second independent T-9 review of e5211994 (#480 (comment): six prior findings resolved and reproduced; two new Minor + two Questions): the review handoff's attempt comes from the cycle state ($attempt on the verify dispatch, --attempt on publish, never the reviewer index) so the second review of a phase lands on its own filename (t9b-1); the behavioral module guard exempts documentation and decision evidence only (t9b-2); the custody chain reads first-parent history, so a foreign PR's seal merged into the branch is never a segment boundary (t9b-3); the delta rule follows the dispatched mode (t9b-4). Suites 226/226.

Cumulative (runs 11–13 + re-runs, harness usage): 50+ agents, 3,322,528 measured tokens, 476 min (251 without the overnight stall).

Open decisions: r5-11 on #483 (accept as latent vs new cycle routing it through a remediation group — which is also the live remediation round AC-10 requires); the AC-14 matched baseline pair needs a separate session on a checkout at 2.0.0 (8b8b2607): the Skill invocations of a run resolve against the session's own installed skills, so 2.0.0 and 3.0.x cannot run from one checkout.

@rucka

rucka commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

T-9 — independent review of PR #480 at 5b10f770 · CHANGES-REQUESTED

Read-only reviewer (pair-reviewer agent, fresh context, no handoff, nothing posted by it, nothing merged). Tier risk:red: general + security + boundary + architecture + adversarial passes. Suites run by the reviewer: contract/coordinator/loop node --test 240/240; workflow-mirror + skills-conformance 104/104; docs-staleness 144/144. pnpm quality-gate not run by the reviewer (green on the pre-push hook of this head).

AC status: met AC-01, 02, 03, 04, 05, 07, 11, 12, 13, 15, 16 · not met AC-06 (t9-2), AC-08 (t9-4), AC-09 (t9-2, t9-3), AC-10 (t9-6), AC-14 (t9-7).

Blocking findings (floor Minor)

id sev location failure case recommendation
t9-1 Major review-phase/scripts/red-snapshot.mjs:446 verifyChain The chain is listed by ONE pr identity while the design splits identities (initial chain pr=0, remediation group pr=N). Reproduced: base → seal a0 pr=0 → GREEN → seal r1-g1 pr=483 → GREEN: verifyChain({pr:483}) verified, but verifyChain({pr:0}) (which review-phase Step 3 also runs) reports out-of-scope + unlisted-test-changed inside a0's segment for commits legitimately made under the r1-g1 seal → terminal failed-custody on the first remediation round of any fresh story. Invisible to the suite and to canary run 11 (zero green-fix handoffs). List snapshots by trailer regardless of pr so a later seal ends the previous segment and supplies the scope in force. Test: pr=0 a0 → GREEN → pr=N r1-g1 → GREEN verifies; one dirtied sealed blob → exactly one test-blob-changed.
t9-2 Major cycle-state.mjs:268 deriveNext review branch policy.reviewers is validated, digested and rendered into the prompt but never read by the transition authority: nothing emits next.reviewer=2. Reproduced with pipeline:{reviewers:2}: ONE verify dispatch returning ready:true, partial:true, reviewer:1 (the non-final shape review-phase Step 5.5 mandates, which must not publish) → batch ready-for-merge. Contradicts AC-06 and ADR-024 (b). deriveNext emits {step:'verify', reviewer:n+1} while reviewers(policy) > review-phase handoffs at this phase, unions finding sets before judging; coordinator refuses done on partial:true.
t9-3 Major pair-implement-batch.js:1592 readiness guard Guard is conditional on the field: remoteHead && remoteHead !== reviewedHead; remoteHead optional in VERIFY_SCHEMA; cycle-state:274 degrades the same way without --head. Reproduced: readiness {ready:true} with no remoteHead → ready-for-merge. A field-dropping structured output (the 3.0.5 failure class) converts unproven readiness into approval; pair-loop cannot catch it. Require SHA40 remoteHead whenever ready===true (hasReviewEvidence); cycle-state without --head returns the re-review branch, never done.
t9-4 Minor cycle-state.mjs:285 Same-seal GREEN return gated on new Set(atf.map(groupId)).size===1. Two groups each with an approved-test-failing finding → falls through to prepare r<n+1>-g1 (fresh inventory/contract/seal for two already-sealed obligations, one budget round consumed). Reproduced with deriveNext. Route per group: GREEN for the first group (dependency order) with retries left; fall through only when a group's greenRetries are exhausted.
t9-5 Minor pair-implement-batch.js:1445 usableNext validates step/phase/base but not contract; implement/green/validate dereference n.contract.snapshot/.path. Reachable: a red-verify published without --predecessor makes contractOf() undefined → resolve returns implement/a0 without contract → TypeError in driveStory → card lands in died ("agents stalled") instead of a typed failed-resume. Guard validate/implement/green: missing contract.path/snapshotfailed-resume naming the field; died stays empty.
t9-6 Major #480 canary summary AC-10 asks for ready-for-merge with one first review AND one synthesis. Run 11 ended escalate (published.synthesis:false); never executed green-fix or a pr=N remediation seal (17 handoffs: red-spec/red-verify/implement-phase/review-phase only); ran at risk:green (general pass only); stopped at 3.0.7 — 3.0.8 has no live run. The escalate itself is correct behaviour on a real late finding. Drive one canary at the delivered version to a genuine ready-for-merge including a remediation group, or record an explicit maintainer decision accepting AC-10 partially with the named gaps.
t9-7 Major #480 canary summary AC-14 asks for lower tokens and lower median time on COMPARABLE COMPLETED runs (TC-16: matched baseline/new pair for cold, one-fix, interrupted resume). Evidence: dry-run dispatch counts + new-engine-only usage (34 agents / 1,865,360 tokens / 349.6 min); no 2.0.0 run of the same canary; the one new-engine run did not complete; elapsedMs absent from handoffs. Dispatch reduction demonstrated; token/time claim unproven. Run the matched pair (same canary, models, floor, budgets; 2.0.0 vs delivered) for cold and one-fix, report per-run tokens/time incl. retries — or record a maintainer decision accepting AC-14 on dispatch-count evidence.
t9-8 Minor batch-engine.mdx:186 Doc says only token counters are unknown; the engine returns wallMs:'unknown' too (no clock in the sandbox, commit 421441f). Name both counters as unavailable; read both from the harness run summary.

Reviewer's summary: engine genuinely strong (exact golden traces, real integration suite against installer/worktrees/GIT_DIR decoy, byte-identical mirrors, fail-closed finding policy) — but two reproduced state-machine holes let an incomplete verification become an approval (t9-2, t9-3), one custody defect fabricates a terminal breach on the first remediation round of any fresh story (t9-1), plus t9-4/t9-5/t9-8. t9-6/t9-7 need the maintainer, not code. Merge stays with the human.

Disposition (author): t9-1..t9-5, t9-8 are accepted as engine defects and will be fixed test-first as pinned successor 3.0.9 on this PR before any further canary; t9-6/t9-7 are carried to the maintainer's decision at the merge gate (external: evidence/AC scope). T-9 stays unticked until an independent re-review of the fixed head.

… identity; the reviewer count is honoured by the cycle state and a partial review never completes; readiness needs a 40-hex remoteHead; approved tests failing return to GREEN group by group; a contract-less next is a typed failed-resume; the budget leaves the inputs digest; a repair/revision result is a delta; behavioral forbids new code modules only

- red-snapshot.mjs: listSnapshots lists every pr identity (t9-1); a blob sealed earlier stays sealed through later segments; isModulePath — a decision-log entry or doc inside an allowed path is legal under behavioral (canary run 12b, CG-3)
- cycle-state.mjs: reviewer k+1 of the same phase until policy.reviewers non-partial reviews of one head exist (t9-2); done only with readiness.remoteHead == reviewedHead (t9-3); greenRetryFor per group + retries continue after the round's review (t9-4)
- coordinator: nextDefect typed refusals incl. contract.path/snapshot (t9-5); partial never done, remoteHead required when ready (t9-2/t9-3); maxFixRounds out of effectiveInputs; delta results accepted by hasPreparedContract (run 12b retry); WORKFLOW_VERSION 3.0.9
- skills: review-phase (one verify-chain per cycle, remoteHead always present, reviewer/partial semantics), red-spec (delta result note)
- tests: red-snapshot (+2 chain, module rule), cycle-state (+3), coordinator (+5); 223/223; ADR-024 and batch-engine reference (t9-8) updated
- Task: T-9 — independent review closure

Refs: #479
@rucka

rucka commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

T-9 — independent re-review of PR #480 at e5211994 (engine 3.0.9) · CHANGES-REQUESTED

Read-only reviewer, fresh context, nothing posted by it, nothing merged. Suites run by the reviewer: node --test contract/coordinator/loop 249/249; pnpm skills:conformance PASS (50 skills); dataset vs installed copies byte-identical (10 scripts, 12 workflow files); pnpm quality-gate not run by the reviewer (green on the pre-push hook of this head).

Prior findings: t9-1, t9-2, t9-3, t9-4, t9-5, t9-8 — all resolved, each reproduced independently (chain verified under both seal identities and exactly one test-blob-changed on a tampered blob; reviewer 2 really dispatched and a partial review never done; readiness without a 40-hex remoteHead refused by both authorities; two approved-test-failing groups return to GREEN group by group; a contract-less next is a typed failed-resume; metrics doc names both unavailable counters). t9-6 / t9-7: maintainer decisions recorded on #479 (comment 5617340826), not re-examined.

AC changes: AC-08 met; AC-09 met for readiness (reviewer-count half tracks AC-06); AC-06 partially met (state machine right, skill protocol not yet executable end to end → t9b-1); AC-07 at risk (t9b-2); AC-12 re-verified met.

Blocking (floor Minor)

id sev location failure case recommendation
t9b-1 Minor review-phase/SKILL.md:94 with cycle-state.mjs:286/301, pair-implement-batch.js:1478 The second review of a phase cannot publish: (a) reviewers=2 + GREEN retry → cycle state asks verify r1 attempt 3/4 reviewer 2, Step 5.5 orders attempt: $reviewer = 2 → publish refuses stale-write on r1-review-phase.attempt-2.json (occupied by reviewer 2 of the first pass) → handoff lost → coordinator seen key → failed-resume. (b) reviewers=1 + GREEN retry: the verify dispatch passes no $attempt, Step 6 omits attempt, publish falls back to 1 → stale-write on r1-review-phase.json. This is the normal outcome of the t9-4 routing. $attempt=${n.attempt ?? 1} in the verify dispatch; $attempt in the Arguments table; attempt in the Step 6 envelope and --attempt $attempt; drop attempt: $reviewer from Step 5.5. Test: r1 review → approved-test-failing → green retry → re-review with reviewers 1 and 2, every publish published:true with distinct filenames.
t9b-2 Minor red-snapshot.mjs:58-63 isModulePath The behavioral guard is bypassed by extension far beyond the motivation: a behavioral GREEN adding .github/workflows/deploy.yml, infra/main.tf, src/config.json inside allowedPaths verifies clean (reproduced); Dockerfile, db/migrations/001.sql, .env.production, web/App.vue, .pair/scripts/tool.mjs, docs/build.js all classified non-module. Only the .md cases are pinned by a test. Invert to an explicit non-module allow-list: .md/.mdx/.txt anywhere, or under .pair/adoption/, .pair/knowledge/, docs/; everything else added/moved under behavioral stays a breach. Tests for each artifact above.

Questions (not blocking)

  • t9b-3 listSnapshots no longer filters by identity: a foreign PR's seal merged into the branch history becomes a segment boundary (reproduced with git merge --no-ff of a side branch carrying pr=777/a0 → fabricated successor-not-above-predecessor). Pre-existing failure class (5b10f77 also fails custody on that history), no in-workflow trigger; findSnapshot seal idempotency still identity-filtered. Recommendation: --first-parent or restrict to identities the cycle owns (0 and $pr).
  • t9b-4 hasPreparedContract reads the agent's self-reported mode for the delta rule, not the dispatched one; the strong invariant (every declared obligation covered) is unchanged and red-verify judges the file, so no accepting case was constructed. Recommendation: pass next.mode, refuse r.mode !== next.mode.

Disposition (author): t9b-1, t9b-2 accepted as defects, t9b-3, t9b-4 accepted as hardening — all four fixed test-first as pinned successor 3.0.10 on this PR; a third independent review follows. T-9 stays unticked.

…mpt is the cycle state's (never the reviewer index); the behavioral module guard exempts documentation and decision evidence only; the custody chain reads first-parent history; the delta rule follows the dispatched mode

- coordinator: $attempt on every verify dispatch (t9b-1); hasPreparedContract takes the dispatched mode and refuses a result claiming another (t9b-4); WORKFLOW_VERSION 3.0.10
- review-phase SKILL: $attempt argument, attempt/reviewer/partial in the envelope, publish --attempt $attempt; Step 5.5 no longer derives attempt from $reviewer
- red-snapshot.mjs: isModulePath allow-list (.md/.mdx/.txt, .pair/adoption/, .pair/knowledge/, docs/) — a new CI workflow, Terraform, migration, Dockerfile, JSON config or .pair/ script stays a breach (t9b-2); listSnapshots --first-parent (t9b-3)
- tests: coordinator +2 (attempt on re-review, dispatched mode), red-snapshot +1 (foreign seal merged) and the module allow-list matrix; 226/226; ADR-024 note
- Task: T-9 — independent review closure

Refs: #479
@rucka

rucka commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

T-9 — independent third review of PR #480 at 8488e1cc (engine 3.0.10) · APPROVED

Read-only reviewer, fresh context; nothing posted by it, nothing merged, no git state touched. Suites run by the reviewer: node --test contract/coordinator/loop 252/252; pnpm skills:conformance PASS (50 skills); dataset vs installed: 22 scripts/coordinator/test files byte-identical, SKILL.md pairs differ only by the installer's name transform. pnpm quality-gate not run by the reviewer (green on the pre-push hook of this head).

Prior findings — all resolved, each reproduced: t9b-1 (attempt discipline replayed through real run directories at reviewers 1 and 2: attempts 1,2 and 1,2,3,4, distinct filenames, cycle reaches done); t9b-2 (isModulePath matrix re-probed: CI workflow, Terraform, migration, Dockerfile, JSON/YAML config, .pair/ script → module; .md, docs, ADL → exempt; no self-breach on seal manifests); t9b-3 (first-parent walk exercised for merge-of-main, fast-forward and foreign-seal shapes); t9b-4 (dispatched mode traced to both call sites, vocabulary cross-checked).

AC changes: AC-06 met (reviewer count executable end to end); AC-07 met (behavioral guard holds for the whole artifact space).

Questions (not blocking, fail-closed)

  • t9c-1 --first-parent hides a seal of THIS cycle placed behind a --no-ff merge of a side branch (fabricated out-of-scope/unlisted-test-changed, terminal failed-custody); no in-workflow trigger (seals and GREENs land on the story branch). Optional: full-history fallback for identities the cycle owns, or a typed seal-off-first-parent.
  • t9c-2 executables under the exempt directories (docs/setup.sh, .pair/knowledge/tool.mjs) are still non-modules — residual of the prescribed allow-list; needs a group whose allowedPaths cover those dirs.
  • t9c-3 extension-less / lightweight-markup docs (README, LICENSE, README.rst) are classified as modules — a behavioral GREEN adding one would be refused (fail-closed); no live instance (existing files change as M). Recommendation: exempt README|LICENSE|CHANGELOG|CONTRIBUTING|NOTICE|AUTHORS and .rst/.adoc.

Disposition (author): verdict recorded as the T-9 closure of the engine code at 8488e1cc; t9c-1..t9c-3 carried to the card as improvements. T-9 is ticked only when AC-10 (live remediation round — canary v4 in progress) and AC-14 (matched baseline pair) carry their evidence, since T-9 depends on every new AC.

…the PR's earlier findings as resolved/superseded history — non-blocking, with read-back evidence — instead of being refused as invented closures

- coordinator: findingErrors takes { history } (resuming && mode first); a carried resolved/superseded finding needs blocking=false and evidence; a fresh-path cycle and an evidence-less closure are still failed-verify; WORKFLOW_VERSION 3.0.11
- test: PR-entry first review carrying r3-9 resolved + r5-11 open continues into prepare r1-g1 (canary v4 run 14 failed-verify reproduced first)
- ADR-024: history rule + the publish-before-judgment gap recorded
- Task: T-8 — live canary follow-up (AC-10 remediation round)

Refs: #479
… an agent can no longer spell it, and resolve compares only script-stamped hashes

- cycle-state.mjs: cardHash({story}) via gh (PAIR_GH_BIN overridable, GIT_* scrubbed); publish replaces any acHash the agent wrote and marks acHashSource: publish; a gh failure keeps the value aside as acHashUnverified, never comparable; resolve reads acThen only when acHashSource === 'publish' (pre-3.0.12 handoffs are history); the ac-hash CLI reuses cardHash
- review-phase / red-spec SKILL: the envelope's acHash is any value — the script replaces it
- tests: cardHash + stamping + gh-down + no-acHash path, and the legacy unstamped handoff resuming to prepare instead of a re-review (canary v4 run 15 reproduced first); 228/228
- ADR-024: the rule recorded
- Task: T-8 — live canary follow-up (AC-10 remediation round)

Refs: #479
…ycle (dirty worktree, moved head) is retryable once the human clears it — the same phase, the next attempt; a second identical refusal, and a refusal the cycle owns, stay terminal

- cycle-state.mjs: EXTERNAL_REFUSALS = {dirty, stale}; deriveNext returns prepare(attempt+1) once, then failed-preparation; unprovable/split-required terminal at once
- red-spec SKILL: the rule stated beside the refusal contract
- test: a dirty r1-g1 retried and the second refusal terminal, a stale a0 retried, unprovable/split-required terminal (canary v4 run 17: a cleared `dirty` left the cycle blocked forever); handoff helper takes an attempt; 229/229
- ADR-024 note
- Task: T-8 — live canary follow-up (AC-10 remediation round)

Refs: #479
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-state:not-approved PR state: changes requested risk:red Classification: high risk tier user story Work item representing a user story

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant