docs(agents): add development principles, slim release flow, correct branch contract#96
docs(agents): add development principles, slim release flow, correct branch contract#96EtienneLescot wants to merge 1 commit into
Conversation
…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>
📝 WalkthroughWalkthroughChangesRelease guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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
| 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. | ||
| 2. **`promote.yml` is the only writer** that turns `-rc.N` into the stable version on the branch. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clarify that promote.yml is the only normal automated writer.
This absolute wording conflicts with the documented manual fallback at Lines 122-127, where a maintainer directly performs the stable-version bump. Reword it to distinguish the workflow contract from the emergency procedure.
🤖 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 @.harness/docs/git-workflow.md around lines 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.
| git checkout -b release/v1.5.0 main # rc.2+: git checkout release/v1.5.0 instead | ||
| sed -i -E 's|("version"[[:space:]]*:[[:space:]]*")[^"]*(")|\11.5.0-rc.1\2|' package.json | ||
| git add package.json && git commit -m "chore(release): bump to 1.5.0-rc.1 [skip ci]" | ||
| git push origin release/v1.5.0-rc.1 | ||
| git push origin v1.5.0-rc.1 | ||
| git push origin release/v1.5.0 | ||
| git tag v1.5.0-rc.1 && git push origin v1.5.0-rc.1 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the RC fallback work for rc.2+.
The comment advertises rc.2 support, but the commands still hardcode 1.5.0-rc.1 and v1.5.0-rc.1. A subsequent RC would overwrite/recreate the wrong tag instead of publishing rc.N.
Proposed fix
-# Cut RC (skips milestone migration and Discord announce)
-git checkout -b release/v1.5.0 main # rc.2+: git checkout release/v1.5.0 instead
-sed -i -E 's|("version"[[:space:]]*:[[:space:]]*")[^"]*(")|\11.5.0-rc.1\2|' package.json
-git add package.json && git commit -m "chore(release): bump to 1.5.0-rc.1 [skip ci]"
+RC_NUMBER=1
+git checkout -b release/v1.5.0 main # rc.2+: git checkout release/v1.5.0 instead
+sed -i -E "s|(\"version\"[[:space:]]*:[[:space:]]*\")[^\"]*(\")|\11.5.0-rc.${RC_NUMBER}\2|" package.json
+git add package.json && git commit -m "chore(release): bump to 1.5.0-rc.${RC_NUMBER} [skip ci]"
git push origin release/v1.5.0
-git tag v1.5.0-rc.1 && git push origin v1.5.0-rc.1
+git tag "v1.5.0-rc.${RC_NUMBER}" && git push origin "v1.5.0-rc.${RC_NUMBER}"🤖 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 @.harness/docs/git-workflow.md around lines 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.
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.
|
|
||
| **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.
| - **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.
| 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.
|
|
||
| ``` | ||
| 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.
1 participant
Closes #95.
Contributor feedback on
AGENTS.md, plus doc corrections that the release-pipeline fixes (#90) made necessary.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. The reference repo's own Maintainability section argues the same, for structure over fewer lines.
.harness/docs/git-workflow.md, which already documents all of it. This was duplicated context loaded into every agent run.package-lock.json, not as a style preference. This was the single most misread part of the file.Net the file is +42 words: Release flow lost ~119, the principles section and npm rationale add ~144.
.harness/docs/git-workflow.md — corrected to match the shipped pipeline
The doc described the naming bug that #90 fixed, so it was actively misleading:
release/vX.Y.Z(one per stable version, created at rc.1 and reused), notrelease/vX.Y.Z-rc.N. The old naming is precisely what would have broken promote, which resolvesrelease/v${STABLE_VERSION}. The contract block now states the branch must never be deleted/recreated, since that drops cherry-picks and silently re-cuts frommain.build.yml.prerelease.ymldispatches it explicitly with--refpinned to the tag, and the doc now says why (aGITHUB_TOKENtag push doesn't firepush:, and the build must check out the tag or it fails the version guard).git tagcommands — it pushed tags it never created.Verification
Naming now matches what actually shipped:
release/v1.7.0exists and bothv1.7.0-rc.1/v1.7.0-rc.2were cut from it, with rc.2 publishing end-to-end with no manual intervention.🤖 Generated with Claude Code
Summary by CodeRabbit