feat(unic-archon-dlc): the config declares the SDLC's needs and the Boxes name no tool by orioltf · Pull Request #461 · unic/unic-agents-plugins

orioltf · 2026-09-04T21:16:57Z

Closes #430 — the plugin half of it. The Consumer's two steps stay with the maintainer.

Why

A Box could not check a claim about a build, a lint, a type-check or a test run, and three faults were stacked on one another. An Archon worktree may or may not carry installed dependencies and no Box asked for them. /pr-review instructed no check anywhere. And the config mapped a command one key per intended node — build.e2e_command, qa.e2e_command — so a command had an owner rather than a home, and a node that wanted one the config did not carry fabricated it instead.

Run 2's verification reported PASSED against a genuinely green suite. Both halves of that were luck: that worktree happened to have dependencies, and the node happened to guess a command that suited the project. On a project of another stack the same node runs something that does not exist and reports a failure that says nothing about the code.

What changes

The config carries one abstract block, sdlc_needs, with nine nullable keys — install, build, test, e2e, lint, format, typecheck, dev, coverage. Every Box reaches the whole of it and no Box names a tool. A key names a need; whichever runner a project uses is the tool that serves it.

Three states everywhere a command runs. pass, fail or unresolved, and unresolved is never a pass. A need the project does not declare is unresolved, not a skip — skip leaves the result enum of /qa's e2e and coverage-gate.
test is the floor, at the two gates that advance work. /qa gains a test node of its own — it ran no test suite at all before — and its merge when: reads == 'pass' for test while the rest keep != 'fail'. /build's evidence withholds on an unresolved suite, because an unresolved suite is not an absence of failures.
Each Box installs once, at bootstrap, and says whether it did. /pr-review included, with no exception: a review sub-agent that decides on its own that it needs to run something should fail on that decision, not on which worktree the run drew.
A check that could not run gets its own durable block. report.md in /build, the posted summary in /pr-review, and a new qa-checks.md in /qa, for which uat-prep gains the Write tool. /pr-review's block also states which needs the project declares and that this Box executed none of them.
/setup proposes each value from the stack it detected, by widening the sweep it already runs rather than adding a third, and a human confirms it one need at a time.

ADR-0037 records the decision. ADR-0025 and ADR-0034 are revised inline; ADR-0025's retired 2026-08-18 amendment blockquote is folded into its body and deleted, per docs/adr/README.md.

It was run in a Consumer, and the new block caught the original defect by itself

unic-dlc-qa run ac71977e, in DXP-DesignSystem, on this branch's Box (byte-identical by shasum), against a config carrying no sdlc_needs block at all — grep -c returned 0. Paused at uat-gate, harvested, abandoned. Nothing was merged, nothing was written to the client's board, and the clone came back byte-identical to how it was found.

bootstrap emitted the block as one object with all nine keys null and reported the install verbatim: "sdlc_needs.install is not declared, so nothing was installed. Dependencies may still be present in this worktree by chance, and a later green check here is not proof of a deliberately built environment." It also recorded that it read the retired qa.e2e_command / build.e2e_command keys and ignored them.

test, e2e and coverage-gate each returned unresolved and fabricated nothing — the e2e node names the project's real suite in prose and does not run it: "Nothing was substituted or inferred."

Then uat-prep wrote qa-checks.md, and its block caught something nobody asked it for:

sdlc_needs.test is not declared, so /qa ran no test command. report.md § 3 records pnpm test PASSED, but that was /build invoking the repo default — it is not a check this Box made.

That is run 2's false pass — register row 41, the finding this ticket exists for — named by the new block, in the Consumer, unprompted.

What that run did not show. /pr-review's criterion-10 block is unrendered: it is read from the prompt text alone, never observed. A second Consumer run would buy exactly that one sentence and would cost a Box whose terminal act writes a review to the client's board. The first real /pr-review run on 0.28.0 is where it gets observed.

Reviewed on two axes, and the four reads ran twice

/code-review found 11, all fixed in c525a91 (summary). Both axes independently found the same five surfaces still describing /qa without the new test node — my own four-reads pass had swept for the retired key names and not for the node chain that gained a member.

Copilot ran seven rounds. A local two-axis /code-review ran twice, the second time with a third
question added — is each file consistent with itself? — which is what the two axes never asked and what
most of the churn turned out to be. Then a five-agent pass (standards, a shallow bug scan, the git
history of every rewritten line, the review comments on earlier PRs touching these files, and the
comments inside the files themselves), and finally one integrity pass and two intent passes.

The honest headline is not the count. Four separate findings were defects introduced by the fix for
the round before, and three of those four were the same mistake: a correction landed beside the
text it should have replaced, leaving two statements that disagree or one sentence carrying half of
each. It happened twice in verification's own paragraph, and the second time was after that failure
mode had been written up as #462 proposal 3.
Every fix from that point rewrites the paragraph whole rather than patching it.

The findings that would have changed what a run does, rather than what a reader understands:

