Skip to content

docs(#11): pin the adoption path to a tagged release - #22

Open
me2resh wants to merge 2 commits into
mainfrom
docs/GH-11-pin-a-version
Open

docs(#11): pin the adoption path to a tagged release#22
me2resh wants to merge 2 commits into
mainfrom
docs/GH-11-pin-a-version

Conversation

@me2resh

@me2resh me2resh commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adopters currently copy the standard off a moving target. The 5-minute adoption path tells you to vendor three files — the template, schema/agdr.schema.json, and scripts/validate-agdr.js — but never says which version of them. None of the three self-identifies: the schema's $id is 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.
  • It links the releases page and CHANGELOG at the decision point, so someone choosing a version can see the delta between them without leaving the quickstart.
  • It ties the pin back to the standard's own philosophy — the tag is what you cite when you record adopting AgDR as an AgDR of your own.

Docs-only, eight added lines, no normative change: SPEC.md is untouched and the validator behaves identically.

Type of change

  • Docs (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:

  • The release targets e4e1a4e, the current tip of main — not 7eca2d8, 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 of main and is missing .github/ISSUE_TEMPLATE/bug-report.yml from docs(#13): add [Bug] issue form #20. Tagging it would have pinned v1.2.0 to an unreachable commit with incomplete content.
  • The prose deliberately links the releases index rather than /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.0 is a hardcoded string in the README that nothing currently watches — check-changelog-lockstep.js covers only the two manifests plus CHANGELOG.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

  1. Read the rendered "Adopt in 5 minutes" section — the pin step sits between the three-file list and the per-agent install instructions.
  2. link-check (lychee): both links introduced here resolve. The job is currently red on a pre-existing failure unrelated to this PR — the apexscript.com adopters-table link, already on main, 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.toml already applies to a third-party badge service.
  3. After v1.2.0 is pushed, git clone --branch v1.2.0 --depth 1 https://github.com/me2resh/agent-decision-record.git should succeed.

No change to examples/, schema/, scripts/, or .claude-plugin/, so npm run validate and the changelog-lockstep check are unaffected.

Checklist

  • npm run validate passes (unaffected — no change to examples/, schema/, or scripts/validate-agdr.js)
  • npm run validate:changelog passes (unaffected — no manifest version bump)
  • Markdown links introduced by this PR resolve; the one failing link is pre-existing on main (see Testing)
  • Normative rules unchanged, so SPEC.md needs no update

Glossary

Term Definition
Pin (a version) Depending on a fixed, named revision — here a git tag — instead of whatever a branch points at today, so the dependency cannot change under you.
Tag An immutable git label on one commit. v1.2.0 always means this exact tree, unlike main, which advances with every merge.
Squash-merge Merging a PR by collapsing its commits into one new commit on the base branch. The original branch commits are never reachable from main — which is why 7eca2d8 and e4e1a4e are different SHAs for the same change.
Vendor (a file) Copying a dependency's source into your own repo instead of installing it from a package registry. The zero-dependency adoption path for AgDR today.
lychee The link checker run by link-check.yml; it fails the build on any markdown link that does not resolve.

Refs #11

- 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 me2resh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

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 Confirmedgit merge-base --is-ancestor 7eca2d8 origin/main exits non-zero
main carries the same change as e4e1a4e (#21) Confirmedorigin/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 e4e1a4e returns 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.0 tag already points at e4e1a4e, 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 a bash language 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.md as 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.md untouched. 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 at blob/main/… throughout. Recording deliberately that this is not an inconsistency to fix — llms.txt is 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.jsonmarketplace.jsonCHANGELOG.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.js to 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 release is imperative, 46 chars, no trailing period, with a WHY-focused three-bullet body and a single Refs #11. The absent (#11) subject scope is not a violation in this repo: its established convention on main is docs: … (#PR) via the squash suffix, and the PR title already carries docs(#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-merge entry does real work — it's the concept that explains why 7eca2d8 and e4e1a4e are different SHAs for the same change, which is the crux of the release-target decision.
  • Ticket link: Refs #11, and Refs rather than Closes is 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 me2resh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

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:

  1. Re-run the job first — it costs nothing and distinguishes a transient outage from a dead site. If it goes green, you're done.
  2. If it persists, fix it in a separate commit, not this one: drop or repoint the row, or add the host to .lychee.toml's exclude. 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 main and 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 (7eca2d8 not an ancestor; main tip is e4e1a4e; bug-report.yml is the sole tree difference; the local tag targets e4e1a4e correctly).
  • 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.md still 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-quality against the new commit: docs: state the accurate harm of vendoring from main is imperative, 51 chars, no trailing period, with a WHY-focused body that records the reasoning and a single Refs #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:

  1. link-check must be green (re-run first — likely transient; fix separately if not).
  2. The v1.2.0 release 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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant