docs(#11): pin the adoption path to a tagged release - #22
Conversation
- Adds a "Pin a version" step to the 5-minute adoption path so adopters vendor the template, schema, and validator from a tag instead of a moving main branch - Links the releases page and CHANGELOG so the delta between versions is discoverable at the moment someone decides which version to copy - Names v1.2.0, the first tagged release of the standard Refs #11
me2resh
left a comment
There was a problem hiding this comment.
Code Review: PR #22
Commit: 7a87aee02e39c41a04c0f1aa9e2fd678b74c6380
Scope: Reduced-scope pass — Lean tier (AgDR-0116). Docs-only, 8 added lines to README.md, no behaviour change, no security / trust-chain / migration path. The mandatory checks (PR description + Glossary, AgDR detection, handbook discovery, marker mechanics) ran unchanged.
Summary
Adds a "Pin a version" step to the "Adopt in 5 minutes" section, telling adopters to take the template, schema, and validator from a tagged release rather than from main. It sits between the three-file list and the per-agent install instructions, links the releases index and CHANGELOG.md, and carries a git clone --branch v1.2.0 --depth 1 snippet.
The change is correct and well-argued. The verdict is APPROVED with one merge-order condition (below), plus two non-blocking notes.
Checklist Results
- Architecture & Design: N/A (reduced scope — no code)
- Code Quality: N/A (reduced scope — no code)
- Testing: N/A (reduced scope — no code)
- Security: Pass — no auth / crypto / secrets / trust-chain path touched
- Performance: N/A
- PR Description & Glossary: Pass
- Summary Bullet Narrative: Pass (no advisory finding)
- Technical Decisions (AgDR): N/A
- Adopter Handbooks: Pass (4 loaded, 0 findings)
- Fallow Static Analysis (JS/TS): N/A — no JS/TS in diff
Verification of the PR body's load-bearing claims
I checked these against git rather than taking them on trust, since the release target depends on them. All three hold.
| Claim | Result |
|---|---|
7eca2d8 is not an ancestor of main |
Confirmed — git merge-base --is-ancestor 7eca2d8 origin/main exits non-zero |
main carries the same change as e4e1a4e (#21) |
Confirmed — origin/main tip is e4e1a4e17029…, subject docs: add llms.txt machine-readable surface (#21); 7eca2d8 carries the same subject without the (#21) squash suffix |
7eca2d8 is missing .github/ISSUE_TEMPLATE/bug-report.yml from #20 |
Confirmed — absent from that tree, present in e4e1a4e |
Two things worth adding, because they make the case stronger than the body states:
git diff --name-status 7eca2d8 e4e1a4ereturns exactly one line —A .github/ISSUE_TEMPLATE/bug-report.yml. The two commits are otherwise identical trees, which is precisely why the wrong one is easy to grab and why the distinction was worth writing down.- The local
v1.2.0tag already points ate4e1a4e, so the staged release is aimed correctly.
Focus area: link-check safety — safe, and settled empirically
lychee already ran against this exact head: 143 links, 134 successful, 9 excluded, 0 errors, job Check markdown links = pass. All three checks on the PR are green.
Neither new link matches an exclude pattern in .lychee.toml (those cover only /actions/workflows/ badges and skillshield.io), so both were genuinely checked rather than skipped, and CHANGELOG.md is confirmed present at the repo root.
On the specific worry about the fenced block — it is moot by construction, not by luck. --branch v1.2.0 is a git CLI flag, not part of the URL. The only URL in the block is the bare repo .git address, which carries no tag component at all. So whichever way lychee handles fenced code blocks, no tag-existence check can reach the link checker. That is a more durable property than "the code block happens not to be extracted", and it means future edits to the snippet's tag can't break link-check either.
Required before merge: publish the release first
The dependency is real, and slightly broader than the body describes. Verified: git ls-remote --tags origin returns nothing — the repo has zero remote tags. The v1.2.0 tag exists only locally, and the GitHub Release is a Draft whose API html_url is literally …/releases/tag/untagged-839e3d09bf312ed1bc29, which independently confirms no tag has been pushed.
That validates the decision to link the index instead of /releases/tag/v1.2.0 — that URL 404s today. But the same fact cuts one step further than the body accounts for: the releases index this prose links is empty to anonymous readers until the draft is published. So merging first doesn't only leave a git clone that fails — it leaves a step titled "Pin a version" pointing at a page listing no versions.
Documenting the ordering in the PR body is good practice, but it is a human-held invariant with no mechanical guard. The cheap fix is to invert the order rather than defend it: publish the draft release (which pushes the tag), then merge. One action, and the dependency disappears entirely.
I'm treating this as a merge-order condition rather than a blocking change request: the README is correct as written the moment the tag lands, the condition is already flagged prominently under its own heading, and the damage from an inverted order is a brief cosmetic window that self-corrects — not anything lossy. Reword the README only if you'd rather it be order-independent; pinning advice reads better with a concrete version in it, so ordering the two steps is the better trade.
Repo conventions — pass
- Style guide (
CONTRIBUTING.md§ Style Guide): no new headers, so ATX is N/A; the fenced block carries abashlanguage hint; no comparison that wants a table. The two new prose lines exceed 100 characters — but so does nearly every prose line in this README, including lines 81, 85 and 87 in the very section being edited. The guide says "where possible", and the new lines match the file's established one-sentence-per-line style. Flagging it would fight the file's own convention, so I'm not raising it. SPEC.mdas the single normative source: pass, and better than neutral. The addition is adoption advice — no RFC-2119 keyword, no rule about the AgDR format,SPEC.mduntouched. It also lands consistent with SPEC's own "Change history" section, which already states that substantive spec changes are breaking for existing validators. The README now gives adopters the mechanism to insulate themselves from exactly that, without restating the rule.llms.txt(shipped in #21, immediately prior): I checked it for a cross-surface conflict, since it points atblob/main/…throughout. Recording deliberately that this is not an inconsistency to fix —llms.txtis a discovery surface that should describe the spec as it is now, while this README advice is about vendoring into your own repo. Different acts, different correct answers. Worth leaving alone.
Issues Found
None blocking.
nit (accuracy) — the causal claim in the "Pin a version" sentence.
"otherwise a later spec change quietly moves the bar your CI enforces" is the weaker half of the argument for the case this paragraph is literally describing. The surrounding text is about vendoring ("copy three things into your repo"), and a vendored copy is frozen the moment you take it — a later upstream change cannot move your bar. The moving-bar mechanism does hold for live references (a JSON-Schema $ref to a main URL, or an agent re-pulling from the blob/main/ links in llms.txt), just not for the copy the sentence is about. Worth noting validate-agdr.yml is not a reusable workflow (on: push/pull_request, no workflow_call), so the "reference the workflow from main" path isn't available either — the vendored reading really does dominate here.
The second clause already carries the accurate harm: "on a commit you never chose" — you can't name what you adopted, cite it in your own AgDR, or read the CHANGELOG delta to upgrade. Both arguments land on "pin", so the advice is right as written; consider just leading with the unidentifiability point and keeping the moving-bar point for live references.
suggestion (maintainability) — v1.2.0 is now an unwatched hardcoded version string.
check-changelog-lockstep.js validates plugin.json ↔ marketplace.json ↔ CHANGELOG.md; README.md is outside its scope, and the repo has no RELEASING.md or release checklist. On the next version bump, every check stays green while the quickstart still tells adopters to clone v1.2.0. Not harmful — a pinned older version is still a valid pin, and the prose links the index and CHANGELOG so a reader can pick a newer one — but it will quietly rot, which is a slightly awkward failure for the section whose whole thesis is "don't copy off a moving target".
Two cheap options, either is fine:
- Extend
check-changelog-lockstep.jsto assert the README snippet's tag matches the manifest version. It already reads files at arbitrary refs, and this fits its stated purpose ("traceability check for this repo's own release process") in a few lines. - Or fold the README bump into the auto-release workflow that #11 still asks for, so it happens on the same trigger as the tag.
Handbook Findings
None. Four handbooks loaded (discovery: path-convention) — handbooks/architecture/migration-safety.md, handbooks/architecture/clean-architecture-layers.md, handbooks/general/commit-message-quality.md, and one private architecture handbook.
- Migration Safety is a blocking handbook, but N/A here — the diff touches zero migration paths.
- Clean Architecture Layering (public + private): N/A — no layered source in the diff.
- Commit Message Quality (advisory): passes.
docs: pin the adoption path to a tagged releaseis imperative, 46 chars, no trailing period, with a WHY-focused three-bullet body and a singleRefs #11. The absent(#11)subject scope is not a violation in this repo: its established convention onmainisdocs: … (#PR)via the squash suffix, and the PR title already carriesdocs(#11):.
PR quality
- Summary bullets: all three are narrative — each names what changed and why it matters, well clear of the label-only heuristic. No advisory finding; this is a good example of the rule rather than an exception to it.
- Glossary: present, five terms, table format. The
Squash-mergeentry does real work — it's the concept that explains why7eca2d8ande4e1a4eare different SHAs for the same change, which is the crux of the release-target decision. - Ticket link:
Refs #11, andRefsrather thanClosesis the right call — #11 also asks for the tag, the Release, and an auto-release workflow, none of which are in this diff.
Technical Decisions (AgDR)
N/A. Docs-only: no new dependency, technology, service, schema, data-model, or security control; no repo-wide pattern; reversible in a single revert. Per the materiality threshold this is not an AgDR-class decision. (The repo keeps no docs/agdr/ of its own — it ships examples/ instead.)
Verdict
APPROVED — conditional on publishing the v1.2.0 release (which pushes the tag) before merging.
The two notes above are non-blocking and can be picked up here or in the follow-up that closes the rest of #11.
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 7a87aee02e39c41a04c0f1aa9e2fd678b74c6380
Rex's review caught that the original wording claimed a later spec change moves the bar your CI enforces. It does not, for the case the paragraph actually describes: a vendored copy is frozen at the moment you copy it, and validate-agdr.yml is not a reusable workflow, so there is no live reference back to main either. The real harm is provenance — nothing records which version you adopted, so the next re-vendor silently lands on a different one. Refs #11
me2resh
left a comment
There was a problem hiding this comment.
Delta Re-Review: PR #22
Commit: 0ed3f591b7886de77ce3ee25b628cdd60f783a45
Previously reviewed: 7a87aee02e39c41a04c0f1aa9e2fd678b74c6380 (review)
Scope: Delta-only pass over the single commit on top of the reviewed head. Everything cleared at 7a87aee — ancestry claims, repo conventions, PR quality, handbooks, AgDR N/A — carries forward; I re-derived only what this delta could disturb.
The delta is clean
git diff 7a87aee 0ed3f59 is 1 file changed, 1 insertion(+), 1 deletion(-) — exactly the causal clause in the "Pin a version" paragraph. Nothing rode along: the link, the fenced block, the trailing CHANGELOG sentence, and every other line in the 8-line addition are byte-identical. PR totals are unchanged at 8 additions / 0 deletions across one file.
The replacement claim is accurate — verified, not assumed
The new clause is "otherwise nothing records which version you adopted, and the next person to re-vendor quietly picks up a different one". I checked the first half against the three artifacts the paragraph tells you to vendor, since "nothing records which version" is only true if none of them self-identifies:
| Vendored artifact | Carries a version? |
|---|---|
schema/agdr.schema.json |
No — $id is …/schema/agdr.schema.json, an unversioned URL |
scripts/validate-agdr.js |
No — no version constant anywhere in the file |
agdr-template.md |
No — no version field in the frontmatter, no version header |
So the claim is literally true, not merely defensible: copy those three files out of main and your repo retains no record of which upstream revision you took. The second half follows from the first — re-vendoring later lands on whatever main is then, and because nothing recorded the original, the drift is genuinely silent. "Quietly" is the right word.
This did not trade one imprecise claim for another. The old clause made an assertion about CI behaviour that was false for the vendored case; the new clause makes an assertion about provenance that I verified holds. It also reads better in context: the very next sentence is "That tag is also what you cite when you record adopting AgDR as an AgDR of your own" — a provenance payoff that the old CI-bar framing set up much more weakly.
Sentence structure is intact. The em-dash aside is slightly longer, but the colon still attaches to the imperative ("take them from a tagged release … :") and the code block still reads as the remedy rather than an illustration of the harm — the same structure as before the edit. The *which* emphasis is valid markdown and consistent with the file.
New finding: link-check is RED at this head — pre-existing, not caused by this PR
Check markdown links fails at 0ed3f59. The cause is unrelated to this change:
### Errors in README.md
* [ERROR] <https://apexscript.com/> (at 292:3) | Connection failed. Check network connectivity and firewall settings
That link is the "Organizations using AgDR" table row, at line 292 here and already present on main at line 284 — the 8-line offset is exactly this PR's addition. Nothing in this delta, or in the PR as a whole, touches it.
The run totals confirm it's a single isolated flip, not a systemic break: 143 total (unchanged), 9 excluded (unchanged), successful 134 → 133, errors 0 → 1. Both of this PR's new links still pass. The previous head was green 26 minutes earlier, and the error is Connection failed — a DNS/TCP failure, not an HTTP status — which is the shape of a transient outage or a site that has just gone down.
This does not change my verdict on the change itself, but it does block the merge under the "no red CI before merge, even when the failure is pre-existing or unrelated" rule. Suggested order:
- Re-run the job first — it costs nothing and distinguishes a transient outage from a dead site. If it goes green, you're done.
- If it persists, fix it in a separate commit, not this one: drop or repoint the row, or add the host to
.lychee.toml'sexclude. There's precedent in that file already — it excludes a third-party badge service on explicitly "decorative, not a doc link worth gating CI on" reasoning, and an unreachable consultancy homepage in an adopters table sits in the same category.
Worth noting the red gate here is block-merge-on-red-ci.sh, not my approval — my marker asserts the code was reviewed, not that CI is green. The two conditions are independent and both must hold.
New finding: the PR body still asserts the claim you just corrected
Non-blocking, but worth fixing before merge. Summary bullet 1 still reads:
Copy from
mainand a later spec change silently moves the bar your own CI enforces, on a commit you never chose.
That is the wording 0ed3f59 removed from the README, restated in the description. The Testing section likewise still says "link-check (lychee) stays green", which is currently false at this head (for the unrelated reason above).
This matters more than a normal description nit because the PR body becomes the squash-merge commit message — so the imprecise claim would be preserved permanently in main's history, in the commit whose entire purpose was to remove it. A one-line edit to the bullet, mirroring the new README wording, closes it. The Glossary needs no change: "re-vendor" is a natural derivation of the Vendor entry that's already there.
Carried forward from the 7a87aee review
- Ancestry claims — all three verified there and untouched by this delta (
7eca2d8not an ancestor;maintip ise4e1a4e;bug-report.ymlis the sole tree difference; the local tag targetse4e1a4ecorrectly). - Merge-order condition — publish the draft release (which pushes the tag) before merging, since the releases index is empty to anonymous readers until then. Understood to be with the operator; not re-litigated here.
- Conventions — the delta adds no normative rule, so
SPEC.mdstill needs no update; fenced block and language hint untouched; line length still consistent with this README's established one-sentence-per-line style. - Handbooks — re-checked
commit-message-qualityagainst the new commit:docs: state the accurate harm of vendoring from mainis imperative, 51 chars, no trailing period, with a WHY-focused body that records the reasoning and a singleRefs #11. Passes. Migration Safety (blocking) and both Clean Architecture handbooks remain N/A. - AgDR — still N/A. Fallow — still N/A (no JS/TS).
Verdict
APPROVED at 0ed3f59 — the delta is clean, narrowly scoped, and the replacement claim is accurate.
Two independent conditions still gate the merge, neither of them a defect in this change:
link-checkmust be green (re-run first — likely transient; fix separately if not).- The
v1.2.0release must be published before merge, per the earlier review.
The PR-body correction above is worth doing on the way past, since that text lands in the permanent commit message.
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 0ed3f591b7886de77ce3ee25b628cdd60f783a45
Summary
schema/agdr.schema.json, andscripts/validate-agdr.js— but never says which version of them. None of the three self-identifies: the schema's$idis unversioned, the validator carries no version constant, the template has no version field. So nothing records what you adopted, and the next person to re-vendor quietly lands on a different revision. This adds a "Pin a version" step pointing at a tagged release instead.Docs-only, eight added lines, no normative change:
SPEC.mdis untouched and the validator behaves identically.Type of change
README.md,CONTRIBUTING.md, etc.)Merge order — this PR depends on the release being published
The snippet runs
git clone --branch v1.2.0, which fails until the tag is pushed. Worse, the paragraph links the releases index, which lists nothing to anonymous readers while the release is still a draft — so merging first ships a step titled "Pin a version" pointing at a page with no versions on it.Publish the v1.2.0 release before merging this. The tag and Release are the other half of #11 and are staged as a draft; they are not in this diff, because a tag is not a reviewable artifact.
Two notes for whoever picks up the rest of #11:
e4e1a4e, the current tip ofmain— not7eca2d8, which carries the same subject line ("docs: add llms.txt machine-readable surface") but is the pre-squash branch commit. It is not an ancestor ofmainand is missing.github/ISSUE_TEMPLATE/bug-report.ymlfrom docs(#13): add [Bug] issue form #20. Tagging it would have pinned v1.2.0 to an unreachable commit with incomplete content./releases/tag/v1.2.0. A direct tag URL 404s until publish, and the index needs no edit on future releases. The trade-off is the empty-index window described above.v1.2.0is a hardcoded string in the README that nothing currently watches —check-changelog-lockstep.jscovers only the two manifests plusCHANGELOG.md. At the next version bump every check stays green while this section still says clone v1.2.0. Worth folding into the auto-release workflow Cut a tagged v1.2.0 release (and release on every future version bump) #11 also asks for.Testing
link-check(lychee): both links introduced here resolve. The job is currently red on a pre-existing failure unrelated to this PR — theapexscript.comadopters-table link, already onmain, returned a connection failure (DNS/TCP, not an HTTP status). A re-run has been triggered to establish whether it is transient; if it persists it wants a separate fix, on the same "decorative, not worth gating CI on" reasoning.lychee.tomlalready applies to a third-party badge service.v1.2.0is pushed,git clone --branch v1.2.0 --depth 1 https://github.com/me2resh/agent-decision-record.gitshould succeed.No change to
examples/,schema/,scripts/, or.claude-plugin/, sonpm run validateand the changelog-lockstep check are unaffected.Checklist
npm run validatepasses (unaffected — no change toexamples/,schema/, orscripts/validate-agdr.js)npm run validate:changelogpasses (unaffected — no manifest version bump)main(see Testing)SPEC.mdneeds no updateGlossary
v1.2.0always means this exact tree, unlikemain, which advances with every merge.main— which is why7eca2d8ande4e1a4eare different SHAs for the same change.link-check.yml; it fails the build on any markdown link that does not resolve.Refs #11