tmu
7973bf0d0b
`npm run check` should be the fast, offline correctness ladder only (tsc + oxlint + oxfmt + cspell, ~3s), so an agent can run it as a confirmation gate during feature work. check:knip / check:outdated were advisory whole-project / network scans, and check-outdated exits non-zero whenever any dep is behind. Keeping them in check made `npm run check` (and the CI build gate) fail on dependency freshness, which must not block an unrelated feature PR. Rename them to maintain:knip / maintain:outdated, aggregate under `npm run maintain`, and run it in CI as a dedicated non-blocking job (continue-on-error) that surfaces findings without ever gating a merge. Update the script-prefix convention, the feedback-tier table, and AGENTS.md: the agent may now run `npm run check`; only `maintain` stays out of the feature loop.
77 lines
9.1 KiB
Markdown
77 lines
9.1 KiB
Markdown
# Contributing
|
||
|
||
This document is for maintainers and contributors working on the project itself. End-user documentation is in [README.md](./README.md). The machine entry point for AI coding agents is [AGENTS.md](./AGENTS.md); keep this file as the prose home for the rules below so agents and humans don't diverge.
|
||
|
||
## Rules the tools don't enforce
|
||
|
||
CI and review will bounce these even though `npm run check` and the linters don't catch them. They're the high-frequency things a contributor (or an agent) reaches for by default:
|
||
|
||
- **Source imports use `.ts` extensions, never `.js`.** `node --strip-types` resolves the `.ts` form at test time; `rewriteRelativeImportExtensions` emits `.js` in `dist/`. "Pre-fixing" an import to `.js` breaks the inner loop. (rationale: README § Tooling decisions)
|
||
- **A new `npm run` script must reuse an existing prefix** (`check:` / `fix:` / `test:` / `watch:` / `maintain:` / `publish:`). If none fits, that's a signal the script doesn't belong in the pipeline — not a reason to invent a new prefix. (see [Script prefix convention](#script-prefix-convention))
|
||
- **`oxlint-disable` directives live in source, not `.oxlintrc.json`.** The trade-off must sit next to the code it silences. This is a _human_ last-resort convention; agents must not add these — see [AGENTS.md § Never do](./AGENTS.md#never-do). (rationale: README § Tooling decisions)
|
||
- **Don't put slow / network / whole-project scans in `check` or pre-commit.** `maintain:knip` (~4s, whole project) and `maintain:outdated` (~6.5s, registry) are advisory, not correctness — they live under the `maintain:` prefix and run in CI as a **non-blocking** job, never gating a merge. (see [Why these splits](#why-these-splits) and [Script prefix convention](#script-prefix-convention))
|
||
- **There is no local `npm run publish`, and `publish:publint` / `publish:attw` don't go in `check`.** They validate the publishable artifact (`dist/`) and run only in the CI `publish` job. (see [Publishing workflow](#publishing-workflow))
|
||
|
||
## Commit messages
|
||
|
||
Gitmoji subject, imperative mood, 50/72 wrapping. The template is `commit-message-template`; `npm run use:git-commit-message` installs it into `.git/COMMIT_EDITMSG`.
|
||
|
||
Examples from history: `:sparkles: Add watch tier with watch:test child`, `:recycle: Move type-aware config to .oxlintrc.json; use source-level disable directives`, `:memo: Restore unique maintainer content as CONTRIBUTING.md`. The body explains _what and why_, not _how_; link issues with `Resolves #...`.
|
||
|
||
## Script prefix convention
|
||
|
||
Script names in `package.json` use a prefix that signals _when_ the script is intended to run. A `<prefix>:<name>` script is implicitly aggregated by a `<prefix>` script (if one exists) and run by the corresponding lefthook hook or CI step. Picking the right prefix documents the script's intended lifecycle:
|
||
|
||
- `check:*` — read-only verification. Aggregated by `npm run check`. Used in pre-commit hooks (on staged files) and CI's build job (on the whole project). Read-only; never modifies files.
|
||
- `fix:*` — mutating counterpart of a `check:*` script. Aggregated by `npm run fix`. Use after `npm run check` to auto-resolve issues; the diff is the review surface.
|
||
- `test:*` — test scripts. `npm run test` is the canonical entry point (`check:tsc` + unit tests); `test:unit` skips the typecheck for fast local iteration; `test:ci` adds c8 coverage and is the CI variant.
|
||
- `watch:*` — long-running watchers, started manually via `npm run watch` for the inner dev loop. Sits "before" pre-commit in the feedback ladder (see Feedback tiers below). Currently a single child (`watch:test`); future `watch:oxlint` / `watch:tsc` would aggregate under the same `watch` umbrella.
|
||
- `maintain:*` — advisory repo-maintenance scans (dead code, dependency freshness): read-only, but whole-project and/or network-bound, so they are **not** correctness gates. Aggregated by `npm run maintain`. Run on an explicit maintenance / update-deps branch, and in CI as a non-blocking job (continue-on-error) that surfaces findings without ever failing a feature PR.
|
||
- `publish:*` — runs only at publish time, in the CI `publish` job (immediately before `npm publish`). There is **no** local `npm run publish` script — publishing is CI-only by policy. The `publish:` prefix still documents intent: this script validates the _publishable artifact_ (e.g., `dist/`) rather than the source.
|
||
|
||
A new script should pick the prefix that matches its lifecycle, not invent a new one. If no existing prefix fits, that's a signal the script doesn't belong in the standard pipeline.
|
||
|
||
## Feedback tiers
|
||
|
||
The tools are organized into a feedback ladder. Each tier catches different things at different costs; the rule of thumb is "earlier tiers fire more often, faster tiers catch less, slower tiers are more thorough":
|
||
|
||
| Tier | When | What it runs | Time |
|
||
| -------------------------------- | ---------------------- | --------------------------------------------------------------------- | ----- |
|
||
| `npm run watch` | manual | `watch:test` — re-runs tests on file save | ~0.1s |
|
||
| Pre-commit (auto) | on stage | tsc + oxlint + oxfmt + cspell (staged files only) | ~1.3s |
|
||
| Pre-push (auto) | on push | `npm test` (full tsc + unit tests) | ~3.5s |
|
||
| `npm run check` | manual | Correctness gates: tsc + oxlint + oxfmt + cspell (whole project) | ~3s |
|
||
| `npm run fix` | manual | Auto-resolve fixable issues (lint, format) | ~3s |
|
||
| `npm run maintain` | manual / CI (advisory) | `maintain:knip` + `maintain:outdated` (whole-project + network scans) | ~10s |
|
||
| CI build (auto) | on push/PR | `npm run check` + `npm run test:ci` | ~30s+ |
|
||
| CI maintain (auto, non-blocking) | on push/PR | `npm run maintain` — reports, never fails the build | ~10s |
|
||
| CI publish (auto) | on tag | `publish:publint` + `publish:attw`, then `npm publish` | ~10s |
|
||
|
||
### Why these splits?
|
||
|
||
- **`watch:*` is a manual tier, not a hook.** The developer starts it on demand (it has to be killed with Ctrl-C) and it runs in a dedicated terminal pane. It sits as the earliest tier in the feedback ladder, catching failures the moment a file is saved — before staging, before commit. The umbrella `watch` script is designed to aggregate multiple `watch:*` children (currently just `watch:test`); if more watchers are added later (e.g. `watch:oxlint`), the umbrella would switch to running them concurrently rather than sequentially.
|
||
- **`check:tsc`, `check:oxlint`, `check:oxfmt`, `check:cspell`** are in pre-commit because they are fast (~0.2–0.5s each), fully offline, and naturally scope to staged files via the `LEFTHOOK_FILES` env var convention. They give instant feedback on what you typed.
|
||
- **`test` (and the `tsc` it includes) is in pre-push** because it runs the whole test suite across the whole project. The pre-commit `LEFTHOOK_FILES` convention doesn't apply to the test runner, so pre-commit isn't the right home. Pre-push runs after all commits are made but before the push leaves the machine, catching regressions that span multiple commits.
|
||
- **`maintain:knip` and `maintain:outdated` are advisory, not correctness** — knip scans the whole project (~4s) and `check-outdated` queries the npm registry (~6.5s, network-dependent, and it exits non-zero whenever any dep is behind). That's why they live under `maintain:`, are excluded from pre-commit and from `check`, and run in CI as a **non-blocking** job: a stale dependency must never block an unrelated feature PR.
|
||
- **`publish:publint` and `publish:attw` are NOT in `check`** — they belong to the `publish:` prefix because they validate the _publishable artifact_ (`dist/`) and require a fresh build. Running them on every commit would be wasteful. They run in the CI `publish` job immediately before `npm publish`.
|
||
|
||
### Before pushing
|
||
|
||
Run `npm run check && npm run test` locally — both are the fast, offline correctness gates, safe to run any time. Anything `maintain:knip` or `maintain:outdated` would catch is reported by the non-blocking CI `maintain` job; run `npm run maintain` yourself only on a maintenance / update-deps branch.
|
||
|
||
## Publishing workflow
|
||
|
||
Publishing is CI-only by policy. Local `npm publish` is not supported.
|
||
|
||
1. Develop and merge PRs to `main`.
|
||
2. CI runs `npm run check` + `npm run test:ci` on every push and PR — this is the authoritative gate.
|
||
3. After all intended changes are on `main`, bump the version locally:
|
||
```sh
|
||
npm version <patch|minor|major>
|
||
```
|
||
4. Push the tag to GitHub:
|
||
```sh
|
||
git push --follow-tags origin main
|
||
```
|
||
5. The `publish` CI job runs on the tag: `build` → `publish:publint` → `publish:attw` → `npm publish --access public`. The publish-tier checks must pass before the artifact is published.
|