A failing e2e could have been certified green. Splitting verification into three commands left
its passed rule naming only "the suite" and the coverage threshold, never e2e.
A partial sdlc_needs block aborted the run before a guard could cancel it cleanly, because each
bootstrap said to pass the block through "exactly as you read it" while the schema requires nine keys.
That is the normal state between two releases, not a corner case.
report referenced two fields verification is told to omit. Both fields are deleted; everything
they carried lives in fields that always exist.
/pr-review told itself it executes none of the sdlc_needs commands, nine lines above being told
to run sdlc_needs.install. An agent honouring the first sentence skips the install.
evidence printed JSON and prose to one stream, so its verdict never parsed. stdout carries the
JSON alone now.

Eight surfaces described /qa's pipeline without the node this PR adds to it — two README tables,
the plugin AGENTS.md doctrine, ADR-0025's chain, two sentences and the frontmatter of the Box doc, and
the workflow's own description: field. Each was found by a different pass, and no pass found more than
three. That number is the most useful thing this branch produced and it is recorded on #462.

Two findings were left out on purpose and are #463:
/qa writes qa-checks.md and nothing commits it, so criterion 10's letter is met and its purpose is
not; and /qa declares no evidence_policy, so an AFK run with an unresolved test finishes green
having merged nothing. Neither is this PR's doing and both are decisions rather than corrections.

Three things worth a reviewer's attention

evidence prints a verdict nothing reads yet, by design. The when: that will consume it belongs to bug(unic-archon-dlc): open-pr precedes its gate, and a config-value gate has no safe middle #439, which edits this node after this lands. bug(unic-archon-dlc): the config maps commands to nodes and the Boxes name tools, so a check runs by luck or not at all #430's body says so; it is a hand-off, not an oversight.
The nine keys are written by hand everywhere that must enumerate them — each Box's output schema and its bootstrap prompt, the /setup step, the README table, the CONTEXT.md entry — and nothing checks those copies agree. Structural, not careless, and named in ADR-0037's consequences. A generator would fix it and would be the module this plugin's no-code rule refuses. The list is left to count itself on purpose: an earlier draft of that consequence said "six places", and 5c7ccc3 made it more.
In /build the install now runs ahead of slopcheck. bootstrap precedes it in the graph. The exposure is narrow — at bootstrap the tree is the branch as a human left it, and every package a slice adds arrives in run-build, downstream of slopcheck — and narrowing it further means a per-Box install exception, which the decision refuses. Named in ADR-0037 rather than left to be discovered. The trade is the maintainer's to confirm; if he rules otherwise it is criterion 6 that changes, not this diff.

What is deliberately not here

Criterion 11 is a scope statement. /pr-review gains no procedure that runs a check against an author's claim; that belongs to a Method and its home is feat(unic-archon-dlc): the review angles become a Method of ours, harvested from unic-pr-review and rewritten stack-agnostic #433. What it gains is the sentence admitting it.
Criteria 13 (second half) and 14 are the maintainer's, inside the Consumer. A comment on bug(unic-archon-dlc): the config maps commands to nodes and the Boxes name tools, so a check runs by luck or not at all #430 lists those steps.
The per-slice test_command written by /tickets and read by RED and GREEN is untouched. Only bootstrap's project-wide inference became sdlc_needs.test. CONTEXT.md now tells the two apart.
ADR-0026 is unchanged. Nothing in it went false: its node graph gains no node and its config block is untouched. The /pr-review install and summary block are recorded in ADR-0037.
ADR-0009's "AI development lifecycle" stays. Criterion 19 names four sites and that is not one; the gloss remains a legitimate name in a historical record.

Verification

pnpm ci:check and pnpm test green by exit code; all nine CI checks green by exit code. All four Box YAMLs parse and every $node.output reference resolves to a real node. grep pnpm over the three Box files returns nothing — slopcheck holds npm, never pnpm. No nested $node.output reference exists in any Box. Version 0.28.0, ### Breaking entry present, verify:changelog green.

None of that ran a node. The Consumer run above is the check that did.

🤖 Generated with Claude Code

…oxes name no tool A Box could not check a claim about a build, a lint, a type-check or a test run. Three faults stacked: an Archon worktree may or may not carry dependencies and no Box asked for them; `/pr-review` instructed no check anywhere; and the config mapped a command one key per intended node, so a node that wanted one the config did not carry fabricated it. One run reported a green suite by luck on both counts. The config now carries one abstract `sdlc_needs` block with nine nullable keys, every Box reaches the whole of it, and no Box names a tool. Every node that runs a command reports `pass`, `fail` or `unresolved`, and `unresolved` is never a pass. `test` is the floor at the two gates that advance work: `/qa` gains a `test` node of its own and its merge `when:` requires a pass, while `/build`'s evidence withholds on an unresolved suite. Each Box installs once at `bootstrap` and says whether it did. A check that could not run reaches the reader as its own durable block, so `/qa`'s `uat-prep` gains `Write` and files `qa-checks.md`. Closes #430 — the plugin half. The Consumer's own two steps are the maintainer's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Three things the criteria did not name and a later reader would re-derive: the per-slice test_command a reader must tell sdlc_needs.test apart from, the six hand-written copies of the nine-key list, and the install now running ahead of slopcheck in /build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Standards axis found five surfaces describing /qa's pipeline without the new `test` node — the plugin AGENTS.md doctrine, both README tables, ADR-0025's node chain and two sentences in the qa Box doc — and two prompts that could not do what they were told. Spec axis found the README threshold row still claiming a null threshold skips the check, two output fields with no reader, and a printed verdict nothing could parse. The two that were defects rather than staleness: `verification` never said whether an unresolved test is a blocker. Listed in `failures`, it would have made `evidence`'s `unresolved` branch unreachable and collapsed three states into two, while the node's own comment asserted that branch was live. An unresolved need now goes in `unresolved_needs` and never in `failures`. `evidence` printed its JSON verdict and then printed prose to the same stream, so stdout was never parseable — not even on the pass path, where two progress lines follow it. A `script:` node's stdout IS its output and its reader is a `when:` with nowhere to put a parse failure, so stdout now carries the JSON alone and every human line goes to stderr. `uat-prep` asked for the reason a need went unresolved. That reason lives in the node summaries #458 covers and this ticket must not give a reader, so the block names the need and reads the common cause off the `sdlc_needs` block it already holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

