fix(agent-plugin): read every bucket, bind the corpus directory, and ratify ADR-0038 - #208
Merged
Merged
Conversation
… work Three defects shipped in 0.3.0, each found by executing the CLI rather than reasoning about it, and each of which offers a repository a decision record it does not need. `adr check`'s `governing` bucket holds `accepted` records alone. Detection read it by itself, so a process record that was `proposed` -- which is exactly what `/adr-draft` writes -- or one that had been `rejected` came back as absence. Measured: that record returns exit 0 with `governing: []` and `activeProposals: ["0001"]`, or `history: ["0001"]` when rejected. The consequence was circular: backfill offered the bootstrap record, `/adr-draft` wrote it `proposed`, and the next run offered the same decision again. A `rejected` record was re-proposed, which decision-memory names as the third failure it exists to prevent, and it contradicted this skill's own rule to reconcile against accepted, proposed, rejected and superseded records. Detection now reads all three buckets and distinguishes already-proposed (ratify it) from settled-against (never re-propose). The exit-code guard read `adr check`'s exit code as though it certified the corpus. It is scoped to the paths it was handed, so a malformed process record beside a healthy one returns exit 0 with empty findings while corpus-wide `adr lint` exits 1. The corpus-wide gate is now `adr lint`. The offer named `/adr-draft`, whose gate stopped on `adr lint` exit 2 -- precisely what a repository with no corpus returns -- so the headline case could not be written at all. `adr new` exits 0 there and creates the corpus, because `createAdr` makes the directory itself. That gate is narrowed to the absent- corpus case; an unparseable corpus stays a hard stop. Detection guidance now lives in `commands/adr-backfill.md`, not the skill alone: `/adr-backfill` loads the command, so a procedure only the skill carried was unreachable from the entry point people invoke. The safety-policy checker enforces it against both surfaces. Per ADR-0016 the checks were observed failing first: four new assertions red against 0.3.0 guidance, and the retained contradictory fixture gained matching negative cases so the new rules have a permanent executable counter-example rather than an attestation. ADR-0038 is amended rather than superseded -- it is still `proposed`, and the decision it records is unchanged; what was wrong was the detection mechanic and the reachability of the write path. Two action items are added and left open: `ADR_DIR` resolution ignoring step-1 discovery, and mirroring the no-corpus clause into `/adr-context` and `/adr-check`. Signed-off-by: Mark Beacom <m@beacom.dev>
Decisions governing this change
Active proposals touching this changeThese are not yet ratified and do not bind this change:
|
There was a problem hiding this comment.
🟡 Changes recommended
The bootstrap write path and backfill/public guidance still have unresolved issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR releases agent plugin 0.3.1 and fixes bootstrap detection, corpus validation, and draft scaffolding.
Changes:
- Reads all decision buckets and uses corpus-wide linting.
- Makes backfill detection reachable and updates absent-corpus handling.
- Refreshes tests, documentation, ADR evidence, and version metadata.
File summaries
| File | Summary |
|---|---|
site/src/content/docs/backfill.mdx |
Updates public guidance; status coverage and the corpus-wide lint prerequisite need alignment. |
packages/adapters/agent-plugin/test/wiring.test.ts |
Expands guidance regression tests. |
packages/adapters/agent-plugin/test/fixtures/unsafe-backfill-guidance.md |
Adds contradictory guidance cases. |
packages/adapters/agent-plugin/skills/decision-memory/SKILL.md |
Updates skill version and decision-memory guidance. |
packages/adapters/agent-plugin/skills/decision-backfill/SKILL.md |
Revises reconciliation across decision buckets. |
packages/adapters/agent-plugin/package.json |
Bumps the package to 0.3.1; expected-version references need alignment. |
packages/adapters/agent-plugin/commands/adr-draft.md |
Adds absent-corpus handling, but checks still precede scaffolding. |
packages/adapters/agent-plugin/commands/adr-backfill.md |
Ports detection to the command; missing-directory handling needs source-inventory qualification. |
packages/adapters/agent-plugin/apm.yml |
Updates the plugin version. |
packages/adapters/agent-plugin/.claude-plugin/plugin.json |
Updates the plugin version. |
docs/reference-verification-agent-plugin.md |
Adds re-measured verification scenarios. |
docs/adr/0038-offer-the-bootstrap-decision-record-as-an-offer-rather-than-a-backfill-candidate.md |
Amends the bootstrap decision guidance. |
CHANGELOG.md |
Documents the 0.3.1 fixes. |
bun.lock |
Aligns workspace version metadata. |
AGENTS.md |
Updates plugin status and repository guidance. |
.claude-plugin/marketplace.json |
Updates marketplace version metadata. |
Review details
Suppressed comments (2)
site/src/content/docs/backfill.mdx:196
- The public matrix omits two statuses that the new bucket contract treats as decisive:
draftis inactiveProposals, anddeprecatedis inhistory. As written, a process record in either state matches no row, so this page does not tell the agent to ratify an existing draft or never re-propose a deprecated decision. Include both statuses in the corresponding rows.
| A process record covering it is already proposed | Nothing — ratify the one you have | none |
| A process record covering it was rejected or superseded | Nothing — it is settled against | none; never re-proposed |
site/src/content/docs/backfill.mdx:202
- This new site guidance still treats an empty bucket result as enough to offer bootstrap, but does not require the corpus-wide exit-code check. For an unmigrated MADR corpus,
adr checkreturns empty buckets at exit 1 because records do not parse, so readers following this page will offer a duplicate instead of migrating. Mirror the command/skill rule: runadr lintfirst and interpret empty buckets only after exit 0.
Detection reads all three of `adr check`'s buckets, not `governing` alone —
`governing` holds `accepted` records only, so a process decision that is merely
*proposed* (which is what `/adr-draft` writes) or one that was *rejected* would
otherwise look like no decision at all.
- Files reviewed: 15/16 changed files
- Comments generated: 4
- Review effort level: Lite
Comment on lines
+58
to
+62
| error and also stops — **except** when the corpus directory does not exist | ||
| yet. There is nothing to lint in that case, and `adr new` creates the | ||
| corpus directory and allocates `0001` on its own, so proceed to the | ||
| scaffolding step instead of stopping. Distinguish the two: `Corpus | ||
| directory not found` is the bootstrap case; any other exit `2` is a real |
Comment on lines
+133
to
+135
| corpus directory that does not exist exits `2`; one that exists but | ||
| holds no record exits `0` with an empty result. Both mean the same | ||
| thing here, and the offer is both decisions. Once at least one record |
| { | ||
| "name": "@adrkit/agent-plugin", | ||
| "version": "0.3.0", | ||
| "version": "0.3.1", |
| | No corpus, or no record in any bucket covers the corpus directory | Process and tooling decisions | `relatesTo` between them when split | | ||
| | An accepted process record governs the corpus directory | Tooling decision only | `relatesTo` that record | | ||
| | A process record covering it is already proposed | Nothing — ratify the one you have | none | | ||
| | A process record covering it was rejected or superseded | Nothing — it is settled against | none; never re-proposed | |
…ctory, and ratify ADR-0038 The first functional run of the offer's write path, in an ephemeral consumer repository, found a fourth defect that three static review lenses and the 0.3.1 corrections all missed. `adr new` scaffolds `affects: []` and `status: draft`. So the bootstrap record written through the offer's own prescribed path binds nothing, and a record that binds nothing is invisible to detection however many buckets are read: every bucket comes back empty and the next audit offers the same decision again. The three-bucket fix only engages once the record carries a matcher covering the corpus directory. Measured end to end: adr lint --dir docs/adr -> exit 2 (bootstrap case) adr new "<title>" --dir docs/adr -> exit 0, creates 0001 adr lint --dir docs/adr -> exit 0 adr check --dir docs/adr -- 0001-*.md -> exit 0, ALL THREE buckets empty ...after hand-adding affects docs/adr/** -> activeProposals ["0001"] ADR-0038 had listed exactly this under "how we would know this was wrong" -- "a consumer's bootstrap record lands with no rejected alternative and no `affects` matcher" -- and it turned out to be the default behavior of the path the offer prescribes, not a hypothetical. Both the skill and the command now require the offer to state the matcher. The command says "drafting scaffolds" rather than naming `adr new`, because the write-boundary test forbids the read-only audit command from naming the writer; that guard caught the first wording. ADR-0038 is ratified: accepted by @mbeacom, with `provenance.ratifiedBy` set, which the corpus's own `agent-accepted-requires-ratifier` rule requires of an agent-drafted record and which rejected the first attempt. The decision is unchanged from the original proposal -- what was wrong was the mechanic implementing it, now recorded in Consequences. Action items 5, 7 and 8 stay open and are tracked rather than closed by ratification: a host run confirming the offer is surfaced at all, `ADR_DIR` resolution ignoring repository discovery, and mirroring the no-corpus clause into /adr-context and /adr-check. Signed-off-by: Mark Beacom <m@beacom.dev>
mbeacom
changed the title
Owner
Author
Update: a fourth defect, found by the functional run — and ratificationBefore ratifying I exercised the offer's write path end to end for the first
ADR-0038 had already listed this under how we would know this was wrong — "a Two guardrails in this repository caught my own mistakes while doing it, which is
RatificationADR-0038 is accepted, ratified by Three action items stay open, tracked rather than closed by ratification:
Verification2814 pass / 0 fail, typecheck, lint, seven check scripts, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Follow-up to #199. A deep review of that PR — run after it merged — found three
defects in the 0.3.0 bootstrap offer, each found by executing the CLI rather
than reasoning about it, and each of which offers a repository a decision record
it does not need. The plugin is catalogued from the repo-root marketplace, so
0.3.0 is already reachable by consumers; this is a fix-forward to 0.3.1.
The decision ADR-0038 records is unchanged, so it is amended, not superseded.
What was wrong was the detection mechanic and the reachability of the write path.
1.
governingholdsacceptedalone — blockingDetection read
governingby itself, so a process record that wasproposedorrejectedcame back as absence. Measured against a CLI built from this branch:governingactiveProposalshistorystatus: proposed,affects: docs/adr/**0[]["0001"][]status: rejected0[][]["0001"]The consequence was circular: backfill offers the bootstrap record,
/adr-draftwrites it
proposed, and the next run offers the same decision again — thetool re-offering its own output. A
rejectedrecord was re-proposed, whichdecision-memory/SKILL.md:31names as the third failure the plugin exists toprevent, and which contradicted
decision-backfill/SKILL.md:50's own rule toreconcile against "accepted, proposed, rejected, and superseded records".
Detection now reads all three buckets and distinguishes already proposed
(ratify it, offer nothing) from settled against (report it, never re-propose).
2. The exit-code guard read a path-scoped signal as corpus-wide — major
adr check's exit code covers the paths it was handed, not the corpus. Measured:a malformed process record beside a healthy unrelated record returns exit
0with empty findings when the healthy record is probed, while corpus-wide
adr linton the same repository exits1. The corpus-wide gate is nowadr lint.3. The offer's write path did not work — blocking
The offer named
/adr-draft, whose gate stops onadr lintexit2— preciselywhat a repository with no corpus returns, which is the offer's entire audience.
Measured on a fresh repo:
adr lint --dir docs/adrexits2;adr new "<title>" --dir docs/adrexits0, creates the directory and allocates0001, becausecreateAdrcallsmkdirrecursively. ADR-0038's own Option C already recordedthis. The gate is narrowed to the absent-corpus case; a corpus that exists and
does not parse stays a hard stop.
4. The detection procedure was unreachable from the entry point — major
/adr-backfillloads the command, and the whole detection procedure lived inthe skill. The command also never stated the edge table's middle row. Both are
ported, and the safety-policy checker now enforces the bootstrap rules against
both surfaces, so guidance cannot regress into the skill alone again.
Evidence
Per ADR-0016,
the checks were observed failing first — four assertions red against 0.3.0
guidance, then green after the fix. The retained contradictory fixture
(
test/fixtures/unsafe-backfill-guidance.md) gained matching negative cases, sothe new rules have a permanent executable counter-example rather than an
attestation that one was once seen. That gap was itself a review finding against
0.3.0.
docs/reference-verification-agent-plugin.mdgains a re-measured table coveringthe
proposed,rejected, and mixed-validity corpora the original five-shapematrix never exercised.
Checklist
proposed); two new action items left openpackages/ci/srcand@adrkit/coreunchanged — N/A;packages/ci/distdeliberately untouchedtypecheck,test(2814 pass / 0 fail),lint,check:changelog,check:site-grammar,check:doc-pins,check:freeze-hashes,check:clause8,check:no-spike-heuristics,check:deps,adr lint(38 records, 0 errors), bothclaude plugin validatetargets, and the site build all passKnown limitations, deliberately left open
item 5 is widened to cover the write path, not just whether a host surfaces
the offer.
ADR_DIRresolution still ignores step-1 discovery (new action item 7). Apre-adrkit MADR corpus at a non-default path —
docs/decisions/is MADR's ownconvention — resolves to a nonexistent
docs/adr, exits2, and reads as "nocorpus" instead of routing to migration. Narrowed in guidance here; the
resolution order is unchanged.
/adr-contextand/adr-checkdo not carry the no-corpus clause thatdecision-memorygained (new action item 8). Left as a scope call for themaintainer rather than decided here.