Files
msd-core/CONTRIBUTING.md
Tom Boucher 1c1af70a4b refactor(#2724): delete the committed golden fixtures and size baselines (#2767)
* test(#2724): delete golden-install-parity fixtures, test, and generator

Removes the 19 committed path->hash manifests, the two per-file size
baselines, tests/golden-install-parity.test.cjs, and
scripts/gen-golden-install-parity-zcode.cjs. These were pure functions
of the source tree (ADR-2719); the differential attribution check
(tests/emitted-attribution.test.cjs + tests/emitted-provenance.test.cjs)
is now the sole gate for emitted-artifact propagation.

tests/fixtures/install-tree/*.json and tests/golden-install-tree.test.cjs
are unchanged (ADR-2719 section 7 exception).

Follow-up commits fix the resulting bookkeeping: scripts/ci-test-scope.cjs's
existence guard, .gitattributes, package.json scripts, the emitted-provenance
totality guard's IO, the differential check's baseline acquisition, CI
wiring to publish/restore the baseline artifact, and docs.

* refactor(#2724): make the differential attribution check self-sufficient

Three fixes required to delete the golden fixtures without breaking CI:

- scripts/ci-test-scope.cjs: remove tests/golden-install-parity.test.cjs
  from the three rules that named it. #2759's missingRuleTestFiles guard
  hard-throws at module load if a rule names a test file absent from
  disk, which would break the changes job on every PR the moment the
  fixture-deletion commit landed.

- tests/helpers/emitted-provenance.cjs: loadManifests() read the
  committed golden fixture directory. With that directory deleted at
  every future ref, this would throw at module load forever, taking
  the Phase 2 totality guard down with it. Rebuilt from real installer
  spawns (MANIFEST_FAMILIES + runMinimalInstall + buildParityManifest),
  the same shape emitted-runtime.cjs's currentManifests() already uses.

- tests/emitted-attribution.test.cjs / tests/helpers/emitted-runtime.cjs:
  the real-tree test's baseline acquisition swaps from
  baselineManifestsAtRef(base) (git show at a ref that no longer carries
  fixtures) to resolveBaseline()'s documented precedence: env, then the
  on-disk cache, then an in-job build. The build fallback
  (buildBaselineAtRef, new) checks out base into a throwaway git
  worktree and runs the new scripts/gen-emitted-baseline.cjs there --
  no npm ci needed, since bin/install.js and the test helper shells are
  Node-builtins-only. That script also publishes the baseline artifact
  from CI's push-to-next job (wired in a follow-up commit).

* refactor(#2724): retire the merge-driver bridge and per-file size baselines

The Phase 1 bridge (#2721) is retired now that the artifacts it guarded
are deleted: scripts/git-merge-regen-driver.cjs, its test, and the
'setup:merge-driver' npm script are removed, and the .gitattributes
merge=gsd-regen/linguist-generated block for the three deleted-path
globs is dropped. tests/fixtures/install-tree/*.json keeps its normal
merge behavior, unchanged (ADR-2719 section 7).

scripts/update-size-baseline.cjs and its test are removed: their sole
purpose was regenerating tests/workflow-size-baseline.json and
tests/agent-size-baseline.json, both deleted. The 'size:baseline' npm
script and its step in 'regen:derived' go with it. The per-file
baseline describe blocks in tests/workflow-size-budget.test.cjs and
tests/agent-size-budget.test.cjs are removed for the same reason; the
independent loose-tier hard caps are untouched. The differential
attribution check's size ratchet (tests/emitted-diff.cjs, already
shipped in #2723) is the replacement anti-creep mechanism.

'npm run gen:golden' is replaced by 'npm run gen:install-tree', which
keeps regenerating tests/fixtures/install-tree/*.json (the one artifact
family ADR-2719 section 7 keeps committed); tests/golden-install-tree.test.cjs's
error messages point at the new command name.

tests/golden-parity-single-source.test.cjs's anti-divergence guard
(#2266) is retargeted from the two deleted golden-parity consumers to
their two replacements (tests/helpers/emitted-runtime.cjs and
tests/helpers/emitted-provenance.cjs), which import buildParityManifest
the same way — the divergence risk the guard exists for is unchanged.

Also wires CI: a new publish-emitted-baseline job runs
scripts/gen-emitted-baseline.cjs after a push to next and caches the
result keyed on the sha; the test and test-full jobs restore that cache
on pull_request events, keyed on the PR's base sha, and export
GSD_EMITTED_BASELINE for tests/emitted-attribution.test.cjs's real-tree
test to pick up.

* docs(#2724): flip ADR-2719 to Accepted and update contributor docs

Status: Proposed -> Accepted. Regenerated docs/adr/README.md index.

CONTRIBUTING.md, docs/TESTING-SUITES.md, and CONTEXT.md (RULESET.
EMITTED_ATTRIBUTION, RULESET.WORKFLOW_SIZE_BUDGET, RULESET.
AGENT_SIZE_BUDGET, and the Emitted Artifact Provenance glossary entry)
no longer point at the deleted golden-install-parity fixtures, size
baselines, gen:golden, UPDATE_GOLDEN, or the setup:merge-driver /
git-merge-regen-driver.cjs bridge. Editing shipped content now
requires zero manual fixture regeneration, documented against the
differential attribution check instead of the deleted commands.

* docs(#2724): add changeset for removed golden-parity commands

* fix(#2724): drop stale scripts/update-size-baseline.cjs glossary ref

check-glossary-refs.cjs verifies every backtick-wrapped scripts/*.cjs
token in CONTEXT.md resolves to a real file. The RULESET.
EMITTED_ATTRIBUTION rewrite named the deleted script inside backticks,
which the checker reads as a live reference, not historical prose.

* test(#2724): retarget ci-test-scope tests off the deleted golden test

tests/ci-test-scope.test.cjs asserted specific RULES entries select
tests/golden-install-parity.test.cjs, and that every rule selecting it
also selects both emitted gates. Both premises broke when the golden
test was deleted (#2724): the deleted filename never re-appears in
targeted_tests, and there was no longer a third file for the gates to
travel alongside. Retargeted the two selection describe blocks to
assert tests/emitted-provenance.test.cjs directly (the drift guard the
golden gate's rules were retargeted to), and simplified the third block
to assert the two emitted gates always travel together, without
reference to the golden filename.

* docs(#2724): repoint two contributor how-to guides at the differential check

Both guides told contributors to regenerate a baseline against
tests/golden-install-parity.test.cjs, which #2724 deletes. Repointed
at the differential attribution check (tests/emitted-attribution.test.cjs,
ADR-2719), which needs no manual regeneration step.

* fix(#2724): repair phase6-capstone-conformance's deleted-baseline read

An independent orthogonal review caught a real regression this branch
introduced into a test file the branch's diff never touched:
tests/phase6-capstone-conformance.test.cjs read
tests/workflow-size-baseline.json (deleted earlier in this branch) with
no fallback, so the whole suite would throw ENOENT the moment this
branch landed. The test's actual intent — prove the host-loop workflow
files are real, tracked, non-empty docs — is preserved by asserting the
live byte count via the same shared counter (scripts/workflow-size.cjs)
the size guards already use, instead of a committed snapshot.

Also, from the same review: a stale doc comment in
scripts/workflow-size.cjs still named the deleted
scripts/update-size-baseline.cjs as a consumer, and
buildBaselineAtRef's cleanup in tests/helpers/emitted-runtime.cjs left
two fs.rmSync calls unguarded against masking the primary result/error,
inconsistent with the try/catch already wrapping the git cleanup beside
them. Both fixed. A doc comment was added to baselineFamilyNamesAtRef
explaining why it (and its siblings) are kept despite having no
production caller post-cutover — they still answer real questions
about refs that predate the cutover.

* fix(#2724): repair three real regressions found by remote verification

1. tests/emitted-provenance.test.cjs's two hostile-input tests
   (non-object manifest, unreadable fixture) drove loadManifests(tmp)
   and monkeypatched fs.readFileSync, both premised on the deleted
   fixture-directory read this branch already replaced with real
   installer spawns -- the negative assertions silently stopped firing.
   loadManifests() now accepts injected {families, install, build,
   clean} (defaulting to production values), giving the tests a real
   seam to drive a bad build result and a build failure through the
   ACTUAL loader instead of a reimplementation, and added coverage that
   clean() still runs on both paths.

2. .github/workflows/test.yml's two 'Export GSD_EMITTED_BASELINE'
   steps hardcoded shell: bash, which is wrong on windows-latest (native
   pwsh) and on test-full's macos-latest legs (native zsh per that job's
   own matrix) -- the repo's H1 shell policy (tests/policy-shell-pinning
   .test.cjs) caught it. Replaced the inline bash script with
   scripts/ci-export-emitted-baseline-env.cjs, a plain Node script: a
   bare 'node <path>' command line has no shell-specific syntax, so it
   runs correctly under bash, zsh, and pwsh without a shell override.

tests/phase6-capstone-conformance.test.cjs's deleted-baseline read
(caught by the same remote run, at a commit prior to this one) was
already fixed in d0c3b1242 and is not touched here; verified still
passing after these changes.

* fix(#2724): revive ADR-1610's new-file size cap inside the differential

An isolated review caught a real regression: deleting
tests/workflow-size-baseline.json silently dropped NEW_FILE_CAP
(ADR-1610 Decision point 3, the Codex project_doc_max_bytes anchor)
with no successor. tests/helpers/emitted-diff.cjs's size ratchet
already 'continue's past any file absent from sizeBaseline -- exactly
the files this cap exists to bound -- so a brand-new workflow file
sized 32,769-40,960 bytes passed CI clean and shipped, then risked
silent truncation at the Codex anchor at runtime. ADR-1610 is Accepted
and never referenced anywhere in this branch.

Fix: NEW_FILE_CAP=32768 revived inside emitted-diff.cjs's own
size-ratchet loop, keyed off the SAME hasOwnProperty(sizeBaseline,
name) signal the growth check already computes -- 'new' is exactly
'present in sizeCurrent, absent from sizeBaseline'. Not ack-able,
matching the tier hard caps it sits beside: the fix is extraction, not
an acknowledgment entry. Documented, disclosed narrowing: the pure
differential module cannot see XL_WORKFLOWS/LARGE_WORKFLOWS tiering
(tests/workflow-size-budget.test.cjs's classification), so a
legitimately large new file must extract rather than tier in, one
release earlier than an existing file would need to. ADR-1610 itself is
left unamended -- this restores its decision rather than re-litigating
it.

Also fixes a stale comment plus a redundant real 19-installer-spawn
assertion left over from the pre-injection-seam version of
tests/emitted-provenance.test.cjs's build-failure test, and annotates
3 of 4 stale golden-fixture citations in
docs/reference/host-integration-capability-matrix.md as superseded
(the 4th is an accurate historical PR narrative, left alone).

* fix(#2724): repair three red CI defects on the golden-fixture cutover

Windows-only provenance false attribution (defect A): the `hooks-built`
provenance rule attributed `hooks/<name>.cmd` to itself. Those shims are
Windows-only installer output (ensureCodexHooksJsonSessionStart /
ensureCodexHooksJsonEvent, both in src/runtime-hooks-surface.cts) wrapping
the same-named `.js` hook — no `.cmd` file is ever tracked in the repo, so
the self-attribution resolved to a path that exists on no platform. Only
windows-latest ever emits the key, so this only failed there. Fixed by
special-casing `.cmd` inside the SAME `hooks-built` rule (not a dedicated
rule) — a dedicated rule would match zero paths, and therefore report as a
dead rule, on every non-Windows lane of the same totality guard. `sources`
already supported per-match functions; `transforms` is extended to support
the same shape so the attribution can vary by match within one rule.

Baseline bootstrap was structurally impossible (defect B): `buildBaselineAtRef`
ran `scripts/gen-emitted-baseline.cjs` from INSIDE the base-ref worktree, but
that script is new in this PR and therefore absent at any base ref that
predates it — every call failed closed with "Cannot find module". Fixed by
running the PR checkout's own generator against the worktree via a new `--dir`
parameter, decoupling "which copy of the script runs" from "which tree it
measures" (`currentManifests`/`currentSizes` gained a `repoRoot` override,
threaded down to `runMinimalInstall`'s new `installScript` override). This is
not just a bootstrap fix: a differential needs ONE measurement schema applied
to both sides, or the two stop being comparable the moment that schema
evolves — running each side's own copy would silently reintroduce that risk.
Verified locally end-to-end against real origin/next: resolves a valid
{version, sha, manifests, sizes} artifact with the correct sha and no leaked
worktree.

Changeset placeholder (defect C): `pr: 0` -> `pr: 2767`, which is what let
docs-lint evaluate the fragment for the first time; it already passes
(docs/TESTING-SUITES.md and friends already document the removed scripts).

Also fixed while in this file: an eslint no-unused-vars warning surfaced by
the changed lint run (unused `cleanup` import in
tests/emitted-provenance.test.cjs).

Added regression coverage for both A and B: a cross-platform spot-check that
drives the real hooks-built rule against `.cmd` keys directly (not through a
real Windows install), and a real-tree test that drives buildBaselineAtRef
against a base ref verified (via git cat-file) to lack the generator, both
skipping honestly rather than false-passing when their precondition does not
hold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

* fix(#2724): repair false .cmd byte-provenance and a permanently-skipping regression test

Two isolated-review findings on PR #2767:

- `hooks-built`'s `.cmd` branch attributed the Windows shim's bytes to the
  wrapped `hooks/<name>.js` script, asserting a byte-provenance link that
  does not exist — traced against buildCodexHookWindowsShimIR
  (src/runtime-hooks-surface.cts), only the script's NAME (a literal in that
  same file) flows into the .cmd bytes, never its content. Point `sources`
  at HOOKS_WINDOWS_SHIM_SRC instead, matching the code-derived convention
  used elsewhere in the table. Since `sources` is checked before
  `transforms` in the differential, the wrong mapping silently excused any
  .cmd byte movement caused by editing the wrapped .js file.

- The `buildBaselineAtRef` regression test skipped unless a resolvable base
  ref still lacked scripts/gen-emitted-baseline.cjs — true only until this
  PR merges, after which every base ref carries the file and the test skips
  forever with zero ongoing coverage. Rebuilt hermetically: synthesize the
  missing-generator condition in-place via git plumbing (a throwaway commit,
  child of HEAD, with just that one file removed from a scratch index),
  never touching the real working tree, HEAD, or index, and never depending
  on ambient history or remotes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

* fix(#2724): tolerate the remote runner's dubious-ownership git mount in the emitted baseline path

The runner container mounts the repo at a path owned by a different uid than
the process running the suite, so git's dubious-ownership protection refuses
every git operation there. GitHub Actions never hits this because
actions/checkout registers the workspace as safe automatically; this
runner's container does not.

buildBaselineAtRef is the production build-fallback the sole remaining
emitted gate depends on (resolveBaseline's in-job-build leg), not just a
test helper, so the fix is in the shared git() wrapper (emitted-runtime.cjs)
that every caller — resolveChangedPaths, resolveBase, buildBaselineAtRef's
worktree add/remove/prune, and the hermetic regression test added in the
prior commit — funnels through, plus gen-emitted-baseline.cjs's own
rev-parse (now reusing that same wrapper instead of a second execFileSync,
so the fix has one source of truth). Each call declares -c
safe.directory=<the exact directory it already operates on>, never the *
wildcard.

Audited every other helper on this surface (emitted-diff.cjs,
emitted-baseline.cjs, install-shared.cjs) for the same gap: none of them
shell out to git at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 15:41:43 -04:00

54 KiB

Contributing to GSD Core

Getting Started

# Clone the repo
git clone https://github.com/open-gsd/gsd-core.git
cd gsd-core

# Activate the pinned Node version from .nvmrc
nvm use

# Validate your environment
npm run check:env

# Install dependencies (reproducible, lockfile-driven)
npm ci

# Run tests
npm test

npm ci is required over npm install. It installs exactly what package-lock.json specifies and fails fast if the lockfile is out of sync — this is intentional.

docs/contributing/bootstrap.md is the source of truth for setup. See it for Node version managers other than nvm (fnm, asdf, mise), the environment validator, daily commands, and troubleshooting.


Types of Contributions

GSD accepts three types of contributions. Each type has a different process and a different bar for acceptance. Read this section before opening anything.

🐛 Fix (Bug Report)

A fix corrects something that is broken, crashes, produces wrong output, or behaves contrary to documented behavior.

Process:

  1. Open a Bug Report issue — fill it out completely.
  2. Wait for a maintainer to confirm it is a bug (label: confirmed-bug). For obvious, reproducible bugs this is typically fast.
  3. Fix it. Write a test that would have caught the bug.
  4. Open a PR using the Fix PR template — link the confirmed issue.

Rejection reasons: Not reproducible, works-as-designed, duplicate of an existing issue.


⚡ Enhancement

An enhancement improves an existing feature — better output, faster execution, cleaner UX, expanded edge-case handling. It does not add new commands, new workflows, or new concepts.

The bar: Enhancements must have a scoped written proposal approved by a maintainer before any code is written. A PR for an enhancement will be closed without review if the linked issue does not carry the approved-enhancement label.

Process:

  1. Open an Enhancement issue with the full proposal. The issue template requires: the problem being solved, the concrete benefit, the scope of changes, and alternatives considered.
  2. Wait for maintainer approval. A maintainer must label the issue approved-enhancement before you write a single line of code. Do not open a PR against an unapproved enhancement issue — it will be closed.
  3. Write the code. Keep the scope exactly as approved. If scope creep occurs, comment on the issue and get re-approval before continuing.
  4. Open a PR using the Enhancement PR template — link the approved issue.

Rejection reasons: Issue not labeled approved-enhancement, scope exceeds what was approved, no written proposal, duplicate of existing behavior.


✨ Feature

A feature adds something new — a new command, a new workflow, a new concept, a new integration. Features have the highest bar because they add permanent maintenance burden to a solo-developer tool maintained by a small team.

The bar: Features require a complete written specification approved by a maintainer before any code is written. A PR for a feature will be closed without review if the linked issue does not carry the approved-feature label. Incomplete specs are closed, not revised by maintainers.

Process:

  1. Discuss first — check Discussions to see if the idea has been raised. If it has and was declined, don't open a new issue.
  2. Open a Feature Request issue with the complete spec. The template requires: the solo-developer problem being solved, what is being added, full scope of affected files and systems, user stories, acceptance criteria, and assessment of maintenance burden.
  3. Wait for maintainer approval. A maintainer must label the issue approved-feature before you write a single line of code. Approval is not guaranteed — GSD is intentionally lean and many valid ideas are declined because they conflict with the project's design philosophy.
  4. Write the code. Implement exactly the approved spec. Changes to scope require re-approval.
  5. Open a PR using the Feature PR template — link the approved issue.

Rejection reasons: Issue not labeled approved-feature, spec is incomplete, scope exceeds what was approved, feature conflicts with GSD's solo-developer focus, maintenance burden too high.


📐 Proposing an ADR or PRD

An ADR (Architecture Decision Record) documents a significant architectural decision. A PRD (Product Requirements Document) captures the what and why of a feature before implementation. Both are governed by the same issue-first rule as everything else.

Process:

  1. Open an issue of the appropriate type (enhancement for an ADR revisiting an existing area, feature for a new architectural surface, chore for policy/docs decisions). Fill it out completely.
  2. Wait for maintainer approval. A maintainer must label the issue approved-enhancement, approved-feature, or confirm the chore before any file is created.
  3. The GitHub-assigned issue number becomes your filename prefix. Create the file on a branch named after the issue:
    • docs/adr/<issue#>-<slug>.md for ADRs
    • docs/prd/<issue#>-<slug>.md for PRDs
    • Branch: docs/<issue#>-<slug>
  4. Open a PR using the appropriate template and close the issue with Closes #<issue#> in the PR body.

One issue = one ADR-or-PRD = one PR. Do not batch multiple decisions into one file or one PR.

Do not compute a "next number" locally. Any PR that uses the legacy NNNN-* sequential pattern for a new ADR or PRD will be asked to rename the file to the <issue#>-<slug>.md format before merge.

Example: Issue #3485 was opened, approved, and its number became the prefix: docs/adr/3485-adr-prd-naming-convention.md on branch docs/3485-adr-prd-naming-convention.

Rejection reasons: Issue not approved before file was created, filename uses local-compute sequential number instead of issue#, multiple decisions bundled in one PR, file placed in wrong directory (docs/adr/ vs docs/prd/).


The Issue-First Rule — No Exceptions

No code before approval.

For fixes: open the issue, confirm it's a bug, then fix it. For enhancements: open the issue, get approved-enhancement, then code. For features: open the issue, get approved-feature, then code.

PRs that arrive without a properly-labeled linked issue are closed automatically. This is not a bureaucratic hurdle — it protects you from spending time on work that will be rejected, and it protects maintainers from reviewing code for changes that were never agreed to.


Where Do I Open My PR? (Branching Model)

GSD uses two long-lived branches: main (production, what's on npm @latest) and next (integration for the upcoming release). Almost every PR targets next. Full guide: docs/branching.md.

Your branch PR target Notes
feat/NNN-slug next Default for all new features
fix/NNN-slug next Default for all bug fixes; ships in next minor or via hotfix cherry-pick
chore/, docs/, refactor/, test/, perf/, ci/, revert/ next All routine work
fix/critical-NNN-slug main Production-down emergencies only; auto-back-merges to next
release/X.Y.0 main Created by release.yml — don't make these by hand
hotfix/X.Y.Z main Created by release.yml (dispatch with a patch version X.Y.Z) — don't make these by hand
Stabilization PR for an in-flight release release/X.Y.0 Fix a regression found during the RC cycle

Day-to-day commands:

git fetch origin
git checkout next
git pull --ff-only origin next
git checkout -b fix/3187-config-corruption
# ... commit, push
gh pr create --base next --repo open-gsd/gsd-core

If you target the wrong branch by accident, the PR Target Validator workflow will post a comment with the one-line fix (click "Edit" by the PR title and change the base branch — no need to recreate the PR).

Why this matters: Under the old single-branch model, every PR required rebasing onto main because branch protection required "up-to-date before merging" and main moved on every merge. With next as the integration branch and that flag disabled on next, concurrent PRs can merge in any order as long as they don't conflict on the same lines. The rebase treadmill is gone for the 95% case.


Pull Request Guidelines

Architecture & Domain Standards (Maintainer-Defined)

The following files are maintainer-owned coding standards and must be treated as canonical when contributing:

  • CONTEXT.md — domain language and module naming standards
  • docs/adr/ — Architecture Decision Records (ADRs) for accepted architectural decisions

Full contributor requirements — including CONTEXT.md format, ADR governance, and AI-agent-assisted work standards — are in docs/contributor-standards.md.

Contributor requirements (summary):

  • Read CONTEXT.md before naming or refactoring modules/interfaces/seams.
  • Use CONTEXT.md vocabulary consistently in code comments, tests, issue/PR text, and docs for the touched area.
  • Check relevant ADRs in docs/adr/ before proposing or implementing architectural changes.
  • If a change intentionally revisits an ADR decision, call it out explicitly in the linked issue and PR rationale.
  • Do not rewrite maintainer intent in CONTEXT.md/ADRs as part of drive-by cleanup; propose focused updates tied to approved scope.
  • If using an AI assistant, prompt it to read CONTEXT.md and the relevant ADRs before writing any code or docs, and verify it used the correct vocabulary before opening the PR.

Every PR must link to an approved issue. PRs without a linked issue are closed without review, no exceptions.

  • No draft PRs — draft PRs are automatically closed. Only open a PR when it is complete, tested, and ready for review. If your work is not finished, keep it on your local branch until it is.
  • Use the correct PR template — there are separate templates for Fix, Enhancement, and Feature. Using the wrong template or using the default template for a feature is a rejection reason.
  • Link with a closing keyword — use Closes #123, Fixes #123, or Resolves #123 in the PR body. The CI check will fail and the PR will be auto-closed if no valid issue reference is found.
  • One concern per PR — bug fixes, enhancements, and features must be separate PRs
  • No drive-by formatting — don't reformat code unrelated to your change
  • Don't bundle test-fixture updates into docs: or unrelated commits — when a production change makes an existing test assertion stale, the test correction MUST land as its own test: (or fix:) commit, not bundled into a docs: commit that also updates the explanation. The release-sdk hotfix cherry-pick filter routes by commit-subject prefix (fix:, chore:, test:); a test-fixture correction packed under a docs: prefix is invisible to the picker and ships a half-state to the hotfix branch — production code changed, test assertion stale. v1.42.3 hit this exact mode (#3621). The fix is upstream: keep the test-fixture commit separate.
  • CI must pass — all configured matrix jobs must be green. Node 22 remains the compatibility floor; Node 24 is the primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available.
  • Scope matches the approved issue — if your PR does more than what the issue describes, the extra changes will be asked to be removed or moved to a new issue

CHANGELOG Entries — Drop a Fragment

Do not edit CHANGELOG.md directly. Two PRs that both append to a ### Fixed block always conflict on merge — git can't pick a serialization order without a human. Instead, every PR with user-facing changes drops a fragment file in .changeset/.

npm run changeset -- --type Fixed --pr <YOUR_PR_NUMBER> \
  --body "**\`/gsd-foo\` no longer drops trailing slashes** — explain the user-visible change."

This writes .changeset/<adjective>-<noun>-<noun>.md. Three random words → concurrent PRs never collide. Allowed type: values follow Keep a Changelog: Added, Changed, Deprecated, Removed, Fixed, Security.

Fragments are consolidated into CHANGELOG.md at release time by the release workflow. See .changeset/README.md for the format spec and #2975 for the rationale.

CI enforcement: the Changeset Required workflow (scripts/changeset/lint.cjs) fails any PR that touches bin/, gsd-core/, src/, agents/, commands/, hooks/, or sdk/src/ without a .changeset/*.md fragment. (src/ is the TypeScript source of truth compiled into gsd-core/bin/lib/*.cjs, so editing it is a user-facing change even though the generated .cjs is gitignored and never appears in the diff.)

Running it locally. The lint derives its changed-file set from GITHUB_BASE_REF, which only CI sets. node scripts/changeset/lint.cjs on a developer machine therefore does not evaluate your branch and can report success on a PR that CI will fail. Pass the base explicitly to reproduce the CI result:

GITHUB_BASE_REF=next node scripts/changeset/lint.cjs
``` The gate also **validates the content** of every changed fragment: a fragment whose frontmatter does not parse (e.g. a `pr: 0` placeholder that was never backfilled to the real PR number) fails the gate with `fail_invalid_fragment`, naming the offending file. This stops a malformed fragment from merging to `next` and only detonating later in the release job's CHANGELOG render.

Opt-out: PRs with no user-facing impact (test refactors, lint config changes, CI tweaks, formatting-only changes) can add the no-changelog label. The lint honors it. When unsure whether a change is user-facing, add the fragment.

Release notes formatting

GitHub release notes are generated automatically. The release and hotfix workflows first create the release with gh release create --generate-notes, then run scripts/release-notes/format-github-release-notes.cjs --apply to rewrite the body into the project's curated format: an Install block, followed by What's Changed grouped into Feature / Enhancement / Fix sections (classified by each PR's conventional-commit title prefix — feat → Feature, fix → Fix, everything else → Enhancement), then New Contributors and the Full Changelog link.

To re-format an existing release by hand (e.g. backfilling an older release):

node scripts/release-notes/format-github-release-notes.cjs \
  --tag vX.Y.Z --repo open-gsd/gsd-core --apply

Omit --apply to print the reformatted body to stdout for review without publishing.

PR title convention (enforced at open time)

Because the changelog is built from PR titles, your PR title must follow:

type(#<issue>): short summary
  • Start with the type — feat, fix, or any other conventional type (chore, docs, refactor, …). No leading tags or prefixes: a title like [security] fix(config): … defeats the ^fix bucket anchor and silently files the entry under the wrong changelog section.
  • Put the linked issue ref in the scope — (#<digits>). This is what renders as a link to the issue in the changelog line. fix(core): … buckets correctly but produces a changelog entry with no issue link.
  • A breaking-change marker is fine: feat(#42)!: ….

Examples: fix(#1542): roadmap rollback, feat(#39): milestone-prefixed phase IDs, enhance(#1549): add PR-title validator.

CI enforcement: pr-title-validator.yml checks the title on open/edit and fails with the required format if it doesn't conform. It reuses the same matcher the changelog classifier uses (scripts/release-notes/conventional-title.cjs), so a title that passes the check is guaranteed to bucket and link correctly. Fix a flagged title by editing it in place — the check re-runs on edit, no need to recreate the PR.

Documentation Updates — Update the Relevant Docs

If your PR adds, changes, deprecates, or removes user-visible behavior, you must update the relevant documentation in docs/. CI will fail any PR whose changeset fragment is typed Added, Changed, Deprecated, or Removed without also modifying at least one file under docs/ (#3213).

Fixed and Security fragments do not trigger this lint — bug fixes restore documented behavior, they do not introduce new behavior to document. (Edit the docs anyway if a fix corrects something the docs got wrong.)

Which docs to update

Change type Required doc updates
New command or flag docs/COMMANDS.md, docs/FEATURES.md
Changed command behavior or output docs/USER-GUIDE.md, docs/COMMANDS.md
Configuration / schema change docs/CONFIGURATION.md
Architectural change docs/ARCHITECTURE.md, docs/adr/
Agent or skill change docs/AGENTS.md
Removed command, flag, or workflow All docs that referenced it

Language policy

All content in docs/ and the root README.md must be written in English. English is the canonical source. The translated READMEs (README.pt-BR.md, README.zh-CN.md, README.ja-JP.md, README.ko-KR.md) are community-maintained translations and do not need to be updated by every PR.

CI enforcement

The Docs Required workflow (scripts/lint-docs-required.cjs) reads the changeset fragments touched in the PR diff. If any has type Added / Changed / Deprecated / Removed, it requires at least one file under docs/ to also appear in the diff.

Opt-outs (with paper trail)

When a change genuinely has no user-facing documentation impact (infrastructure rewrite, internal refactor, test-only addition, CI fix), use one of:

  • Label: add the no-docs label to the PR. Leave a comment explaining why no docs update was needed.
  • Per-fragment marker: add <!-- docs-exempt: <reason> --> on its own line inside the body of each triggering changeset fragment (typically at the end). The reason is required and must be non-empty — a bare <!-- docs-exempt --> or <!-- docs-exempt: --> is rejected (no audit trail = no exemption). The marker is extracted at parse time by scripts/changeset/parse.cjs and stripped from the body before the CHANGELOG.md and GitHub release-notes serializers see it — it leaves a paper trail in the source fragment without leaking into published release notes. Inline mentions of the marker syntax (e.g. inside backticks) are intentionally ignored; the parser only acts on a marker that occupies its own line. Both routes leave a paper trail; the label is global, the marker is per-fragment for mixed PRs.

When unsure whether a change is user-facing, update the docs.

Testing Standards

All tests use Node.js built-in test runner (node:test) and assertion library (node:assert). Do not use Jest, Mocha, Chai, or any external test framework.

Suite grouping. Tests live in named suites (unit, integration, install, security, slow) selected by filename suffix: a file named foo.security.test.cjs belongs to the security suite; a file with no suffix (foo.test.cjs) belongs to unit. See docs/TESTING-SUITES.md for the full policy, CI matrix, and per-suite scripts (npm run test:unit, npm run test:security, npm run test:coverage:unit, …). Default npm test still runs every test — backwards compatible.

Required Imports

const { describe, it, test, beforeEach, afterEach, before, after, mock } = require('node:test');
const assert = require('node:assert/strict');

Setup and Cleanup

There are two approved cleanup patterns. Choose the one that fits the situation.

Pattern 1 — Shared fixtures (beforeEach/afterEach): Use when all tests in a describe block share identical setup and teardown. This is the most common case.

// GOOD — shared setup/teardown with hooks
describe('my feature', () => {
  let tmpDir;

  beforeEach(() => {
    tmpDir = createTempProject();
  });

  afterEach(() => {
    cleanup(tmpDir);
  });

  test('does the thing', () => {
    assert.strictEqual(result, expected);
  });
});

Pattern 2 — Per-test cleanup (t.after()): Use when individual tests require unique teardown that differs from other tests in the same block.

// GOOD — per-test cleanup when each test needs different teardown
test('does the thing with a custom setup', (t) => {
  const tmpDir = createTempProject('custom-prefix');
  t.after(() => cleanup(tmpDir));

  assert.strictEqual(result, expected);
});

Never use try/finally inside test bodies. It is verbose, masks test failures, and is not an approved pattern in this project.

// BAD — try/finally inside a test body
test('does the thing', () => {
  const tmpDir = createTempProject();
  try {
    assert.strictEqual(result, expected);
  } finally {
    cleanup(tmpDir); // masks failures — don't do this
  }
});

try/finally is only permitted inside standalone utility or helper functions that have no access to test context.

Use Centralized Test Helpers

Import helpers from tests/helpers.cjs instead of inlining temp directory creation:

const { createTempProject, createTempGitProject, createTempDir, cleanup, runGsdTools } = require('./helpers.cjs');
Helper Creates Use When
createTempProject(prefix?) tmpDir with .planning/phases/ Testing GSD tools that need planning structure
createTempGitProject(prefix?) Same + git init + initial commit Testing git-dependent features
createTempDir(prefix?) Bare temp directory Testing features that don't need .planning/
cleanup(tmpDir) Removes directory recursively Always use in afterEach
runGsdTools(args, cwd, env?) Executes gsd-tools.cjs Testing CLI commands

Test Structure

describe('featureName', () => {
  let tmpDir;

  beforeEach(() => {
    tmpDir = createTempProject();
    // Additional setup specific to this suite
  });

  afterEach(() => {
    cleanup(tmpDir);
  });

  test('handles normal case', () => {
    // Arrange
    // Act
    // Assert
  });

  test('handles edge case', () => {
    // ...
  });

  describe('sub-feature', () => {
    // Nested describes can have their own hooks
    beforeEach(() => {
      // Additional setup for sub-feature
    });

    test('sub-feature works', () => {
      // ...
    });
  });
});

Fixture Data Formatting

Template literals inside test blocks inherit indentation from the surrounding code. This can introduce unexpected leading whitespace that breaks regex anchors and string matching. Construct multi-line fixture strings using array join() instead:

// GOOD — no indentation bleed
const content = [
  'line one',
  'line two',
  'line three',
].join('\n');

// BAD — template literal inherits surrounding indentation
const content = `
  line one
  line two
  line three
`;

QA Matrix Requirements

Happy-path tests are not enough for code that accepts user input, reads project files, writes to disk, shells out, generates artifacts, or builds prompts. New tests for those areas must include adversarial inputs and negative proof that unsafe behavior did not happen.

See TEST-EXAMPLES.md for concrete demo tests that show these requirements in practice.

Use this matrix when it applies to the changed surface:

  1. Happy path
  2. Missing input
  3. Empty input
  4. Whitespace-only input
  5. Malformed input
  6. Out-of-range input
  7. Duplicate or conflicting input
  8. Hostile input
  9. Filesystem failure
  10. Concurrency or retry
  11. Cross-platform path/newline behavior
  12. Regression fixture from the linked issue

You do not need all twelve cases for every PR. You do need to cover the cases that match the risk of the touched code. If a case is not applicable, the PR should make that obvious from the issue scope or test rationale.

CLI and command routing

Changes to CLI parsing, command dispatch, query dispatch, command routers, gsd-tools, or gsd-sdk must include a negative input matrix for the affected command family.

Required cases where relevant:

  • Missing required arguments
  • Empty strings, for example --phase ""
  • Whitespace-only values
  • Duplicate flags, for example --phase 1 --phase 2
  • Conflicting flags, for example --json --raw
  • Malformed assignments, for example --phase= and --phase==1
  • Unknown subcommands at the touched command depth
  • Values that look like flags, for example --name --weird
  • Very long values and Unicode values
  • Shell metacharacters in values, for example ;, &&, $(), backticks, and quotes

CLI tests must assert on the full command contract:

  • Exit status
  • Structured --json result when the command supports JSON
  • Filesystem mutation or absence of mutation
  • No stack trace in non-debug failure output
  • No shell interpolation of attacker-controlled values

Prefer spawnSync(process.execPath, [scriptPath, ...args], { cwd, encoding: 'utf8' }) or execFileSync() with argv arrays. Do not use shell strings for tests that contain hostile values.

Parser and project-file inputs

Changes to markdown, TOML, frontmatter, roadmap, phase, state, config, or schema parsing must include adversarial fixtures. Put reusable fixtures under tests/fixtures/adversarial/ with a directory that names the input type, such as roadmap/, frontmatter/, config/, toml/, or planning-state/.

Required cases where relevant:

  • Malformed frontmatter
  • Duplicate keys
  • Mixed CRLF/LF newlines
  • Unclosed or nested fenced code blocks
  • Headings inside fenced code blocks
  • Unicode headings
  • Repeated or decimal phase IDs
  • Path traversal-like names such as ../../x
  • Null bytes or replacement characters
  • Huge but bounded files
  • TOML duplicate tables or trailing garbage
  • Empty arrays vs missing arrays
  • Scalars where arrays are expected, and objects where strings are expected

Property-style parser tests are encouraged for high-risk parsers. They must be deterministic: pin the seed, bound the iteration count, and print replay data on failure.

Fixture provenance (#2371)

A gate's fixtures may not be derived from the gate's own writer, grammar, or docstring examples. A negative fixture must come from a source that does not know the gate exists.

This is stricter than the adversarial-input rule above and exists because of it: tests/fixtures/adversarial/ covers hostile input, but a fixture written by the parser's own author — even a deliberately "realistic" one — is still drawn from the author's mental model of the format. It can only ever confirm what the author already believed, never surface what they didn't anticipate. A property-test generator has the same failure mode one level up: seeding the generator from the writer/render function that produces the same format makes the document shape a constant, so the property can never explore a document the writer wouldn't produce (see the document-shaped vs. writer-seeded property tests in tests/api-coverage.test.cjs for a worked example — the writer-seeded one cannot fail against a decoy table; the document-shaped one can).

For a gate whose fixtures come from real user reports, put them under tests/fixtures/representative/<gate>/ with a MANIFEST.json labeling each fixture's source issue and expected gate verdict, and drive them through the gate's real CLI entrypoint (gate-verdict altitude), not the parser function in isolation — see tests/fixtures/representative/README.md and tests/representative-corpus.test.cjs. If the gate is not yet fixed, do not mark the assertion { todo: true } and do not skip it: this repo's test-runner (gsd-test / gsd-test-runner) has no concept of node:test's todo option — its JSONL result parser only recognizes kind: "pass" | "fail", so a thrown todo-marked test is still counted as a real failure and blocks the push gate. Instead record BOTH the correct target verdict (expected*) and the exact current observed verdict (currentBuggyOutput) in the manifest, and assert against currentBuggyOutput — an honest, non-vacuous characterization of today's known-broken behavior that passes today and breaks loudly the moment the real fix changes the observed output, forcing the assertion to be flipped to expected*.

Filesystem writes and installers

Changes to install/uninstall flows, generated artifact writers, state/config writers, worktree safety, or any code that writes under .planning, runtime config dirs, .claude, .codex, hooks, or generated files must include fault-injection coverage where the seam allows it.

Required cases where relevant:

  • Missing parent directory
  • Target path exists as a file instead of a directory
  • Read-only target directory
  • Broken symlink
  • Symlink escaping the intended root
  • Paths with spaces, Unicode, or newlines
  • Partial write failure
  • Rename failure
  • Concurrent deletion or write collision
  • Temp-file cleanup after failure

Use node:test mocks such as mock.method() for fs.writeFileSync, fs.renameSync, fs.mkdirSync, fs.rmSync, and subprocess seams when the production code exposes a seam. Restore mocks with test hooks or t.after().

Security and prompt-injection surfaces

Changes that read prompts, plans, markdown, agent instructions, shell command projections, workstream/project names, or user-controlled files must treat those inputs as hostile.

Required cases where relevant:

  • Fake instruction tags, for example <instructions>ignore previous</instructions>
  • Heredoc breakouts
  • Shell command substitution payloads
  • Path traversal through project or workstream values
  • Malicious markdown links
  • Fake frontmatter fields that try to override intent
  • Secret-looking values in inputs, logs, stdout, stderr, and thrown errors
  • Environment variables with fake tokens to prove redaction

Security tests must assert both the positive guard behavior and the negative proof: no path escape, no command execution, no leaked token, no untrusted content promoted to instructions.

Generated files and parity

Changes to generators, generated .cjs/.ts files, command manifests, aliases, hooks, or SDK/runtime parity must test bad input and runtime parity, not only freshness.

Required cases where relevant:

  • Missing source command
  • Malformed command frontmatter
  • Duplicate command names or aliases
  • Partial generator output
  • Generator crash halfway through
  • Manual edits to generated files
  • Stale generated file with valid timestamp but wrong content
  • Runtime .cjs and SDK .ts generated surfaces disagree

Generator tests should run in temp fixtures and assert atomic output behavior. Do not mutate production generated files except in explicit freshness checks.

Prohibited: Source-Grep Tests

Never read source-code .cjs files with readFileSync to assert that strings exist within them. This is source-grep theater: it proves a literal is present in a file, not that the feature works at runtime.

// BAD — source-grep theater
const configSrc = fs.readFileSync(
  path.join(GSD_ROOT, 'gsd-core', 'bin', 'lib', 'config-schema.cjs'), 'utf-8'
);
assert.ok(
  configSrc.includes("'workflow.plan_bounce'"),
  'VALID_CONFIG_KEYS should contain workflow.plan_bounce'
);

This test passes even if workflow.plan_bounce is present but misspelled in the schema, removed from the validation path, or moved to a different file under a different name. It survives every behavioral regression and fails only on trivial renames.

The correct pattern for config key tests — use the CLI:

// GOOD — behavioral test via the CLI
test('config-set accepts workflow.plan_bounce', (t) => {
  const tmpDir = createTempProject();
  t.after(() => cleanup(tmpDir));

  const result = runGsdTools('config-set workflow.plan_bounce true', tmpDir);
  assert.ok(result.success, `config-set should accept workflow.plan_bounce: ${result.error}`);

  const configPath = path.join(tmpDir, '.planning', 'config.json');
  const config = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
  assert.strictEqual(config.workflow?.plan_bounce, true, 'value must be persisted');
});

This single test covers key registration in VALID_CONFIG_KEYS, the key's namespace resolution in KNOWN_TOP_LEVEL, and value persistence — all behaviors that the source-grep test could not touch.

Why this pattern broke at scale: Commit 990c3e64 in this repo updated 5 source-grep tests in one pass when VALID_CONFIG_KEYS moved between files. Zero of those tests were testing behavior. If they had been behavioral tests, the migration would have been invisible.

CI enforcement: The local/no-source-grep ESLint rule (eslint-rules/no-source-grep.cjs, wired in eslint.config.mjs) detects violations. Any test file that calls readFileSync on a .cjs path in a source directory without the exemption annotation below is flagged by npx eslint . (the Lint — ESLint CI step).

Exception: allow-test-rule: <reason>

Some tests legitimately read source files. There are six recognized categories:

Reason When to use
source-text-is-the-product Agent .md, workflow .md, command .md files — their text IS what the runtime loads. Testing text content tests the deployed contract.
architectural-invariant Implementation must use a specific primitive (e.g., Atomics.wait, atomic file writes) that cannot be tested by observing outputs.
structural-regression-guard A specific code pattern must (or must not) exist to prevent a class of bug (e.g., regex global-state misuse). Behavioral tests cannot distinguish which pattern was used.
docs-parity A reference doc must stay in sync with source-defined constants (e.g., CONFIG_DEFAULTS). The source is the canonical list; there is no runtime API to enumerate it.
integration-test-input A source file is used as a real fixture input to a transformation function under test — the file is not inspected for strings but passed as data.
structural-implementation-guard A feature's interception or wiring point is not reachable end-to-end via runGsdTools. Used temporarily until a behavioral path exists.
pending-migration-to-typed-ir Tracked for correction, not exempted. Test was identified by the lint as carrying a raw-text-matching pattern that contradicts the rule above. Each annotated file MUST cite the open migration issue (e.g. // allow-test-rule: pending-migration-to-typed-ir [#NNNN]) so the tracking is auditable. New tests cannot use this category — they must refactor production to expose typed IR. The annotation is removed when the test is corrected.

Annotate with a standalone // comment before the file's opening block comment:

// allow-test-rule: architectural-invariant
// state.cjs locking must use Atomics.wait(), not a spin-loop. Behavioral tests
// cannot observe which sleep primitive was chosen — only source inspection can.

/**
 * Regression tests for locking bugs #1909...
 */

The annotation must be a standalone // allow-test-rule: line, not inside a /** */ block comment — the CI linter scans for the pattern // allow-test-rule:.

Prohibited: Raw Text Matching on Test Outputs (file content, stdout, stderr)

Source-grep is not just readFileSync of a .cjs file. The same anti-pattern shows up wherever a test pattern-matches against text that a system-under-test produced, regardless of whether that text came from a source file, a rendered shim, a child process's stdout, or a free-form reason string. All forms are forbidden.

The following are all violations of the same rule:

// BAD — substring match on text written by the code under test
const cmdContent = fs.readFileSync(path.join(tmpDir, 'gsd-sdk.cmd'), 'utf8');
assert.ok(cmdContent.includes(`@node ${jsonQuoted} %*`), '.cmd embeds shim path');

// BAD — regex match on a child process's human-readable stdout formatter
const r = cp.spawnSync(SCRIPT, ['--patches-dir', dir]);
assert.match(r.stdout, /Failures: 1/);
assert.match(r.stdout, /not a regular file/);

// BAD — "structured parser" that hides string ops behind a function wrapper
function parseCmdShim(content) {
  const lines = content.split('\r\n').filter((l) => l.length > 0);
  return { header: lines[0], usesCRLF: content.includes('\r\n') };
}

// BAD — assert.match on a free-form `reason` string from a JSON report
assert.ok(/not a regular file/.test(report.results[0].reason));

Each of these passes on accidental near-matches (a comment containing @node somewhere, a stack trace that happens to say Failures: 1, a mis-typed reason that still contains the substring you're matching) and fails on harmless reformatting (changing Failures: 1 to 1 failure, swapping CRLF rendering style, rewording the error prose).

The rule

Tests assert on typed structured values. If the code under test produces text, the code under test must also expose a structured intermediate representation, and the test must assert on that IR — never on the rendered text.

Concretely: for any system-under-test that produces text output (a file renderer, a CLI formatter, an error-message builder), the production code MUST expose a typed alternative that the test consumes:

Output kind Required structured surface What the test asserts on
Rendered file (shim, template, generated code) A pure builder function returning the IR ({ invocation, eol, fileNames, render }) triple.invocation.target === expected, triple.eol.cmd === '\r\n'
CLI human-formatter output A --json mode that emits the same data structurally report.results[0].reason === REASON.FAIL_INSTALLED_NOT_REGULAR_FILE
Error / status / reason A frozen enum (Object.freeze({ FAIL_X: 'fail_x', ... })) assert.equal(result.reason, REASON.FAIL_X)
File presence after a write fs.statSync().isFile(), .size > 0, .mtimeMs advances Filesystem facts; never read the file content back

Concrete example from this repo

gsd-core/bin/verify-reapply-patches.cjs exposes a frozen REASON enum and emits it through --json. Tests assert report.results[0].reason === REASON.FAIL_USER_LINES_MISSING rather than regex-matching the human-readable prose. The human formatter exists for operator console output only — tests must not depend on it. Adding a new reason code requires updating the REASON enum, the --json output, AND the test that locks Object.keys(REASON).sort() — three coordinated changes that keep the code surface from drifting from the test surface. A pure builder that returns the IR (no I/O) and a writer that consumes it — fs.statSync(target).size === Buffer.byteLength(render()) to prove the writer writes what the renderer produces, without comparing content — is the same pattern applied to rendered files.

Hiding grep behind a function is still grep

parseCmdShim, parsePs1Invocation, etc. that internally do content.split(...), lines[1].trim(), content.includes(...) are still string manipulation. The fact that the entry point looks like a parser doesn't change what's happening underneath — the test is still asserting on the lexical shape of rendered text. The fix is not "wrap the grep in a function with a typed-looking return value." The fix is to eliminate the rendered text from the test path entirely by surfacing the IR.

When you cannot eliminate text matching

There are exactly two cases where text content is the legitimate object of a test, both already covered by the existing exemption matrix:

  1. source-text-is-the-product — workflow .md / agent .md / command .md files where the deployed text IS what the runtime loads.
  2. docs-parity — a reference doc must mirror source-defined constants and there is no runtime enumeration API.

For everything else, if a test reaches for .includes() / .startsWith() / assert.match(text, /…/), the production code is missing a typed surface. Add the typed surface; do not work around it.

CI enforcement: the local/no-source-grep ESLint rule (eslint-rules/no-source-grep.cjs) is being extended (see issue tracker for the latest scope) to flag String#includes/String#startsWith/String#endsWith/assert.match on readFileSync results and on cp.spawnSync stdout/stderr in test files, with the same // allow-test-rule: exemption mechanism.

Node.js Version Compatibility

Node 22 is the minimum supported version. Node 24 is the primary CI target. Node 26 is the forward-compatibility target: do not add tests or production code that depend on deprecated behavior likely to fail there.

Version Status
Node 22 Minimum required — Active LTS until October 2026, Maintenance LTS until April 2027
Node 24 Primary CI target — current Active LTS, all tests must pass
Node 26 Forward-compatible target — avoid deprecated APIs and exact runtime-error prose

Do not use:

  • Deprecated APIs
  • APIs not available in Node 22

Safe to use:

  • node:test — stable since Node 18, fully featured in 24
  • describe/it/test — all supported
  • beforeEach/afterEach/before/after — all supported
  • t.after() — per-test cleanup
  • mock.method() — approved for scoped filesystem/subprocess fault injection
  • t.plan() — fully supported
  • Snapshot testing — fully supported

Assertions

Use node:assert/strict for strict equality by default:

const assert = require('node:assert/strict');

assert.strictEqual(actual, expected);      // ===
assert.deepStrictEqual(actual, expected);  // deep ===
assert.ok(value);                          // truthy
assert.throws(() => { ... }, /pattern/);   // throws
assert.rejects(async () => { ... });       // async throws

Running Tests

# Run all tests
npm test

# Run a single test file
node --test tests/core.test.cjs

# Run with coverage
npm run test:coverage

For examples of required negative matrices, parser fixtures, filesystem fault injection, security abuse tests, generated-file checks, and runtime/SDK parity tests, see TEST-EXAMPLES.md.

Preferred local benchmark runner (before PR)

When you can, run the local test bench harness before opening a PR — especially for Windows-sensitive changes.

This gives maintainers a faster, higher-confidence signal than CI-only validation.

Pre-PR Seam Checks (Manifest/Alias Routing)

If you touched any of the command-manifest or generated alias files, run:

npm run check:alias-drift

This verifies generated alias artifacts are in sync with manifest source-of-truth.

Editing shipped content (gsd-core/workflows, references, templates, contexts, agents/, commands/gsd/)

Editing the content of a copied shipped file — a gsd-core/workflows/*.md, an agent, a command definition — requires zero manual fixture regeneration. There is no committed path→hash manifest or per-file size baseline to update by hand; the differential attribution check (tests/emitted-attribution.test.cjs, ADR-2719) computes what your PR changed against next and requires every emitted-artifact hash that moved to be attributable to your diff. If it is not, the check fails and names the paths.

Legitimate cases where emitted bytes move for a reason your diff cannot show directly — a converter change, for example — go through tests/emitted-drift-ack.json (name the path, say why); see CONTEXT.md's ### Emitted Artifact Provenance entry for the full model. Growth in a gsd-core/workflows/*.md or agents/gsd-*.md file is reported with its exact byte delta and needs the same acknowledgment; the outer tier hard caps in tests/workflow-size-budget.test.cjs / tests/agent-size-budget.test.cjs are unaffected and still apply.

npm run regen:derived still exists for the artifacts that ARE committed and derived — sync-manifest-versions, the ADR index, the capability matrix, the inventory manifest, the registry, and tests/fixtures/install-tree/*.json (npm run gen:install-tree, the one fixture family ADR-2719 §7 keeps committed, because it conflicts on 0 of 7 and its diffs are readable). Run it after a change to any of those, before committing:

npm run regen:derived

Optional local pre-commit hook entry (Git-native):

# one-time setup
mkdir -p .githooks
cat > .githooks/pre-commit <<'EOF'
#!/usr/bin/env bash
set -euo pipefail

if git diff --cached --name-only | grep -Eq "^sdk/src/query/command-manifest\.|^sdk/src/query/command-aliases\.generated\.ts$|^gsd-core/bin/lib/command-aliases\.generated\.cjs$|^sdk/scripts/gen-command-aliases\.ts$"; then
  npm run check:alias-drift
fi
EOF
chmod +x .githooks/pre-commit
git config core.hooksPath .githooks

Optional local pre-push hook to block a private author-email pattern:

# set locally in your shell profile (example)
export GSD_BLOCKED_AUTHOR_REGEX='@example-corp\\.com$'

cat > .githooks/pre-push <<'EOF'
#!/usr/bin/env bash
set -euo pipefail

zero_sha='0000000000000000000000000000000000000000'
blocked_regex="${GSD_BLOCKED_AUTHOR_REGEX:-}"
[[ -z "$blocked_regex" ]] && exit 0
violations=()

while read -r local_ref local_sha remote_ref remote_sha; do
  [[ "$local_sha" == "$zero_sha" ]] && continue
  if [[ "$remote_sha" == "$zero_sha" ]]; then
    commits=$(git rev-list "$local_sha" --not --remotes)
  else
    commits=$(git rev-list "$remote_sha..$local_sha")
  fi
  while read -r commit; do
    [[ -z "$commit" ]] && continue
    email=$(git show -s --format='%ae' "$commit" | tr '[:upper:]' '[:lower:]')
    if printf '%s' "$email" | grep -Eq "$blocked_regex"; then
      violations+=("$commit <$email>")
    fi
  done <<< "$commits"
done

if [[ ${#violations[@]} -gt 0 ]]; then
  echo "Push blocked: commit author email matched local blocked regex ($blocked_regex)." >&2
  printf '  - %s\n' "${violations[@]}" >&2
  exit 1
fi
EOF
chmod +x .githooks/pre-push

CI Test Quality Checks

The following checks run on every PR in addition to the test suite:

Job What it checks How to pass
Lint — ESLint No source-grep tests (see above), via the local/no-source-grep rule Replace with runGsdTools() behavioral tests, or add // allow-test-rule: <reason>
Lint — cross-platform portability Windows-portability defects in tests, via local/no-path-literal-in-assert (more rules land per ADR-1703) — e.g. a path-returning call asserted against a hardcoded /-literal Normalize the actual: String(pathFn(...)).replace(/\\/g, '/'), or structure platform-specific code behind a process.platform !== 'win32' guard. No eslint-disable — see cross-platform-portability-rules.md

Run locally before pushing: npm run lint (or npx eslint .)

Architecture-Aware Testing Requirements

When work touches architecture, routing, policy, registry assembly, or command semantics:

  • Write tests against module interfaces and seam behavior, not implementation trivia.
  • Prefer invariant/contract tests that protect ADR-backed behavior and CONTEXT.md terminology.
  • Ensure tests validate canonical behavior through the defined seam (for example: structured result contracts, canonical command metadata, and adapter parity), not source-text coupling.
  • If ADRs define expected behavior, tests should assert those expectations directly.

Test Requirements by Contribution Type

The required tests differ depending on what you are contributing:

Bug Fix: A regression test is required. Write the test first — it must demonstrate the original failure before your fix is applied, then pass after the fix. A PR that fixes a bug without a regression test will be asked to add one. If the bug involves CLI input, parsers, filesystem writes, security/prompt surfaces, generated files, or SDK/runtime parity, the regression test must use the relevant QA matrix above and include negative proof that the bad behavior no longer happens. "Tests pass" does not prove correctness; it proves the bug isn't present in the tests that exist.

Enhancement: Tests covering the enhanced behavior are required. Update any existing tests that test the area you changed. If the enhancement expands accepted input, changes command routing, broadens parser behavior, changes generated output, or touches installer/write paths, add the relevant adversarial cases from the QA matrix above. Do not leave tests that pass but no longer accurately describe the behavior.

Feature: Tests are required for the primary success path and enough failure scenarios to cover the relevant QA matrix above. At minimum, every feature must cover one failure scenario; features that expose CLI input, parse user files, write files, generate artifacts, call subprocesses, or build prompts must cover the relevant negative/hostile cases. Leaving gaps in test coverage for a new feature is a rejection reason.

Behavior Change: If your change modifies existing behavior, the existing tests covering that behavior must be updated or replaced. For high-risk surfaces, update the adversarial tests as well as the happy path. Leaving passing-but-incorrect tests in the suite is not acceptable — a test that passes but asserts the old (now wrong) behavior makes the suite less useful than no test at all.

Reviewer Standards

Reviewers do not rely solely on CI to verify correctness. Before approving a PR, reviewers:

  • Build locally (npm run build if applicable)
  • Run the full test suite locally (npm test)
  • Confirm regression tests exist for bug fixes and that they would fail without the fix
  • Validate that the implementation matches what the linked issue described — green CI on the wrong implementation is not an approval signal

"Tests pass in CI" is not sufficient for merge. The implementation must correctly solve the problem described in the linked issue.

Code Review Lessons

Input validation: check shape, not just type

Defensive normalization at trust boundaries must validate both the value's type and its semantic shape. A typeof === 'string' check is necessary but insufficient when the field's contract requires a specific format (UUID v4, semver, file path, etc.). See ADR 227 for the architectural standard and concrete cases.

Code Style

  • CommonJS (.cjs) — the project uses require(), not ESM import
  • No external dependencies in core — gsd-tools.cjs and all lib files use only Node.js built-ins
  • Conventional commits — feat:, fix:, docs:, refactor:, test:, ci:. The full grammar is <type>(<scope>): <subject> (enforced by hooks/gsd-validate-commit.sh; subject ≤72 chars, lowercase, imperative mood, no trailing period). When the work resolves a tracked issue, put the issue number in the scope: fix(#1520): randomize mktemp temp paths on BSD/macOS. The same convention applies to PR titles — release notes are grouped by the title's type prefix (feat → Feature, fix → Fix, everything else → Enhancement).

File Structure

bin/install.js          — Installer (multi-runtime)
gsd-core/
  bin/lib/              — Core library modules (.cjs)
  workflows/            — Workflow definitions (.md)
                          Large workflows split per progressive-disclosure
                          pattern: workflows/<name>/modes/*.md +
                          workflows/<name>/templates/*. Parent dispatches
                          to mode files. See workflows/discuss-phase/ as
                          the canonical example (the discuss-phase/modes split, #717). New modes for
                          discuss-phase land in
                          workflows/discuss-phase/modes/<mode>.md.
                          Per-file growth is caught by the differential
                          attribution check (tests/emitted-attribution.test.cjs,
                          ADR-2719) — it reports the exact byte delta and
                          requires an entry in tests/emitted-drift-ack.json,
                          no committed snapshot to regenerate. Loose tier
                          hard caps remain in tests/workflow-size-budget.test.cjs.
                          The same applies to agent files (agents/gsd-*.md,
                          tests/agent-size-budget.test.cjs). Full how-to +
                          reference in docs/TESTING-SUITES.md (Workflow &
                          agent size budget); see issue #1074.
  references/           — Reference documentation (.md)
  templates/            — File templates
agents/                 — Agent definitions (.md) — CANONICAL SOURCE
commands/gsd/           — Slash command definitions (.md)
tests/                  — Test files (.test.cjs)
  helpers.cjs           — Shared test utilities
docs/                   — User-facing documentation

Source of truth for agents

Only agents/ at the repo root is tracked by git. The following directories may exist on a developer machine with GSD installed and must not be edited — they are install-sync outputs and will be overwritten:

Path Gitignored What it is
.claude/agents/ Yes (.gitignore:9) Local Claude Code runtime sync
.cursor/agents/ Yes (.gitignore:12) Local Cursor IDE bundle
.github/agents/gsd-* Yes (.gitignore:37) Local CI-surface bundle

If you find that .claude/agents/ has drifted from agents/ (e.g., after a branch change), re-run bin/install.js to re-sync from the canonical source. Always edit agents/ — never the derivative directories.

Security

  • Path validation — use validatePath() from security.cjs for any user-provided paths
  • No shell injection — use execFileSync (array args) over execSync (string interpolation)
  • No ${{ }} in GitHub Actions run: blocks — bind to env: mappings first