Test & quality alignment plan

# Test & Quality Alignment Plan — git-snitch

_Revised 2026-09-05: CI removed by owner decision — quality gates are local commands run by the owner/agents; no GitHub Actions._

- **Project**: `/home/didi/workspace/git-snitch` (plain-CJS Node CLI; npm package named `git-moar` v0.0.1; remote `DimitriGilbert/git-snitch`)
- **Date**: 2026-09-05
- **Generated by the fleet test-alignment initiative**
- **Inputs**:
- Audit report: [`../.test-alignment-audit/reports/git-snitch.md`](../.test-alignment-audit/reports/git-snitch.md)
- Fleet synthesis & decisions: [`../.test-alignment-audit/synthesis-decisions.md`](../.test-alignment-audit/synthesis-decisions.md)
- **Status**: plan only — nothing in this document has been implemented.

---

## 1. Current state snapshot (grounded in the repo)

- 12 source files, 5,864 LOC: `bin/cli.js` (66) + 11 modules in `lib/` (largest: `lib/charts.js` 1,622; `lib/templates.js` 1,063; `lib/scattered.js` 582).
- Runtime deps: `chalk ^4.1.2`, `sloc ^0.3.2`. **Zero devDependencies.**
- `"test": "echo \"No tests yet\" && exit 0"` (`package.json` line 11) — fake-green runner. Zero test files, zero CI (no `.github/`), zero hooks, no lint/format config.
- `engines.node >=14.0.0`; local Node is v24.20.0. `package-lock.json` present (npm; no pnpm/bun artifacts).
- Git access is hard-coupled to `child_process.execSync` at **6 invocation sites** (the audit's "~10" counts these plus their `require('child_process')` destructures):
- `lib/utils.js:66` — `git config --get remote.origin.url` (getRepoInfo)
- `lib/utils.js:208` — `git branch -a --contains ` (getCommitBranchInfo)
- `lib/snitch.js:309` — `git log --pretty=format… --numstat` (snitch run)
- `lib/snitch.js:364` — `git config --get remote.origin.url` (snitch run)
- `lib/scattered.js:486` — `git log …` with `cwd: repoPath` (scattered run)
- `lib/templates.js:1048` — `xdg-open`/`open`/`start` (openInBrowser — not git)
- Pure, dependency-free logic already exported and unit-testable without any refactor: `parseArgs` (`lib/snitch.js`), all of `lib/commit-classifier.js`, `lib/quality-metrics.js`, `lib/productivity.js`, `lib/hotspots.js`, and in `lib/utils.js`: `parsePeriodToGitSince`, `parseGitLog`, `calculateTimingStats`, `generateUserStats`, `generateProjectStats`, `createCommitUrl`, `createBranchUrl`.
- HTML generation (`templates.js` + `charts.js` = 46% of LOC) is string templating over a `reportData` object — ideal for golden-file tests.
- `.npmignore` already excludes `*.test.js` and `test/`, so colocated tests will not ship in the package. `.gitignore` only has `node_modules` and `*.tgz`.
- Known hygiene defects: package name/bin/URLs say `git-moar`, dir/remote say `git-snitch`, sibling repo `/home/didi/workspace/gitsnitch` holds the reverse; `"author": "Your Name"` scaffold placeholder.

---

## 2. Adopted decisions (synthesis D1–D7, translated to plain-CJS)

| Decision | Applies? | Translation for this repo |
|---|---|---|
| **D1 — runner & layout** | Yes, with the repo-specific ruling | Synthesis explicitly assigns **`node:test` + `node --test`** to git-snitch (zero devDeps fits the repo's philosophy). **Explicit deviation from the fleet-default vitest, per the synthesis itself** — vitest would be the only test dep in a zero-devDep CJS repo for no benefit at this scale. Layout: tests colocated as `lib/*.test.js` (per D1 colocation rule; `.npmignore` already excludes them) plus a `test/` dir for integration/golden fixtures. Root-level headless one-command entry: `npm test`. Requires `engines.node` bump `>=14` → `>=18` (D1 sanctions this; package is unpublished at v0.0.1). |
| **D2 — coverage** | Yes, state 3 | Zero-test repo → **coverage reported, no thresholds** until seeds exist, then ratchet. Analogy for the vitest/v8 provider: `node --test --experimental-test-coverage` (report-only; keeps zero-devDep style; `c8` only if the flag proves insufficient). Baseline recorded in AGENTS.md once measured. No threshold ever lowered. |
| **D3 — lint & format** | Yes, JS analogy | **ESLint 9 flat config** (`eslint.config.js`, CommonJS `module.exports` form) with `@eslint/js` recommended + node globals. typescript-eslint / `recommendedTypeChecked` / verbatimModuleSyntax are N/A (no TS). Prettier: **deferred in v1**, stated in AGENTS.md rather than pretended (D3 allows this). Gates: zero errors. |
| **D4 — quality metrics** | Partial, report-only | knip is nominated for publishing repos **"later git-snitch"** — adopt as a report-only script here (export-surface drift is real: 11 `module.exports` blocks). jscpd/CRAP: not in this repo's v1 — they arrive later as local report scripts when needed; sequencing coverage (D2) before CRAP is respected. |
| **D5 — no CI by design (owner decision 2026-09-05)** | Yes | **No GitHub Actions, no reusable-workflow caller, no local workflow fallback** — the earlier "thin caller of `DimitriGilbert/.github` (with a minimal local-workflow fallback)" plan is dropped entirely. Enforcement = the root-level headless verification entrypoint (`npm run verify`: lint → test, composed in Phase 5) run by the owner and by agents; the plan's phase validation gates are run by implementer/validator agents before any phase is declared done. Gates in v1: lint + tests; typecheck N/A. Coverage stays a report-only on-demand local command (D2 state 3). |
| **D6 — shared config distribution** | Analogy only | `@dg/config` ships TS consumables (tsconfigs, vitest factory) — **N/A for a plain-CJS repo**. Analogy adopted: keep this repo's configs local and minimal (`eslint.config.js`, `renovate.json`), converge later if a JS ESLint preset is published to npm. No config-churn PR for its own sake (D6 rule). |
| **D7 — hooks** | Yes | No hooks. The local gate commands run by the owner/agents are the only authoritative gate; AGENTS.md will state it. A repo with no hooks and a green local `npm run verify` is compliant. |

---

## 3. Synthesis Definition of Done — checklist with current status

| # | DoD item | Current status | Target |
|---|---|---|---|
| 1 | Root-level headless test command, documented in AGENTS.md | Missing — fake `echo` script; no AGENTS.md | `npm test` = `node --test`; documented (Phase 1, 6) |
| 2 | Headless verification entrypoint running typecheck + test + lint in one local command; agent records green before a phase is done | None (no aggregate command) | `npm run verify`; **typecheck N/A (JS)** — lint + tests are the static/dynamic gates (Phase 5) |
| 3 | Coverage measured & reported; baseline in AGENTS.md; thresholds only per D2 state | None | D2 **state 3**: report-only via `node --test --experimental-test-coverage`, baseline recorded, no thresholds (Phase 5–6) |
| 4 | Language-appropriate lint, zero errors, wired into the headless entrypoint; dead lint task implemented or deleted | None | ESLint 9 flat, zero errors, in `npm run verify`; **dead lint task N/A** (no turbo/makefile) (Phase 4, 5) |
| 5 | AGENTS.md "Testing" section matches reality | No AGENTS.md | Created in Phase 6 after infra exists — no phantom docs |
| 6 | Dead scaffold test deps removed or used | **N/A** — zero devDeps, no scaffold; principle restated: devDeps stay minimal (ESLint toolchain only) | Hold the line |
| 7 | Renovate one-liner; no `"latest"` deps | No Renovate; no `"latest"` pins (chalk/sloc are normal ranges) | `renovate.json` extending `local>DimitriGilbert` (Phase 6) |
| 8 | Quality-report script (knip + jscpd, CRAP where coverage) runnable locally, report-only | None | knip report-only locally (Phase 6) |
| 9 | Nothing satisfied by a fake pass | **Violated today** — `echo "No tests yet" && exit 0` is exactly the banned pattern | Deleted in Phase 1; validators enforce no tautological assertions fleet-wide |

---

## 4. Phased plan

Execution model per the `subagent-orchestration` skill: each phase (or sub-phase) is executed by an **implementer** subagent that runs the gatekeeping commands before reporting done, then validated by a **different validator** subagent that must actually read the code and enforce NO-SLOP; failures go to a **fixer** (all errors at once) and re-validation, max 3 attempts. Phases are sequential unless marked parallelizable.

**NO-SLOP policy for this repo (JS adaptation)**: no `TODO`/`FIXME`/placeholder code; no unused imports/variables (ESLint will enforce this from Phase 4 onward); no console.log hacks to suppress errors (note: `console.log` is this CLI's legitimate output channel — the ban applies to error suppression, not product output); no fake or tautological assertions (every test asserts real behavior of a real import from `lib/`); deterministic tests (fixed `GIT_AUTHOR_DATE`/`GIT_COMMITTER_DATE`, `fs.mkdtemp` tmpdirs, no network); no package-manager switches; no hand-edited lockfile. **Gatekeeping commands** (this repo has no typecheck/build — the skill's check-types mandate maps to these): `npm test`, and `npm run lint` once Phase 4 lands.

### Phase 0 — Preflight hygiene (effort: S)

**Goal**: repo is ready to receive a test runner without carrying known blockers.

Tasks (implementer):
- `package.json`: `engines.node` → `">=18.0.0"` (D1 prerequisite for node:test).
- `package.json`: `author` → `"Dimitri Gilbert"` (audit P3 scaffold leftover; safe, unrelated to the naming-swap decision).
- `.gitignore`: add `coverage/` and test scratch dirs if any are introduced later.
- `.npmignore`: add `coverage/` (once `.npmignore` exists it overrides `.gitignore` for packing, so new dev artifacts must be listed explicitly).
- Sanity: `npm ci` (or `npm install`) completes; `node bin/cli.js` prints help; `node --version` ≥ 18 confirmed.

Validation gate (validator): `npm ci` clean; `node bin/cli.js` exits 0 printing usage; `engines`/`author` correct in `package.json`; no other files touched (`git status`).

### Phase 1 — Real test runner + pure-logic seed suite (effort: M)

**Goal**: the fake-green script is dead; `npm test` runs real assertions over the ~1,300 LOC of pure logic. Per D1's named seed targets.

Tasks (implementer — parallelizable as 2 sub-phases per the skill: (a) runner + parseArgs/utils tests, (b) classifier/metrics/productivity/hotspots tests; each with its own validator, then a phase-wide validator):
- `package.json`: `"test": "node --test lib/ test/"` (explicit paths, no default-glob surprises); the `echo` script is deleted in the same commit — the repo is never left without a working `test` script.
- Colocated unit tests (`.npmignore` already keeps them out of the package):
- `lib/snitch.test.js` — `parseArgs`: every flag/short form, defaults, invalid `--sort-by`/`--sort-order` fallback to defaults (assert the returned object, not the console warning), `--all-branches` + `--branch` conflict resolution, unknown-option tolerance.
- `lib/utils.test.js` — `parsePeriodToGitSince` (all period formats), `parseGitLog` against a committed fixture string of real `git log --pretty=format:"%H|%an|%ae|%ad|%s" --numstat` output (edge cases: renames, binary lines, empty diff), `calculateTimingStats`, `generateUserStats`, `generateProjectStats`, `createCommitUrl`/`createBranchUrl` for https/SSH/enterprise remotes.
- `lib/commit-classifier.test.js` — `classifyCommit` across conventional-commit/fix/feature/docs types, `generateCommitTypeBreakdown`, `extractIssueReferences` (with and without config), `analyzeCommitMessageQuality`.
- `lib/quality-metrics.test.js` — known-value math: `calculateChurnRate`, `calculateBusFactor` (single-author vs distributed), `calculateGiniCoefficient` (perfect equality = 0, concentration → 1), `calculateAvgCommitSize`, `calculateCodeStability`, `calculateHealthScore` + `getHealthScoreRating` boundaries, `calculateTechnicalDebt`.
- `lib/productivity.test.js` — `findPeakHours`/`findPeakDays` on fixed-date fixtures, `calculateVelocity`, `analyzeDevelopmentRhythm`, `calculateCollaborationScore`, `calculateFocusTime`, `calculateTimeToValue`.
- `lib/hotspots.test.js` — `findFileHotspots`, `calculateRiskLevel` matrix, `findKnowledgeSilos`, `analyzeFileTypeVolatility`, `calculateHotspotStats`.
- `lib/config.test.js` — the exported singleton's load/save/get/set behavior against a tmpdir (it writes config files — keep it hermetic).
- Tests import only real modules; no mocks needed at this phase (that is the point of seeding pure logic first).

Validation gate (validator — reads every test file): `npm test` exits 0 with a real node:test summary (> 0 suites, target ≥ 60 test cases across the 7 files); zero assertions on literals the test itself constructs (tautology check); every test file imports the production module under test; fake script gone (`grep '"test"' package.json`).

### Phase 2 — execSync seam + integration tests over real temp repos (effort: L)

**Goal**: git invocation moves behind one injectable seam; the two `run()` pipelines get integration coverage against deterministic throwaway git repos.

Sub-phase 2a — seam refactor (implementer → validator):
- New `lib/git-runner.js`: a single `runGit(command, options)` thin wrapper over `execSync` (encoding `utf8`, `stdio: 'pipe'`, optional `cwd`), plus a documented injection hook: call sites accept an optional runner parameter (or module-level override) so tests can substitute a fake without touching `child_process`.
- Behavior-preserving rewires — the 5 git sites: `lib/utils.js:66`, `lib/utils.js:208`, `lib/snitch.js:309`, `lib/snitch.js:364`, `lib/scattered.js:486`. No logic changes, no formatting churn. Note for the implementer: `lib/snitch.js` line 377 does a lazy `require('./utils')` inside `run()` — hoisting it to the top-level requires is acceptable if behavior-identical.
- `lib/templates.js:1048` (`openInBrowser`) stays out of the git seam — P3; optionally give it its own `openInBrowser` injection later.
- Phase-2a gate: `npm test` still green (seeds must not move); `grep -n "execSync" lib/ bin/` returns hits **only** in `lib/git-runner.js` (+ `lib/templates.js` openInBrowser); `node bin/cli.js` help still works.

Sub-phase 2b — integration tests (implementer → validator, then phase-wide validator over 2a+2b):
- `test/helpers/git-fixture.js`: creates a temp repo via `fs.mkdtemp` + `git init`, sets local `user.name`/`user.email`, makes N deterministic commits using fixed `GIT_AUTHOR_DATE`/`GIT_COMMITTER_DATE`, files with known line counts (so `--numstat` math is assertable), an optional second branch and a `remote.origin.url`.
- `test/snitch.run.test.js`: run the snitch pipeline end-to-end in the fixture repo with `--output` into the tmpdir (never opening a browser) → assert the HTML file is created, is non-trivial, contains known commit hashes/authors/stats.
- `test/scattered.run.test.js`: same over 2–3 fixture repos → assert comparison output written.
- `test/git-runner.test.js`: fake-runner injection proves the seam (command strings passed through, cwd honored, non-zero exits surface as the call sites expect).
- Tests skip gracefully with a clear message only if `git` itself is absent from PATH (CI images have git; local robustness, not a fake pass).

Validation gate (validator — reads the diff and the harness): `npm test` green including integration suite; injected-fake test proves call sites go through the seam; fixture repos are created under `os.tmpdir()` and cleaned up; no test writes into the repo tree; total runtime reasonable (< ~30s).

### Phase 3 — Golden-file tests for HTML generation (effort: M)

**Goal**: pin the 2,685 LOC of `templates.js` + `charts.js` (46% of the codebase) behind committed golden outputs. Parallelizable with Phase 2 (touches disjoint files; golden fixtures use `reportData` objects directly, no git needed).

Tasks (implementer):
- `test/golden/reportData.fixture.js`: one representative `reportData` (commits, users, locDetails, options) exercising most chart generators.
- `test/templates.golden.test.js` / `test/charts.golden.test.js`: render each exported generator; concatenate `generateHTMLDocument` output; compare against committed goldens in `test/golden/`.
- Determinism pass (required): audit the rendered HTML for embedded `new Date()`/timestamps/locale-dependent formatting; normalize (inject a fixed clock argument where the function accepts one, or canonicalize known-volatile substrings before compare) so goldens are stable across machines. Any injected-clock parameter must default to the current behavior.
- Regeneration flow: `UPDATE_GOLDEN=1 npm test` rewrites goldens; documented in AGENTS.md (Phase 6) with the rule: goldens change only in a commit whose diff shows the intended output change.
- Keep goldens readable: one file per generator group rather than one monolith, so diffs localize.

Validation gate (validator): `npm test` green including goldens; deliberately mutate one generator string locally → golden test fails (mutation check), revert → passes; `UPDATE_GOLDEN=1` round-trip works; two consecutive runs byte-identical (determinism check).

### Phase 4 — ESLint flat config (effort: S)

**Goal**: lint installed, zero errors, wired as `npm run lint` (the JS analogue of the fleet's check-types gate).

Tasks (implementer):
- devDependencies (minimal, npm): `eslint@^9`, `@eslint/js`, `globals` — nothing else.
- `eslint.config.js` (CommonJS): `@eslint/js` recommended; `languageOptions.ecmaVersion: "latest"`, `sourceType: "commonjs"`, node `globals`; ignores: `node_modules/`, `coverage/`, `test/golden/`, `package-lock.json`.
- `package.json`: `"lint": "eslint ."`; `"test"` stays the headless gate.
- Fix every reported error **at the cause** — expected finds in a legacy codebase: `no-case-declarations` in `parseArgs`'s switch (fix: braces around the case blocks), unused variables/imports (fix: remove), `no-unused-vars` on caught `error` params in intentional swallow paths (fix: rename to `error` with `caughtErrors` handled per rule config documented in the config file — choose the smallest config that reflects intent, never blanket-disable the rule).
- Policy: no rule silently disabled. Any downgrade needs a comment in `eslint.config.js` stating why, scoped as narrowly as possible.

Validation gate (validator — reads the config and each fix diff): `npm run lint` exits 0 with zero errors and zero suppress-happy config; `npm test` still green (fixes must not change behavior); spot-check that behavior-changing "fixes" would have been caught (any fix that alters runtime behavior must be flagged, reverted, and re-done as a true no-op or accompanied by a new test).

### Phase 5 — Local gate composition (effort: S)

**Goal**: one headless local command gates lint + tests — the fleet's single biggest gap (audit P1), realized locally per D5.

Tasks (implementer):
- `package.json`: `"verify": "npm run lint && npm test"` — the headless verification entrypoint (D5).
- Coverage stays a report-only on-demand local command: `node --test --experimental-test-coverage` documented in AGENTS.md (Phase 6), not wired into `verify` by default (keeps the fast path fast; D2 state 3).
- No workflow file is created anywhere; nothing depends on `DimitriGilbert/.github` (D5).

Validation gate (validator): `npm run verify` exits 0 locally and both sub-commands exist in `package.json` scripts; fail-fast proven once (validator introduces a failing assertion in a scratch copy and confirms `verify` exits non-zero at the failing step); the implementer/validator agents record the green output in the phase report before the phase is declared done (D5); no workflow file exists.

### Phase 6 — Renovate, knip (report-only), AGENTS.md (effort: S)

**Goal**: DoD items 1, 5, 7, 8 closed; docs match reality; the repo is left maintainable.

Tasks (implementer):
- `renovate.json`: `{"$schema": "https://docs.renovatebot.com/renovate-schema.json", "extends": ["local>DimitriGilbert"]}` (D4 org-preset one-liner). Confirms no `"latest"` deps (none exist today).
- knip report-only: devDep `knip` + `"quality:knip": "knip"` with a minimal `knip` config (entry: `bin/cli.js`, project: `lib/**/*.js`, test files ignored as entry-adjacent); **report-only — never a gate in v1** (D4: nothing gated on an unmeasured number). Surface the first report's findings as information, not fixes.
- `AGENTS.md` (new): Testing section — runner (`node --test`), command (`npm test`, `npm run lint`, `npm run verify`), layout (colocated `lib/*.test.js` + `test/`), integration/golden strategies, `UPDATE_GOLDEN=1` flow, coverage status + measured baseline (D2), knip report-only; gate section — "the headless local verification command (`npm run verify`) is the quality gate, run by the owner/agents; hooks are local convenience" (D7), no hooks installed; the engines floor (Node ≥ 18); a pointer that AGENTS.md conventions here are this repo's own (not BTS-derived).
- Verify `.npmignore` still keeps tests, goldens, coverage out of the packed tarball (`npm pack --dry-run` inspection).

Validation gate (validator — final full gate): `npm ci && npm run lint && npm test` all green; `renovate.json` parses and matches the preset convention; **AGENTS.md audited claim-by-claim against the repo** (no phantom docs — DoD 5); `npm pack --dry-run` contains no `*.test.js`, `test/`, or `coverage/`.

---

## 5. Risks & notes specific to this repo

- **execSync seam strategy**: the seam is deliberately tiny — one `runGit()` wrapper plus an injection parameter; tests use (a) fake runners for unit-level seam proofs and (b) real `git` in `fs.mkdtemp` repos with pinned dates for integration. Mocking git output strings wholesale is rejected: it would freeze today's command construction (string-concatenated `git log …`) into the mocks; the integration fixtures catch real command regressions instead. Shell-escaping of user-supplied options (`--branch`, `--start-date` interpolated into command strings, e.g. `snitch.js:298-307`) is worth a dedicated look once the seam exists — options flow into `execSync` strings today.
- **Naming swap — FLAGGED, NOT EXECUTED**: `package.json` name/bin/`repository.url`/`bugs`/`homepage` say `git-moar`; the directory and git remote say `git-snitch`; sibling repo `/home/didi/workspace/gitsnitch` packages itself as `git-snitch` (a TS/BTS CLI). This is a pre-existing owner-level hygiene decision (synthesis §5); **no phase of this plan renames anything, and no publishing step may proceed before the collision is resolved**. The only metadata this plan touches is `author` (Phase 0) and `engines` (Phase 0).
- **Lazy `require('./utils')`** inside `lib/snitch.js` `run()` (line 377) — keep or hoist during Phase 2a, but do not let the refactor silently change module-load side effects (`lib/config.js` exports a singleton that may read/write config at require time; `config.test.js` must cover that).
- **Golden determinism**: HTML output may embed dates/locale-formatted numbers; the determinism pass in Phase 3 is load-bearing — a flaky golden suite would be worse than none.
- **No shared-workflow dependency**: under D5 there is none — the gate is the local `npm run verify` and nothing depends on `DimitriGilbert/.github`.
- **Dependency aging (P3, out of scope)**: `sloc ^0.3.2` is effectively unmaintained and `chalk` is pinned to CJS-era v4. Neither is swapped in this effort (synthesis non-goal: don't churn what works); Renovate (Phase 6) will surface them. `getLinesOfCode` already defends against `sloc` failures.
- **Fake-pass ban is the headline**: DoD 9 is violated by the repo's current `test` script; Phase 1 removes it in the same change that introduces the real runner so `npm test` is never absent and never fake.

## 6. Non-goals

- No TypeScript migration, no bundler, no package-manager switch (npm stays), no monorepo conversion.
- No renaming of package/dir/bin (owner decision, see risk above); no `npm publish`.
- No Prettier in v1 (deferred and stated in AGENTS.md, per D3).
- No coverage thresholds, jscpd gates, or CRAP gates (D2 state 3; D4 report-only).
- No git hooks (D7).
- No replacement of `chalk`/`sloc`.

## 7. Effort summary

| Phase | Goal | Effort | Role flow |
|---|---|---|---|
| 0 | Preflight hygiene | S | implementer → validator |
| 1 | Real runner + pure-logic seeds | M | 1–2 parallel implementers → per-slice validators → phase-wide validator |
| 2 | execSync seam + integration tests | L | 2a implementer → validator; 2b implementer → validator → phase-wide validator |
| 3 | Golden files for templates/charts | M | implementer → validator (parallelizable with 2) |
| 4 | ESLint flat config | S | implementer → validator |
| 5 | Local gate composition | S | implementer → validator |
| 6 | Renovate + knip + AGENTS.md | S | implementer → validator (final full gate) |

Total: ~3–4 focused sessions. The repo ends at full DoD compliance modulo the flagged owner decision (naming swap).

Inizia da package.json e dalla checklist delle Fasi 0–1; esegui npm ci, verifica che Node sia almeno alla versione 18 e ispeziona i moduli di pura logica indicati e i file di test pianificati. Prosegui attraverso le fasi elencate, usando npm test e npm run verify come controlli. Il lavoro è completato quando sono presenti una copertura reale con node:test, il lint e i comandi di verifica locale, insieme alla baseline di qualità documentata, senza CI.

Scritto dal modello di indicizzazione a partire dal testo della issue.