docs(agents): add development principles, slim release flow, correct branch contract - #96
Conversation
📝 WalkthroughWalkthroughThe release documentation now defines stable-named frozen branches, tag-pinned builds, updated fallback and backport procedures, npm-only setup, product constraints, and repository-wide documentation validation. ChangesRelease guidance and documentation validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.harness/docs/git-workflow.md:
- Around line 103-104: Revise the promote.yml statement in the git workflow
documentation to say it is the only normal automated writer that converts -rc.N
to the stable version. Preserve the documented manual maintainer fallback for
emergency use, while keeping the prerelease.yml branch reuse requirement
unchanged.
- Around line 116-120: Update the release workflow commands around the rc.1
version and tag literals so the RC fallback for rc.2+ derives or accepts the
intended RC number consistently. Ensure the package.json version, commit
message, and pushed tag all use the same 1.5.0-rc.N value while preserving the
existing rc.1 initial-release path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a6e935f-3dbd-4bf5-9a0f-497166f441e0
📒 Files selected for processing (2)
.harness/docs/git-workflow.mdAGENTS.md
There was a problem hiding this comment.
Review — LGTM
Does exactly what the title promises: adds a Development principles section + npm pin rationale to AGENTS.md, slims the duplicated Release-flow content into .harness/docs/git-workflow.md, and corrects the branch-contract and pipeline-dispatch details that were stale after the release-pipeline fixes (post-#90 / #91). All CI checks green.
Verified the doc matches the shipped workflow YAML (not the older pre-d5966ed5 shapes in the working tree):
- Branch naming
release/vX.Y.ZmatchesBRANCH="release/v${NEXT}"inprerelease.yml:99andBRANCH="release/v${STABLE_VERSION}"inpromote.yml:71. - Explicit
--refdispatch at line 59 matchesprerelease.yml:136-139's dispatch invocation. - Manual fallback now correctly adds the
git taglines the old version was missing (the old snippet pushed two tags it never created). - Branch contract (lines 92-106) preserves the four-key invariant the v1.6.0 incident was about. The postmortem paragraph at line 108 is preserved verbatim.
AGENTS.md's release section now correctly defers to git-workflow.md § Release flow rather than restating it, and the v1.6.0 reference stays in the right place.
Suggestions (non-blocking, could ride along or follow-up)
.harness/memory/MEMORY.md:26still describes the pre-d5966ed5pipeline (Bump package.json ... push tag) — same class of stale info this PR is correcting. Either ship with this PR or earmark as a tiny follow-up.docs/github-actions-workflows.md:42-43, 121-138describe the pre-f62162b1pipeline (the mermaid diagram still shows "push tag → build" edges that don't match today's dispatch flow). Intentional out-of-scope? Worth aligning.- The new
Product constraintsbullet ("no feature-gating logic") is broader than the previousREADME tonerule. Could you add a one-line clarifier or example so an agent knows where the line is (e.g. "feature flags for in-development capture backends are fine; paywall gates are not")? - "No abstraction for hypothetical needs (YAGNI)" and "the simplest solution that stays readable" overlap thematically — intentional (the first cites the principle, the second operationalizes it), or merge?
| - Format: `npm run format` (Biome, tabs, double quotes, 100-col) | ||
| - i18n check: `npm run i18n:check` (validates the 13 locale files) | ||
|
|
||
| **Use npm, not bun/pnpm/yarn/Deno.** Not a style preference: the native Swift (macOS) and C++ (Windows) capture helpers are rebuilt against Electron's ABI by electron-builder + `@electron/rebuild` resolving `package-lock.json`, and other package managers break that path. `packageManager` + `engines` in `package.json` pin the versions; CI installs with `npm ci`. |
There was a problem hiding this comment.
✓ The npm-pin rationale is the right kind of explanation. The "not a style preference: ..." framing converts a vague imperative into a justified constraint, which is exactly what AGENTS.md should be doing. Future agents will read this and understand why without needing to spelunk through the native build docs.
Minor: "the native Swift (macOS) and C++ (Windows) capture helpers are rebuilt against Electron's ABI by electron-builder + @electron/rebuild resolving package-lock.json" is dense. Could split into two sentences — one for the what (electron-builder + @electron/rebuild read package-lock.json), one for the consequence (other package managers break that path). Not blocking.
There was a problem hiding this comment.
Split in 8da0c3c — the what and the consequence are now separate sentences:
Not a style preference. The native Swift (macOS) and C++ (Windows) capture helpers are rebuilt against Electron's ABI by electron-builder +
@electron/rebuild, which resolve the tree throughpackage-lock.json. Another package manager writes a different lockfile, so that rebuild breaks.
|
|
||
| **Use npm, not bun/pnpm/yarn/Deno.** Not a style preference: the native Swift (macOS) and C++ (Windows) capture helpers are rebuilt against Electron's ABI by electron-builder + `@electron/rebuild` resolving `package-lock.json`, and other package managers break that path. `packageManager` + `engines` in `package.json` pin the versions; CI installs with `npm ci`. | ||
|
|
||
| ## Development principles |
There was a problem hiding this comment.
✓ The "No mandated app-stack choice yet." qualifier with the asterisk'd "may be enforced later" is the right escape hatch. Tells future agents this is provisional, not a forever rule.
Small wording nit: "Open to React/Vue/Svelte/whatever" is a bit informal. Consider "No mandated UI framework; React is current, but other choices are open" — same meaning, less jokey.
There was a problem hiding this comment.
No change needed here — the string you quoted isn't in the diff. The bullet reads:
No mandated app-stack choice yet. Contributors pick their own state/data library. Don't impose one across the codebase and don't refactor existing code onto a different one — keep each addition self-contained and consistent within its own module. A single choice may be enforced later.
No "React/Vue/Svelte/whatever" anywhere in AGENTS.md (git grep -i svelte is empty), and the scope is deliberately the state/data layer rather than the UI framework — React is already load-bearing here, so it isn't up for contributor choice and the bullet shouldn't imply it is. Flagging in case the rewording suggestion was aimed at something else.
| - **i18n**: 13 locales in `src/i18n/locales/<locale>/` (e.g. `src/i18n/locales/en/settings.json`). The `i18n:check` script validates them — run it after touching translation files. | ||
| - **Build pipeline**: `npm run build` is full electron-builder. For iterating on renderer only, use `npm run build-vite` (Vite + tsc, no packaging). | ||
| - **README tone**: the project is explicitly "not production-grade" and free forever — don't add paywalls, premium tiers, or upsell language to UI/copy. | ||
| - **Product constraints**: the project is free forever and explicitly "not production-grade". Don't add paywalls, premium tiers, or feature-gating logic, and don't add upsell language to the README or UI copy. This is a hard constraint, not a judgement call. |
There was a problem hiding this comment.
✓ The new Product constraints bullet is well-placed at the very end of AGENTS.md (the "don't forget this" spot). The "This is a hard constraint, not a judgement call." closer is a strong move.
The expansion to "feature-gating logic" beyond the previous README tone rule is meaningful but ambiguous — feature flags for in-development capture backends are arguably the right tool; paywall gates are not. Suggest a one-line exemplar like "feature flags for in-development backends are fine; paywall gates are not" so agents don't over-correct.
There was a problem hiding this comment.
Added in 8da0c3c, phrased as the axis rather than as an example list so it generalises past capture backends:
Don't add paywalls, premium tiers, or logic that gates a feature on who the user is, and don't add upsell language to the README or UI copy. This is a hard constraint, not a judgement call. (A flag that hides an unfinished capture backend is fine — it gates on readiness, not on the user.)
Gating on readiness vs. gating on the user is the line, and an agent can apply that to a case neither of us listed.
| 3. Commits `package.json` → `X.Y.Z-rc.N` on a fresh branch `release/vX.Y.Z-rc.N`. **The branch is NOT merged into `main`** — it stays frozen so the RC build only contains what was on `main` at the moment of cut. | ||
| 4. Pushes the tag `vX.Y.Z-rc.N` at the release branch tip. This triggers `build.yml`, which publishes a **GitHub pre-release** (badged as such, does not become "Latest"). macOS notarization is skipped on RC tags. | ||
| 3. Creates `release/vX.Y.Z` (at rc.1) or reuses it (rc.2+), and commits `package.json` → `X.Y.Z-rc.N` there. **The branch is NOT merged into `main`** — it stays frozen, so the RC contains the cut snapshot plus cherry-picks only. The branch is named for the **stable** version, with no `-rc.N` suffix, because `promote.yml` has to resolve the same ref. | ||
| 4. Pushes the tag `vX.Y.Z-rc.N` at the release branch tip, then **explicitly dispatches `build.yml` with `--ref` pinned to that tag**, which publishes a **GitHub pre-release** (badged as such, does not become "Latest"). Two reasons the dispatch is explicit and pinned: a `GITHUB_TOKEN` tag push does not fire `build.yml`'s `push:` trigger, and the build must check out the *tag* — `main` still carries the previous stable version and would fail the publish step's version guard. macOS notarization is skipped on RC tags. |
There was a problem hiding this comment.
✓ Explicit --ref dispatch matches prerelease.yml:136-139. The two cited reasons are accurate: GITHUB_TOKEN-pushed tags don't fire build.yml's push: trigger (correct — GitHub deliberately withholds that for security), and the build must check out the tag because main still carries the previous stable version (correct — the publish step has a version guard in build.yml:304-308).
| Pin the pre-release link in `#rc-testing`. Get the maintainer team + a few early adopters to install and smoke-test. | ||
|
|
||
| **Between RC cut and promote**, the only thing that may happen on `release/vX.Y.Z-rc.N` is **cherry-picks of bugfixes** that address problems discovered in the RC. Features, refactors, and CI/docs changes are **not** applied to the release branch — they live on `main` and ship in the next release cycle. | ||
| **Between RC cut and promote**, the only thing that may happen on `release/vX.Y.Z` is **cherry-picks of bugfixes** that address problems discovered in the RC. Features, refactors, and CI/docs changes are **not** applied to the release branch — they live on `main` and ship in the next release cycle. |
There was a problem hiding this comment.
✓ The "only cherry-picks of bugfixes" rule is the right contract. The wording "Features, refactors, and CI/docs changes are not applied to the release branch" is precise — those go to main and ship in the next cycle. Worth a git log release/vX.Y.Z..main hint for maintainers to see what's NOT on the branch.
There was a problem hiding this comment.
Added in 8da0c3c: git log release/vX.Y.Z..main --oneline lists exactly what is not in the RC.
|
|
||
| ``` | ||
| release/vX.Y.Z-rc.N exists from RC cut until promote finishes | ||
| release/vX.Y.Z created at rc.1, frozen through promote, kept for backports |
There was a problem hiding this comment.
✓ Branch-naming examples are correct: release/vX.Y.Z is the frozen RC history, release/vX.Y.Z-sync is the ephemeral post-promote sync branch. Matches the actual BRANCH="release/v${STABLE_VERSION}" in promote.yml:71 and the --branch release/v${STABLE_VERSION}-sync in the sync PR step.
| Key rules: | ||
|
|
||
| 1. **`prerelease.yml` creates the branch.** Nothing else pushes to it except the cherry-pick workflow during the RC window. | ||
| 1. **`prerelease.yml` creates the branch at rc.1 and reuses it for later RCs.** It must never delete or recreate it: that would drop the cherry-picks and silently re-cut from `main`, which defeats the freeze this contract exists to guarantee. |
There was a problem hiding this comment.
✓ Rule 1 — "prerelease.yml creates the branch at rc.1 and reuses it for later RCs. It must never delete or recreate it" — this is the exact invariant the v1.6.0 incident violated. The wording is correct, and the reference to the postmortem at line 108 (v1.6.0, 2026-07-05, 23 unreleased commits) ties it back to the real failure.
| ``` | ||
|
|
||
| The pipeline can't tell the difference between a manually-pushed tag and a workflow-pushed one — same `build.yml` runs either way. | ||
| A tag pushed manually with your own credentials **does** fire `build.yml`'s `push:` trigger, so the release publishes on its own. (The workflows push tags with `GITHUB_TOKEN`, which does *not* fire it — that's why they dispatch `build.yml` explicitly.) Either way the same `build.yml` builds and publishes. |
There was a problem hiding this comment.
✓ Manual fallback correctly distinguishes PAT-pushed tags (which do fire build.yml's push:) from GITHUB_TOKEN-pushed ones (which don't, requiring explicit dispatch). The old version of this snippet was confusingly identical for both cases. The new git-tag lines are also now present — the old version pushed tags it never created, which would have left users confused why the workflow didn't fire.
…branch contract
Contributor feedback on AGENTS.md, plus doc corrections the release-pipeline
fixes made necessary.
AGENTS.md:
- Add a "Development principles" section: simplest readable solution, no
speculative abstraction, and an explicit "no mandated app-stack choice yet"
rule so agents stop imposing one state/data library across the codebase.
Deliberately avoids "prefer one-liners" phrasing, which pushes models toward
dense code that hides control flow.
- Relabel "README tone" -> "Product constraints". The note bundles copy rules
with a hard product rule (no paywalls/premium gating); "tone" read as if the
model were being asked to reflect on the business model rather than follow a
constraint.
- Slim "Release flow" from 305 to 186 words, deferring the branch contract,
cherry-pick rules and manual fallback to .harness/docs/git-workflow.md, which
already documents them in full. The section was duplicated context loaded into
every agent run.
- Make the npm pin rationale explicit: it exists because the native helpers are
rebuilt against Electron's ABI via package-lock.json, not as a style choice.
.harness/docs/git-workflow.md — corrected to match the shipped pipeline:
- Release branch is release/vX.Y.Z (one per stable version, created at rc.1 and
reused), not release/vX.Y.Z-rc.N. The old naming is what broke promote, which
resolves release/v${STABLE_VERSION}.
- The RC tag push does NOT trigger build.yml; prerelease.yml dispatches it
explicitly with --ref pinned to the tag. Documented why.
- Manual fallback: fix the branch naming and add the missing `git tag` commands
(it pushed tags it never created).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The manual fallback was the only real defect. It bumped `package.json` with a `sed` while `set-release-version.mjs` — which prerelease.yml and promote.yml both call — also writes `package-lock.json`. A fallback release cut by hand therefore shipped a lockfile disagreeing with the package it locks, the exact drift that script was written to stop. It now calls the script, and hoists the RC version into `$RC` so rc.2+ does not need three edits kept in sync. Also: "promote.yml is the only writer" now says *automated* writer, so it stops contradicting the manual fallback three sections below; the cherry-pick rule points at `git log release/vX.Y.Z..main` for seeing what is not in the RC; the product constraint distinguishes gating on readiness from gating on the user, so an unfinished capture backend can still hide behind a flag; and `.harness/memory/MEMORY.md` no longer describes the pre-#90 `release/vX.Y.Z-rc.N` naming.
0a2c5dc to
8da0c3c
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Review round addressed — rebased onto
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.harness/docs/git-workflow.md:
- Line 90: Update the release-branch lifecycle guidance in git-workflow.md to
remove the exception allowing deletion when a future major release cuts over.
Ensure it consistently states that release branches remain available
indefinitely and are never deleted or recreated.
- Around line 96-99: Update the fenced code block containing the release branch
names in the documentation to specify the text language identifier, preserving
its contents unchanged.
- Line 116: Update the fallback guidance in git-workflow documentation to state
precisely that npm ci validates dependency data but does not reject mismatched
root version fields; explain that changing only package.json leaves stale
package-lock.json metadata, and continue directing releases through
set-release-version.mjs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4da868c6-c265-41f5-b9e0-f46ba6bfd4f5
📒 Files selected for processing (3)
.harness/docs/git-workflow.md.harness/memory/MEMORY.mdAGENTS.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.harness/docs/git-workflow.md:
- Line 48: Update the documentation note near the release-reference statement to
remove the claim that this copy is outside the docs checker. Clarify that the
page remains a pointer to avoid duplicated release instructions, and state that
scripts/check-docs.mjs validates both technical-documentation/ and .harness/.
In `@AGENTS.md`:
- Line 132: Update the release-branch policy sentence near “frozen until
promote” to use a clear noun phrase for the promotion event, such as “frozen
until stable promotion” or the existing promote workflow reference, while
preserving the surrounding release-branch rules.
In `@scripts/check-docs.mjs`:
- Line 77: Update the usage comment near the argument parser in check-docs.mjs
to document the supported equals-sign syntax as --only=a.md,b/c.md, matching the
parser’s handling of the only argument; do not imply that the space-separated
form is supported.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a8d2ed94-4a34-4ab7-96a3-79acb25287a2
📒 Files selected for processing (5)
.harness/docs/git-workflow.mdAGENTS.mdscripts/check-docs.mjsscripts/check-docs.test.mjstechnical-documentation/engineering/release-and-secrets.md
| Two `workflow_dispatch` workflows: cut an RC, then promote it to stable. **Full operational guide, branch contract, cherry-pick rules, and manual fallback: `technical-documentation/engineering/release-and-secrets.md`.** Read it before touching a release. | ||
|
|
||
| **Release branches freeze the build between cut and promote.** Every RC cut creates `release/vX.Y.Z-rc.N`. The branch is *not* merged into `main` until the stable tag is published; only cherry-picks of bugfixes land on the release branch during the RC window. The stable tag points at the branch tip (RC + cherry-picks), then `promote.yml` opens a `release/vX.Y.Z-sync → main` PR to bring main into line. This contract exists because of the v1.6.0 incident (2026-07-05) where the original promote workflow tagged `main` instead of the RC snapshot, causing 23 unreleased commits to ship in `v1.6.0`. Full rules in `.harness/docs/git-workflow.md` § Release branches. | ||
| The one rule to know before you merge anything: **there is one release branch per stable version** (`release/vX.Y.Z`), created at rc.1 and **frozen** until promote. Only cherry-picked bugfixes land on it, so anything merged to `main` after the cut ships in the *next* cycle, not the one in flight. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a noun for the promotion event.
frozen until promote is grammatically incomplete and does not identify the event clearly. Use frozen until stable promotion or frozen until \promote.yml` promotes it`.
The supplied LanguageTool hint identifies this line.
🧰 Tools
🪛 LanguageTool
[grammar] ~132-~132: Ensure spelling is correct
Context: ...zen** until promote. Only cherry-picked bugfixes land on it, so anything merged to `main...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` at line 132, Update the release-branch policy sentence near
“frozen until promote” to use a clear noun phrase for the promotion event, such
as “frozen until stable promotion” or the existing promote workflow reference,
while preserving the surrounding release-branch rules.
Source: Linters/SAST tools
| ]; | ||
|
|
||
| // `--only a.md,b/c.md` limits both checks to those docs-relative paths, so a | ||
| // `--only a.md,b/c.md` limits both checks to those repo-relative paths, so a |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the supported --only= syntax.
The parser only reads an argument that starts with --only=. The comment shows --only a.md,b/c.md; that form leaves only unset and runs both checks over all trees. Use --only=a.md,b/c.md, or add support for the space-separated form.
This follows directly from the parser at Lines 79-80.
Proposed fix
-// `--only a.md,b/c.md` limits both checks to those repo-relative paths, so a
+// `--only=a.md,b/c.md` limits both checks to those repo-relative paths, so a📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // `--only a.md,b/c.md` limits both checks to those repo-relative paths, so a | |
| // `--only=a.md,b/c.md` limits both checks to those repo-relative paths, so a |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check-docs.mjs` at line 77, Update the usage comment near the
argument parser in check-docs.mjs to document the supported equals-sign syntax
as --only=a.md,b/c.md, matching the parser’s handling of the only argument; do
not imply that the space-separated form is supported.
`.harness/docs/git-workflow.md` and `technical-documentation/engineering/release-and-secrets.md` both documented the release flow, and only one of them was ever checked. `check-docs.mjs` walks `technical-documentation/` alone, so when #90 changed the release-branch naming the docs tree was corrected and the harness copy was not — it kept sending readers to `release/vX.Y.Z-rc.N`, a branch the workflows never create, for a month. `.harness/memory/MEMORY.md` repeated it. Both files already declared non-overlapping scopes: git-workflow.md is "conventions for the Mavis reins", release-and-secrets.md is "the operational reference for cutting releases", and the latter already delegated branching and PR procedure back. The release flow sitting in git-workflow.md violated that split. So: move, then collapse. Moved into release-and-secrets.md, none of it duplicated there before — why the build dispatch is explicit and pinned to the tag, the numbered branch contract including "never delete or recreate", the v1.6.0 postmortem with what actually shipped, the runnable manual fallback, backports, and milestone handling. git-workflow.md §Release flow is now four lines and a pointer: 154 lines to 48, in a file loaded into every agent run. The dispatch rationale is documented per workflow rather than in general, because the two do not push their tags the same way: promote.yml uses GITHUB_TOKEN, whose tag push does not fire build.yml's `push:` trigger, while prerelease.yml uses the PAT because a GITHUB_TOKEN tag push is answered with a 500 from a tag ruleset — the failure that took down the v1.8.0-rc.1 cut. Only the `--ref` pinning is required in both cases, for the publish version guard. The fallback there also still said to prepare "the correct `package.json` commit" — the same lockfile trap the harness copy had, in the file operators actually read. It now points at set-release-version.mjs and says why. Root cause last: `check-docs.mjs` walks `.harness/` too, and the retired branch naming is a LEGACY identifier, so CI fails on any doc reintroducing it. `.harness/` passes all three existing passes today, so this adds a gate without a migration. The new test asserts the script *rejects* a planted violation, because a checker that silently stops checking still prints OK — which is how this rotted unnoticed in the first place.
ad496c1 to
98e38c9
Compare
The LEGACY entry added alongside the release-docs consolidation was a literal substring test on `release/vX.Y.Z-rc.N`, so it only caught the placeholder spelling used in prose. The rot it exists to prevent used both: the prose placeholder *and* a concrete `release/v1.5.0-rc.1` inside the copy-pasteable manual-fallback shell block. Restoring that block from git history — exactly what an operator does when the dispatch UI is down — passed the lint clean. The concrete spelling is the more dangerous one, because it is the one someone pastes into a terminal. Replaced with a regex over both forms. It matches the branch shape only, so the RC tags (`v1.6.0-rc.1`, no `release/` prefix) that the v1.6.0 postmortem depends on, the stable `release/vX.Y.Z`, its ephemeral `-sync` branch, and backport lines like `release/1.4.x` all still pass — pinned as a test, since a guard that over-fires is a guard someone deletes. Found by adversarially verifying the parent commit rather than by trusting it.
Duplication resolved, plus the reason it happenedTwo more commits. The first does the consolidation we discussed; the second fixes a hole the verification pass found in the first. The split was already written downNeither file needed a new decision — both already declared non-overlapping scopes, and one of them was violating its own:
So a 110-line release-operations section living in the conventions file was the anomaly. Moved, not deleted — these had no equivalent in
A factual correction along the wayWriting the dispatch rationale forced me to check it, and my first draft was wrong: I said the workflows push their tags with Also closed the gap flagged earlier: Root cause, not just the symptomThe duplication was survivable. What made it rot is that
What the verification caughtI ran an adversarial pass over the consolidation rather than trusting it — four independent lenses (fact loss, dangling references, checker soundness, accuracy against the workflow YAML), each finding refuted by an independent skeptic. 30 agents, one finding survived, and it was right:
The concrete spelling is the more dangerous one, because it's the one someone pastes into a terminal. Replaced with a regex over both forms, matching the branch shape only, so RC tags ( The everything-else findings were refuted — mostly "fact X was dropped" where X survived in the new home under different wording. One was interesting in reverse: an auditor claimed the move kept the wrong variant of the Verification
|
1 participant
Summary
Contributor feedback on
AGENTS.md(#95), plus the doc corrections that the release-pipeline fixes (#90) made necessary — and, from review, the consolidation and the missing guard that let those docs go stale in the first place.AGENTS.md
Deliberately not phrased as "prefer one-liners" (the original suggestion): optimizing for line count pushes models toward dense code that hides control flow.
package-lock.json, not as a style preference. This was the single most misread part of the file.mainafter the cut ships next cycle) plus a pointer to the operational reference.One release reference instead of two
.harness/docs/git-workflow.mdandtechnical-documentation/engineering/release-and-secrets.mdboth documented the release flow. Both already declared non-overlapping scopes — "conventions for the Mavis reins" vs "the operational reference for cutting releases" — and the latter already delegated branching/PR procedure back, so a 110-line release-operations section in the conventions file was the anomaly.Moved into
release-and-secrets.md(none of it duplicated there before): why the build dispatch is explicit and pinned to the tag, the numbered branch contract including "never delete or recreate", the v1.6.0 postmortem with what actually shipped, the runnable manual fallback, backports, milestone handling.git-workflow.md§Release flow is now four lines and a pointer — 154 → 48 lines, in a file loaded into every agent run.Two corrections surfaced while writing it:
promote.ymlusesGITHUB_TOKEN(a tag is a ref, not a file change), which genuinely needs the explicit dispatch.prerelease.ymluses the PAT, because aGITHUB_TOKENtag push is answered withremote: Internal Server Errorby a tag ruleset — the failure that took down thev1.8.0-rc.1cut. Only the--refpinning is required in both cases, for the publish version guard. Documented per workflow rather than generalised.package.jsonwith ased, whileset-release-version.mjs— which both workflows call — also writespackage-lock.json. A release cut by hand therefore shipped a lockfile disagreeing with the package it locks, the exact drift that script exists to stop, andnpm cinever rejects it (the rootversionfield is not a dependency). Fixed inrelease-and-secrets.mdtoo, which is the copy operators actually read.The reason it rotted: an unwatched tree
check-docs.mjswalkedtechnical-documentation/only. So when #90 changed the release-branch naming, the docs tree was corrected and the harness copy was not — for a month it sent readers torelease/vX.Y.Z-rc.N, a branch the workflows never create, and.harness/memory/MEMORY.mdrepeated it.It now walks
.harness/too, and the retired naming is a hard failure..harness/already passes all three existing passes, so this adds a gate with no migration behind it.The guard needed a second pass of its own: as first written it was a literal substring test on the
release/vX.Y.Z-rc.Nplaceholder, so the concreterelease/v1.5.0-rc.1sitting in the copy-pasteable shell block still passed — the more dangerous spelling, because it's the one someone pastes into a terminal. Now a regex over both forms, matching the branch shape only, so RC tags (v1.6.0-rc.1),release/vX.Y.Z,release/vX.Y.Z-syncandrelease/1.4.xall still pass..harness/memory/MEMORY.md
Same class of stale info, flagged in review: it still described the pre-#90 naming.
Related issue
Closes #95.
Type of change
Release impact
Desktop impact
Screenshots / video
n/a — no UI surface.
Testing
node scripts/check-docs.mjs→OK (31 files)(was 22 —.harness/is now in scope).scripts/check-docs.test.mjsasserts the script rejects a planted violation, not merely that it exits 0 — a checker that silently stops checking still printsOK, which is exactly how this went unnoticed. Covers both retired spellings and pins the current naming as a non-match..harness/fails (it passed before), current naming passes,--onlystill filters with the new repo-relative paths,LEGACY_ALLOWEDstill exemptsdecisions.md.main:prerelease.yml:96,promote.yml:71,--refatprerelease.yml:156-157andpromote.yml:124-125, and the credential difference atprerelease.yml:118-139/promote.yml:85-102.node .github/scripts/set-release-version.mjs 1.5.0-rc.1→ 3 lines acrosspackage.jsonandpackage-lock.json, confirming thesedit replaces was under-writing the release commit.ca5124b6; see the review comment.useSequentialTimelineOps.test.ts) fails only under full-suite concurrency in a worktree with a cross-branchnode_modulesjunction; it passes 4/4 in isolation and in CI. This PR changes nothing undersrc/.Rebased onto current
main(the branch was originally cut againstd5966ed5): RC notarization is no longer skipped (6f8e74e), anddocs/secrets.mdis nowtechnical-documentation/engineering/release-and-secrets.md(a6a57ba).🤖 Generated with Claude Code