orioltf · 2026-09-04T21:25:30Z

Two-axis review — 11 findings, all closed in `c525a91`

/code-review against origin/develop, spec axis reading #430's body and its implementation opener. The axes ran as separate agents and did not see each other's output.

Axis	Findings	Worst
Standards	6 (4 hard, 2 agent-readability)	`verification` never said whether an unresolved test is a blocker
Spec	5 (2 partial, 1 scope, 2 wrong)	`evidence` printed a verdict nothing could parse

Every finding was checked against the files before being acted on; two needed correcting, below.

What both axes found independently

Five surfaces still described /qa without the test node. The plugin AGENTS.md doctrine, both README tables, ADR-0025's node chain, and two sentences in docs/boxes/unic-dlc-qa.md. Standards called it four hard violations of the "load-bearing invariants" and "written once" rules; Spec called it criterion 9's floor going undocumented. Same five lines, two readers, no contact between them — that is the finding I would trust most, and it is the one my own four-reads pass missed, because I swept for the retired key names and not for the node chain that gained a member.

The two that were defects, not staleness

verification could have made a third state unreachable. The prompt said "set passed false whenever test_outcome is not pass" and "list blockers ONLY in failures", and never said which of those an unresolved test is. Listed in failures, evidence's unresolved branch never fires and the three states collapse to two — while the comment I wrote directly above that branch asserts it is live. An unresolved need now goes in unresolved_needs and never in failures, and the prompt says why.

evidence's printed verdict was not parseable. A script: node's stdout is its whole output, and this node printed JSON and then printed prose to the same stream. stdout now carries the JSON alone; every human line goes to stderr, which the run log still shows.

Fixed, with the reasoning that constrained the fix

uat-prep asked for a "why" it cannot see. The block wanted each unresolved need "and why", but only result is substituted; the reason lives in e2e.summary and coverage-gate.summary, which are the fields #458 covers and this ticket must not give a reader. So the block names the need and reads the common cause — no command declared — off the sdlc_needs block uat-prep already holds.
e2e_outcome and coverage_outcome had no reader. Criterion 8 excuses exactly one readerless field, evidence's verdict, and says a second is a defect. report § 4 now reads both, rather than the fields being deleted: criterion 7 asks every node that runs a command to report a state, and an absent field means the node wanted no such check — which is not the same fact as a check that could not run.
README.md's build.coverage_threshold row still said a null threshold "skips the check". Nothing skips any more.
ADR-0037 §5 justified the e2e asymmetry and not the coverage one. /qa's coverage-gate reports unresolved on a null threshold and /build's verification omits the field. That is the same wanting-versus-declaring rule landing differently in two nodes, and it is now written down as such — it is the asymmetry most likely to be read as an inconsistency.

Two corrections to the review

Spec said the evidence stream "is clean JSON only when the verdict is pass". It was never clean: two progress lines follow the JSON on the pass path as well. The finding stands and its reasoning understated it.
Spec flagged that ADR 0037 is hard-linked from four sites and an in-flight ticket may claim that number first. True, and already accepted — bug(unic-archon-dlc): the config maps commands to nodes and the Boxes name tools, so a check runs by luck or not at all #430's opener names it as a known cost of referencing the ADR by slug until it merges. Not a defect.

Still outstanding

The Consumer run. pnpm ci:check, pnpm test and all nine CI checks are green by exit code, and none of them ran a node — every command defect of this plugin's 0.22.0 was invisible here and visible on the first Consumer run. What that run can and cannot prove is a ruling in its own right and is recorded on #430.

Copilot

🟡 Changes recommended

Several workflow prompts have schema/forward-compatibility and required-field emission risks that could cause early workflow aborts instead of clean “unresolved” reporting.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the unic-archon-dlc plugin to make the tenant config declare the SDLC’s runnable needs via a single sdlc_needs block (nine nullable keys), and removes Box-level tool naming/fallbacks so checks don’t succeed (or fail) “by luck” depending on the worktree and stack.

Changes:

Introduces sdlc_needs (install/build/test/e2e/lint/format/typecheck/dev/coverage) and threads it through /build, /qa, and /pr-review bootstrap schemas and prompts.
Updates /qa to add a dedicated test node and changes command-running nodes to report pass | fail | unresolved (removing skip), with test as the merge “floor”.
Revises docs/ADRs/CHANGELOG and bumps the plugin version to 0.28.0 to document and release the breaking config surface changes.

File summaries

File	Description
apps/claude-code/unic-archon-dlc/README.md	Updates plugin description and config reference to document `sdlc_needs` and `/qa`’s added `test` step.
apps/claude-code/unic-archon-dlc/package.json	Bumps package version to `0.28.0`.
apps/claude-code/unic-archon-dlc/docs/boxes/unic-dlc-qa.md	Updates QA box docs for `test → e2e → coverage` and durable `qa-checks.md`.
apps/claude-code/unic-archon-dlc/docs/boxes/unic-dlc-pr-review.md	Documents `/pr-review` bootstrap reading `sdlc_needs` and running `sdlc_needs.install`.
apps/claude-code/unic-archon-dlc/docs/boxes/unic-dlc-build.md	Documents `/build` bootstrap install, three-state needs outcomes, and evidence/report semantics.
apps/claude-code/unic-archon-dlc/docs/adr/README.md	Updates ADR index rows/statuses and records ADR-0037 in the index.
apps/claude-code/unic-archon-dlc/docs/adr/0037-config-declares-sdlc-needs.md	Adds the new ADR capturing the `sdlc_needs` decision and consequences.
apps/claude-code/unic-archon-dlc/docs/adr/0034-evidence-gate-deterministic-writer.md	Revises evidence-gate ADR to account for `test_outcome`/unresolved semantics and stdout JSON.
apps/claude-code/unic-archon-dlc/docs/adr/0025-qa-pipeline-onramp.md	Revises `/qa` ADR for `test` node and `pass
apps/claude-code/unic-archon-dlc/CONTEXT.md	Adds SDLC vocabulary entry and documents `sdlc_needs` vs per-slice `test_command`.
apps/claude-code/unic-archon-dlc/commands/setup.md	Updates `/setup` instructions to harvest/propose `SDLC_NEEDS` values and write the new config block.
apps/claude-code/unic-archon-dlc/CHANGELOG.md	Adds `0.28.0` breaking/added/fixed entries describing the new config surface and behavior.
apps/claude-code/unic-archon-dlc/AGENTS.md	Updates plugin description/invariants to reflect SDLC terminology and new evidence/QA gating details.
apps/claude-code/unic-archon-dlc/.claude-plugin/plugin.json	Bumps plugin manifest version to `0.28.0`.
apps/claude-code/unic-archon-dlc/.claude-plugin/marketplace.json	Bumps marketplace metadata version to `0.28.0`.
apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-qa.yaml	Implements `/qa` `test` node, `sdlc_needs` bootstrap schema, and durable `qa-checks.md` writing.
apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-pr-review.yaml	Threads `sdlc_needs` through bootstrap and adds a visible “checks not run” block to the posted summary.
apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-build.yaml	Implements `sdlc_needs`-based verification, evidence JSON stdout, unresolved reporting, and report restructuring.

Review details

Suppressed comments (1)

apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-build.yaml:482

unresolved_needs is required by this node’s output_format, but the prompt doesn’t explicitly tell the agent to always emit it (and to use an empty array when nothing is unresolved). Since missing a required output field aborts the workflow, it’s worth making this instruction explicit here, especially because unresolved_needs is new and feeds evidence/report.

         and a `failures` array of short strings, one per distinct failure. List blockers ONLY in
         `failures` — never a non-blocking note. Set `passed` false whenever `test_outcome` is not
         "pass": an unresolved suite is not an absence of failures. **A need that came back
         "unresolved" goes in `unresolved_needs` and NEVER in `failures`**, however blocking it feels.
         `failures` means something ran and was red; an empty `failures` beside a non-"pass"

Files reviewed: 18/18 changed files
Comments generated: 4
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…ead of reporting Copilot round 1, four findings, all real. Every bootstrap said to pass the block on "exactly as you read it, adding and removing no key", while the node schema requires all nine. A tenant config carrying a partial block — this file is hand-edited, and a key added by a later release reaches a tenant only at their next reconfigure — would have produced a short object, failed schema validation and aborted the run before a guard could cancel it cleanly. The three prompts now normalise: copy every value the config sets verbatim, emit null for every key it omits, always nine keys. `unresolved_needs` had the same shape one layer down: required by the schema, never explicitly always-emitted. It is now emitted as an empty array when nothing was unresolved. The README threshold row read as `/build` behaviour when the unresolved is `/qa`'s. ADR-0037's consequence about hand-written copies said "six places" and this commit made it more, so it now names the surfaces and lets the list count itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

/build’s verification prompt currently leaves passed/failures ambiguous for optional executed checks (e2e/coverage), which could allow the evidence gate to certify a non-green run.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 18/18 changed files
Comments generated: 1
Review effort level: Lite

…reen Copilot round 2, one finding, and it is a regression this PR introduced. Before this branch, `verification` ran ONE command — `e2e ?? test ?? pnpm test` — so "true only if the suite is green" named the only fact there was. Splitting the node into a test run, an optional e2e run and an optional coverage run left that sentence behind: it names the suite and the coverage threshold and never names e2e. An implementation following it literally could report a failing e2e with `passed: true` and an empty `failures`, and `evidence` reads exactly those two fields, so a red build would have been certified. `passed` is now true only when `test_outcome` is "pass", no stubs were found, and every optional outcome actually reported is "pass" too. A "fail" on any of them is a blocking entry in `failures`. An "unresolved" is not: it goes to `unresolved_needs` alone, because outside the `test` floor no need is mandatory and an unresolved e2e must not hold the evidence gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

There’s a conflicting instruction in /build’s verification prompt about whether non-blocking unresolved optional outcomes should affect passed, which can cause incorrect evidence withholding behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 18/18 changed files
Comments generated: 1
Review effort level: Lite

…e times Copilot round 3, and the contradiction was mine, made while fixing round 2. That fix bolted a new `passed` rule onto a paragraph that already carried one, so the prompt ended up stating the rule three times: an ALL-of-these version, a "set passed false whenever test_outcome is not pass" version, and the fail/unresolved clauses. Two of them disagreed about an unresolved optional outcome — one made it lower `passed`, the other did not — which would have had `evidence` withhold on a check the project never declared. The rule is now stated once, as the three conditions that make `passed` false, with an explicit line that an unresolved optional outcome leaves it alone. Reading the prompt whole then caught the round-2 omission one step up: step 4's verdict gloss still read "tests green, no stubs, coverage ≥ threshold" and never named e2e. It now names the same conditions as the rule, by the same names, and asks for the unresolved needs in the block a human reads at the gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The documentation updates contain at least two concrete config-reference inaccuracies/omissions that should be corrected to keep the new sdlc_needs/unresolved semantics reliably operator-facing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

apps/claude-code/unic-archon-dlc/docs/boxes/unic-dlc-qa.md:89

This row implies that a null sdlc_needs.install is an “unresolved check”, but in the workflow install is not treated as a check result: it’s an optional bootstrap action reported via install_report (“nothing was installed”), while unresolved is used for command-running nodes (test/e2e/coverage-gate). Clarifying this avoids readers expecting an unresolved install state.

Files reviewed: 18/18 changed files
Comments generated: 1
Review effort level: Lite

…ually reads Copilot round 4, two findings, both real. `/qa`'s bootstrap resolves `qa.coverage_threshold ?? build.coverage_threshold`, and the config reference carried only the second. That table is not a summary: `commands/setup.md` calls it the source of every default and forbids asking the operator for a key it omits, so a key with a live reader and no row reads as a key nothing reads. This PR changed that key's semantics — null is now `unresolved` rather than a skip — which is what makes the omission this PR's to close. The qa Box doc said a null `sdlc_needs` key is an unresolved check and listed `install` among them. `install` is not a check: `bootstrap` runs it once if it is declared and reports either way through `install_report`, which has no `unresolved` state. Fixed in the row that said it. Then re-reading the rows around the new one, rather than the diff as a whole, caught two more: `build.coverage_threshold` and the new `qa.coverage_threshold` both explained `/qa`, and `sdlc_needs.coverage` named only one of the two thresholds it is compared against. Each row now carries one statement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Adding the qa.coverage_threshold row left two neighbours stale: it and build.coverage_threshold both explained /qa, and sdlc_needs.coverage named only one of the two thresholds it is compared against. Each row now carries one statement of one fact. Split from the commit before it because Prettier repads a whole Markdown table on a one-cell edit, so the replacement these three needed did not match the text on disk after the first edit was formatted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The docs/prompts introduce a couple of verified internal inconsistencies (notably around install scope and qa-checks.md gate messaging) that should be corrected before merge to avoid misleading operators and future maintainers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 18/18 changed files
Comments generated: 2
Review effort level: Lite

Copilot

🟡 Changes recommended

There are still concrete documentation/metadata issues to fix (notably broken Markdown in ADR-0037 and terminology inconsistency in plugin metadata).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 18/18 changed files
Comments generated: 2
Review effort level: Lite

…is PR falsified Copilot round 5 found two, both real, both mine: `uat-gate`'s approval message named the e2e and coverage results and not `test`, so the human standing at the one gate this Box pauses at was never told the result of the check the merge gate later refuses to advance without. It also pointed at what `uat-prep` printed rather than at the `qa-checks.md` this PR made it write. The README said `sdlc_needs.install` runs in "Every Box". It runs in three of four; `/explore` runs no check and is out of scope on #430 by name. Then a sweep for anything else the change had falsified — asked for by the maintainer, and it earned its keep: - ADR-0037's own Decision said "every Box reaches the whole of it" and its decision 6 was headed "Each Box"; both now say what is true, and the fourth Box is named with its reason rather than left to look like an omission. - ADR-0025 guaranteed that "a red e2e/coverage or a wrong PR base never auto-merges" — written before the floor existed, so the guarantee omitted the one check that is mandatory. - `/qa`'s finding-capture brief asked a rejected item to cite "the e2e/coverage evidence", and there are three checks now; it also has to say when one of them is unresolved rather than let a reader take it as passing. - ADR-0023 described `/build`'s bootstrap as emitting `{ slug, artifacts_dir, gate, test/e2e/coverage }`. That is the shape this PR replaced. ADR-0023's two legacy amendment blockquotes are deliberately NOT folded. The fold-and-delete rule in docs/adr/README.md fires when a decision is revised inline, and that ADR's decision is untouched here: what changed is a sentence describing an output shape that ADR-0037 now owns, so this is a cross-reference repair. Folding two long blocks about `$CLAUDE_PLUGIN_ROOT` and a deleted `lib/` would be a second change riding in this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

A downstream report prompt in /build references optional verification output fields that the workflow also instructs to omit, which can break runs via missing output-field references.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 19/19 changed files
Comments generated: 5
Review effort level: Lite

…omit Copilot rounds 6 and 7, seven findings, plus two the coherence sweep added. NOT pushed — the maintainer asked to hold, because every push triggers another review and every fix is a push. The one that would have broken a run: `report` referenced `$verification.output.e2e_outcome` and `$verification.output.coverage_outcome`, while `verification` was told to OMIT both when it wanted no such check. A node-output reference to a field that may not exist is the class of thing that kills an Archon run at template resolution with no useful error. Both fields are deleted rather than made mandatory, because everything they carried already lives in a field that always exists: an unresolved need is in `unresolved_needs`, a red one is in `failures`, the figures and counts are in the prose `verdict`, and `test_outcome` stays because the evidence gate reads it. Round 4 gave these two fields a reader instead of deleting them; that was the wrong half of the choice, and it created the dangling reference this fixes. The rest: - The `e2e` comment block ended up above `- id: test` when the node was inserted, so the floor node was introduced by a paragraph about the end-to-end suite. It has its own header now, and `/qa`'s bootstrap header no longer describes the retired qa.* → build.* e2e fallback. - `build.coverage_threshold`'s row said "`null` no longer skips" and then described /build skipping. It now says what null means there: the node wants no figure and reports nothing, which is silence rather than a passing check. - An inline code span split across a newline in ADR-0037 did not render. - ADR-0023's braces read as an exact field list when they are a subset. - The plugin description was the last operator-facing place still on the gloss alone. Fixed in plugin.json and in BOTH copies inside marketplace.json — fixing one would have left two stale ones disagreeing with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…a mangled instruction The two-axis review was re-run on this branch after the churn, with a third question added: is each file consistent with itself? That question is what the two axes never asked, and it returned three defects on a tree they had already passed. The worst was mine, from the commit before: `verification` step 3 read "…compare the total against the threshold, then set / and compare the total against the threshold." A replacement had not consumed the clause it replaced, leaving a dangling verb and the comparison stated twice, in a prompt an agent executes at run time. Step 4 forbade restating step 5's conditions in other words and then restated them in other words, and its gloss was wrong as well: an unresolved coverage check is reported and is not green, so the gloss read PASSED as false where the rule keeps `passed` true. It now cites the rule instead of paraphrasing it. `coverage-gate`'s comment header still promised the `skip` its enum lost. From the Spec axis, which also argued criterion 7 explicitly and found it still holds after the two outcome fields were deleted: - Criterion 7's zero-count clause had reached `test` alone. A coverage or e2e command aimed at the wrong target passed silently. All four check nodes carry it. - ADR-0037 forbade writing a count beside a list, and three prompts say "ALL NINE keys". The consequence now names the exception rather than pretending the rule is absolute: an instruction needs a number because a node checking its own output needs something to check against. - The ADR index dropped the #389 attribution from ADR-0025's row when it gained a revision date. Both are recorded now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… node this change adds A five-agent review pass, run locally against the branch with the PR back in draft so it triggered no further automated round. Four of the five came back clean — standards compliance, a shallow bug scan, the git history of every line this branch rewrote, and the review comments left on earlier PRs that touched these files. The fifth read the comments inside the modified files and found two. `unic-dlc-qa.yaml`'s own `description:` field — the text `archon workflow list` prints — still described the pipeline as `e2e → coverage-gate → UAT → merge`. It is the sixth surface of this class found on this branch, and the first that nothing else caught: not the two-axis review, not the hand sweep, not seven rounds of automated review. All six missed it for the same reason, that it is a YAML field rather than prose in a document. `/build`'s bootstrap header comment was left describing a node that has since grown the whole `sdlc_needs` block and an install step, while `/qa`'s and `/pr-review`'s equivalents were both updated. Parallel nodes, inconsistent treatment. Also checked and clean: the other three workflow descriptions carry nothing this change falsified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…t is told to run An integrity pass and an intent pass, both against this tree rather than an ancestor. Ten findings, five of them behavioural. The one that would have broken a run: `/pr-review`'s bootstrap said "This Box executes none of these commands by instruction" nine lines above "run `sdlc_needs.install` from the repo root". `install` is one of those commands. An agent honouring the first sentence skips the install every other node depends on. ADR-0037 §6 scopes that rule to *checks*, and `reconcile` already stated it correctly; the bootstrap now says the same words and names install as the exception it is. The published block had the mirror of it: it named every declared key, `install` among them, said the review executed none of them, and then printed `install_report`, which on a declared install reads "ran the declared install command: ok". It denied something the next line proved. `install` is out of that sentence. The `sdlc_needs` paragraph copied into all three Boxes said a null key "becomes an unresolved check further down". True in `/qa`, false in `/build`, where `verification` reports nothing at all about an e2e or a coverage it never wanted. The paragraph now states both halves and is true in every Box that carries it. `docs/boxes/unic-dlc-build.md` had the same error in doc form, stating the rule ADR-0037 §5 exists to refuse. Surfaces seven and eight of the `/qa` pipeline omission: the box doc's frontmatter and its opening line. Eight surfaces on this branch have described that pipeline without a node added to it, each found by a different pass. Smaller: a passing e2e's counts were required nowhere, though criterion 7 asks a `pass` to carry them; the shared sentence said `evidence` "treats a test that is not pass as red" when `evidence` keeps `unresolved` and `fail` apart in its own verdict; the CHANGELOG still said "Every Box installs" after the README and the ADR were corrected; ADR-0025 still described `bootstrap` as emitting scalars; and ADR-0037's own bullet against writing a count beside a list carried a count, which was wrong — `commands/setup.md` enumerates the nine keys twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…nded verbs The final intent pass found four. Two are mine and fixed here; two are questions for the maintainer and are not touched. **Mine, and the same mistake twice in one session.** Last commit rewrote the shared sentence about what `evidence` does and left the clause it replaced standing: "…refuses to certify on a test that is not pass whatever the booleans say — keeping unresolved and fail apart in its own verdict, **and withholds on any of those signals**, and `evidence_policy` then gates…". Two stranded verbs, in both copies. This is the second time on this branch that a correction landed beside the text it should have replaced — the first was `verification` step 3 — and it is the failure mode #462 proposal 3 was filed against, committed to, and then repeated. The sentence is now rewritten whole rather than patched, and the `evidence_policy` clause a later ticket owns is left untouched at the end of it. `verification`'s verdict block had two labels for three states. An unresolved test printed `verification: FAILED ✗ —` beside an empty list of failures, because `failures` may not carry an unresolved need. It now prints UNRESOLVED for that case, and says why the three are kept apart: the human at build-pr-gate is the reader who most needs them apart. **Not touched, because they are decisions rather than defects**, both reported to the maintainer: - `/qa` writes `qa-checks.md` into the Session dir and nothing commits it, so it dies with the worktree. Criterion 10's letter is met and its purpose is not. `/build` survives only because `open-pr` stages `report.md`, and `/qa` has no equivalent node. Giving `/qa` a commit is new behaviour, and the criterion's author specified exactly what it asks for. - `/qa` declares no `evidence_policy`. In AFK with an unresolved test, `merge`'s `when` blocks, every node succeeds, and the run reports completed having merged nothing. Pre-existing; ADR-0034 says no other Box gains an evidence gate in it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

A user-facing merge approval message in unic-dlc-qa.yaml is misleading relative to the node’s actual when: conditions and should be corrected for operational clarity.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 19/19 changed files
Comments generated: 1
Review effort level: Lite

orioltf · 2026-09-04T23:22:46Z

Closing out eight review rounds

Every inline thread on this PR is resolved except one, and the review bodies above have gone unanswered as bodies while I answered them thread by thread. This closes them as a set.

What the eight rounds produced

Round	Commit reviewed	Findings	Fixed in
1	`c525a91`	4 + 1 suppressed	`5c7ccc3`
2	`5c7ccc3`	1	`3887199`
3	`3887199`	1	`21829a1`
4	`21829a1`	1 + 1 suppressed	`efeb19b`, `842fe7e`
5	`efeb19b`	2	`c3d2b21`
6	`842fe7e`	2	`7c566b6`
7	`c3d2b21`	5	`7c566b6`, `d2331c5`
8	`0be417a`	1	open — see below

Both suppressed comments were acted on: round 1's (unresolved_needs required by the schema and never explicitly always-emitted) and round 4's (install listed among the check keys when it is not a check). Neither had a thread, and a review body is the only place either appears.

Alongside these, a two-axis /code-review ran twice, a five-agent pass read the git history and the review comments on earlier PRs touching these files, and an integrity pass and two intent passes ran against the final tree.

The findings that would have changed what a run does

Not the count — these five are why the rounds were worth the wall-clock:

A failing e2e could have been certified green. Splitting verification into three commands left its passed rule naming only the suite and the coverage threshold, never e2e.
A partial sdlc_needs block aborted the run before a guard could cancel cleanly. That is the normal state of a hand-edited config between two releases, not a corner case.
report referenced two fields verification is instructed to omit. Both fields are gone; what they carried lives in fields that always exist.
/pr-review told itself it executes none of the sdlc_needs commands, nine lines above being told to run sdlc_needs.install.
evidence printed JSON and prose to one stream, so the verdict never parsed.

What I got wrong, since the reviews were right about it

Four findings were defects introduced by the fix for the round before, and three of those four were the same mistake: a correction landed beside the text it should have replaced. It happened twice inside verification's own paragraph, the second time after that failure mode had already been written up as #462. Every fix after that point rewrites the paragraph whole.

I also left rounds 6 and 7's threads open for an hour after fixing them, and reported "zero unresolved threads" from a single check taken two minutes before round 2 landed. Both are mine, and both are in #462 as things the process should not depend on me remembering.

Still open

Round 8's finding is real and not yet pushed. merge-gate's approval message said "an unresolved test holds this gate", and that gate's when: reads only gates.qa and verify-pr-base.base_ok — the floor is enforced by the merge node downstream, not by the gate the human is standing at. The message now says which node enforces it and that approving will not produce a merge while the test result is not pass. Held locally at the maintainer's request; the thread gets its reply when it lands.

I did not take Copilot's alternative of gating merge-gate itself on $test.output.result == 'pass'. It would stop asking for an approval that cannot merge, and it would also mean an AFK-shaped silence: no approval asked, no merge, no explanation. That failure is #463, and this PR should not deepen it.

Deliberately not in this PR

#463 — /qa writes qa-checks.md and nothing commits it, so criterion 10's letter is met and its purpose is not; and /qa declares no evidence_policy, so an AFK run with an unresolved test finishes green having merged nothing. Neither is this PR's doing and both are decisions rather than corrections.

…esolved test holds Copilot round 8, and it is right. `merge-gate`'s approval message said "an unresolved test holds this gate however clean the rest reads". That gate's `when:` reads `gates.qa` and `verify-pr-base.base_ok` and nothing else — the floor is enforced one node later, by `merge`. So a human standing at that gate was told the gate would stop them when it would not: they could approve, and then nothing would merge. The message now names which node enforces the floor and says what approving will actually do while the test result is not "pass". Not taken: the alternative of gating `merge-gate` itself on `$test.output.result == 'pass'`. It would stop asking for an approval that cannot merge, and it would also remove the last human from a run that then merges nothing and explains nothing — which is the failure filed as #463. This PR should not deepen that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

orioltf and others added 5 commits September 4, 2026 23:14

docs(unic-archon-dlc): changelog entry for the sdlc_needs config block

aad763b

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

chore(unic-archon-dlc): bump to 0.28.0

66cc448

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

orioltf marked this pull request as ready for review September 4, 2026 21:34

Copilot AI lite review requested due to automatic review settings September 4, 2026 21:34

orioltf mentioned this pull request Sep 4, 2026

bug(unic-archon-dlc): the config maps commands to nodes and the Boxes name tools, so a check runs by luck or not at all #430

Open

Copilot started reviewing on behalf of orioltf September 4, 2026 21:35 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 4, 2026 21:41

Copilot started reviewing on behalf of orioltf September 4, 2026 21:42 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-build.yaml Outdated

Copilot AI review requested due to automatic review settings September 4, 2026 21:53

Copilot started reviewing on behalf of orioltf September 4, 2026 21:53 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-build.yaml Outdated

Copilot AI review requested due to automatic review settings September 4, 2026 21:59

Copilot started reviewing on behalf of orioltf September 4, 2026 21:59 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread apps/claude-code/unic-archon-dlc/README.md Outdated

Copilot AI review requested due to automatic review settings September 4, 2026 22:05

Copilot started reviewing on behalf of orioltf September 4, 2026 22:05 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-qa.yaml

Comment thread apps/claude-code/unic-archon-dlc/README.md Outdated

Copilot AI review requested due to automatic review settings September 4, 2026 22:09

Copilot started reviewing on behalf of orioltf September 4, 2026 22:09 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread apps/claude-code/unic-archon-dlc/.claude-plugin/plugin.json Outdated

Comment thread apps/claude-code/unic-archon-dlc/docs/adr/0037-config-declares-sdlc-needs.md Outdated

Copilot AI review requested due to automatic review settings September 4, 2026 22:17

Copilot started reviewing on behalf of orioltf September 4, 2026 22:18 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

orioltf mentioned this pull request Sep 4, 2026

The two-axis review does not ask the question that eleven of PR #461's fifteen defects answered to #462

Open

orioltf marked this pull request as draft September 4, 2026 22:47

orioltf and others added 4 commits September 5, 2026 00:47

orioltf mentioned this pull request Sep 4, 2026

/qa can finish green having merged nothing, and the file that would explain why dies with the worktree #463

Open

orioltf marked this pull request as ready for review September 4, 2026 23:13

Copilot AI review requested due to automatic review settings September 4, 2026 23:13

Copilot started reviewing on behalf of orioltf September 4, 2026 23:13 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-qa.yaml Outdated

Uh oh!

Conversation

orioltf commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changes

It was run in a Consumer, and the new block caught the original defect by itself

Reviewed on two axes, and the four reads ran twice

Three things worth a reviewer's attention

What is deliberately not here

Verification

Uh oh!

orioltf commented Sep 4, 2026

Two-axis review — 11 findings, all closed in c525a91

What both axes found independently

The two that were defects, not staleness

Fixed, with the reasoning that constrained the fix

Two corrections to the review

Still outstanding

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

orioltf commented Sep 4, 2026

Closing out eight review rounds

What the eight rounds produced

The findings that would have changed what a run does

What I got wrong, since the reviews were right about it

Still open

Deliberately not in this PR

Uh oh!

Reviewers

orioltf commented Sep 4, 2026 •

edited

Loading

Two-axis review — 11 findings, all closed in `c525a91`