[#199] chore: resolve audit findings T1-T10 (tech-debt ledger)

.github/workflows/ci.yml

@@ -54,16 +54,24 @@ jobs:
             export $(grep -v '^#' .env | xargs)
           fi
       - name: Run type check
-        run: pnpm ts:check || echo "No ts:check script defined"
+        run: pnpm ts:check
       - name: Run build
-        run: pnpm build || echo "No build script defined"
+        run: pnpm build
       - name: Run lint
-        run: pnpm lint || echo "No lint script defined"
+        run: pnpm lint
+      - name: Run code-hygiene check
+        run: pnpm hygiene:check
+      - name: Run docs-staleness check
+        run: pnpm docs:staleness
+      - name: Run duplication check
+        run: pnpm dup:check
       - name: Install Playwright browsers
         run: |
           pnpm --filter @pair/brand exec playwright install --with-deps chromium
           pnpm --filter @pair/website exec playwright install --with-deps chromium
       - name: Run tests
         run: pnpm test
+      - name: Run coverage thresholds (brand, website)
+        run: pnpm turbo test:coverage --filter=@pair/brand --filter=@pair/website
       - name: Run E2E tests
         run: pnpm --filter @pair/website e2e

.jscpd.json

@@ -0,0 +1,18 @@
+{
+  "threshold": 5,
+  "minTokens": 50,
+  "format": ["typescript", "tsx"],
+  "reporters": ["console"],
+  "gitignore": true,
+  "ignore": [
+    "**/*.test.ts",
+    "**/*.test.tsx",
+    "**/*.ct.test.tsx",
+    "**/dist/**",
+    "**/node_modules/**",
+    "**/coverage/**",
+    "**/*.d.ts",
+    "**/.next/**",
+    "**/*.config.ts"
+  ]
+}

.pair/adoption/decision-log/2026-07-08-test-file-colocation-multi-module.md

@@ -24,6 +24,14 @@ Applied to `idempotent-skill-registry.test.ts`: its `update`-focused cases (idem
 
 This does not apply to end-to-end/page-level tests (named after the user flow or page they exercise, e.g. `landing.e2e.test.ts`) or content/asset-validation tests (named after the asset they validate, e.g. `agents-md.test.ts`, which has no single source module to co-locate against) — both are pre-existing, distinct categories this decision leaves untouched.
 
+**Amendment (2026-07-12, PR #311 review)** — SUPERSEDED, see the 2026-07-12 (#199) amendment below: the e2e exemption above was descriptive ("named after the user flow") but not mechanically checkable — a reviewer could not verify compliance without judgment. Made explicit with three checkable criteria, all of which must hold for a `*.e2e.test.ts` file to qualify:
+
+1. **Named after a real flow**: the file name corresponds to an identifiable user-facing flow or CLI command, not an arbitrary grouping of convenience.
+2. **Additive, not sole coverage**: every module/command the e2e file exercises also has its own co-located unit test file covering its non-integration behavior — the e2e file must be additive cross-cutting coverage, never the only coverage for a module.
+3. **Genuine cross-module interaction**: the file exercises more than one command/module with a real interaction between them. An e2e-named file touching exactly one module with no cross-module flow is a signal it should be a regular unit test in that module's own test file instead, not evidence for the exemption.
+
+**Amendment (2026-07-12, #199 test reorg) — corrects the amendment above**: the three-criteria version was applied to `apps/pair-cli`'s 7-file e2e split (`cli-errors`, `cli-install`, `cli-kb-validate`, `cli-link`, `cli-packaging`, `cli-update`, `cli-validate-config` — all `.e2e.test.ts`) and looked compliant on paper, but a per-test classification of all 51 tests across those 7 files found only 1 was genuinely e2e (a real hand-off of state between independently invoked commands: install → update → update-link against the same disjoint target). The other 50 exercised exactly one command's handler each, with no real cross-command interaction — single-module tests wearing e2e clothing. The criteria were too easy to satisfy without the underlying flows actually being cross-module. Corrected rule: **e2e tests default to one file per application entry point** (e.g. `cli.e2e.test.ts`); splitting is valid only when the production code has genuinely been refactored into isolated modules and the corresponding tests have become true unit tests (not staying e2e-shaped). The 7 files were consolidated back into a single `cli.e2e.test.ts` containing only the one genuinely cross-command test; duplicates of existing unit tests were deleted and the remaining genuine gaps were moved into the relevant `commands/*/handler.test.ts` / `registry/validation.test.ts` files. See `file-structure.md`'s Co-location Rules for the corrected rule text.
+
 ## Alternatives Considered
 
 - **Keep standalone integration-test files, named after the scenario**: Rejected. Scales into a parallel "integration tests" file tree that duplicates or drifts from the sibling `handler.test.ts` files already covering the same root modules, and breaks the "one root module → one test file" discoverability the co-location rule exists for.
@@ -38,3 +46,6 @@ This does not apply to end-to-end/page-level tests (named after the user flow or
 
 - `packages/knowledge-hub/dataset/.pair/knowledge/guidelines/code-design/code-organization/file-structure.md` (+ root mirror `.pair/knowledge/guidelines/code-design/code-organization/file-structure.md`): Co-location Rules section extended with the multi-module case and its two exceptions.
 - `apps/pair-cli/src/commands/update/idempotent-skill-registry.test.ts`: deleted; its cases moved into `update/handler.test.ts` and `install/handler.test.ts`.
+- **(#199 test reorg)** `packages/knowledge-hub/dataset/.pair/knowledge/guidelines/code-design/code-organization/file-structure.md` (+ root mirror): e2e exemption section replaced with the corrected one-file-per-entry-point rule and the checkable "genuine cross-module hand-off" test.
+- `apps/pair-cli/src/cli-errors.e2e.test.ts`, `cli-install.e2e.test.ts`, `cli-kb-validate.e2e.test.ts`, `cli-link.e2e.test.ts`, `cli-packaging.e2e.test.ts`, `cli-update.e2e.test.ts`, `cli-validate-config.e2e.test.ts`: deleted (51 tests); consolidated into a new `apps/pair-cli/src/cli.e2e.test.ts` (1 genuinely cross-command test kept), 15 genuine gaps moved into `commands/install/handler.test.ts`, `commands/update/handler.test.ts`, `commands/kb-validate/handler.test.ts`, `registry/validation.test.ts`, and 35 exact/near duplicates deleted.
+- `apps/pair-cli/src/test-utils/cli-e2e-helpers.ts`: deleted (no remaining consumers after the 7 files above were removed).

.pair/adoption/decision-log/2026-07-12-one-pr-per-story-default.md

@@ -0,0 +1,63 @@
+# Decision: One PR per story is the default
+
+## Date
+
+2026-07-12
+
+## Status
+
+Active
+
+## Category
+
+Process Decision
+
+## Context
+
+Story #199 (tech-debt ledger) was broken into inline tasks T1-T10 within a
+single story issue. All ten tasks landed in one PR (#311). The review flagged
+this as a deviation from the story's own "suggested batching" wording (which
+implied smaller, incremental PRs per task or group of tasks).
+
+The user confirmed, explicitly and generally (not just for #199), that
+bundling a story's tasks into a single PR is correct and should be the
+default going forward — not an exception that needs justifying case by case.
+
+## Decision
+
+A story's work lands in a single PR by default, even when the story is
+broken into multiple inline tasks/findings. Splitting a story's work across
+multiple PRs requires an explicit reason (e.g. the story is unusually large,
+or parts of it are independently shippable/needed sooner) — it is not the
+default.
+
+The adopted convention is recorded in
+`.pair/adoption/tech/way-of-working.md` (Delivery / PR granularity section);
+this ADL is the historical record of why.
+
+## Alternatives Considered
+
+- **Task-per-PR granularity**: rejected — more review overhead (reviewer
+  re-establishes context per PR), more merge-order coordination between
+  dependent tasks, and no clear benefit when tasks are tightly related within
+  one story.
+- **Case-by-case judgment call with no default**: rejected — leaves
+  implementers guessing at review time whether batching will be flagged;
+  a stated default removes the ambiguity while still allowing exceptions.
+
+## Consequences
+
+- Reviewers see the full story diff at once — larger review surface per PR,
+  but full context in one pass instead of piecemeal.
+- Large stories may need a bigger review budget per PR; acceptable tradeoff
+  per user direction.
+- Story/task templates that read as "suggested batching" per task should be
+  read as suggested internal commit structure (commit per task, per existing
+  Commit History Policy in way-of-working.md), not as a mandate for separate
+  PRs.
+
+## Adoption Impact
+
+- `.pair/adoption/tech/way-of-working.md`: added a PR granularity statement
+  under the delivery/commit-history guidance making single-PR-per-story the
+  default.

.pair/adoption/decision-log/2026-07-12-website-coverage-baseline.md

@@ -0,0 +1,88 @@
+# Decision: Website coverage threshold is a realistic baseline, not 80%
+
+## Date
+
+2026-07-12
+
+## Status
+
+Active (amended 2026-07-12 — see Update below: thresholds now gate CI)
+
+## Category
+
+Process Decision
+
+## Context
+
+Story #199 (tech-debt ledger), finding P1.2, asked to add 80% coverage
+thresholds to `apps/website` and `packages/brand` vitest configs to match the
+`@pair/content-ops` bar.
+
+`packages/brand` measures 84.5% lines with the dev harness included (its
+`dev/App.tsx` is unit-tested at 100%), so it takes the 80% gate cleanly.
+
+`apps/website` measures ~9.9% lines. It is a Next.js documentation app whose
+behavior is validated primarily by Playwright component (`ct`) and end-to-end
+(`e2e`) suites, not by jsdom unit tests. Forcing an 80% unit-coverage gate would
+either break `pnpm --filter @pair/website test:coverage` (and CI once wired) or
+push low-value unit tests onto UI already covered by e2e.
+
+Note: the quality-gate command runs `test`, not `test:coverage`, so these
+thresholds bite only on explicit coverage runs / trend tracking — they do not
+gate normal PRs.
+
+## Decision
+
+- `packages/brand`: 80% global thresholds (lines/statements/functions/branches).
+- `apps/website`: realistic baseline floors instead of 80% —
+  lines 9, statements 9, functions 40, branches 60 — set just below current
+  measured coverage (9.9 / 9.9 / 47.2 / 66.2). These catch regressions without
+  mandating unit tests for e2e-covered UI.
+- A committed baseline (`reports/coverage-baseline.json`, P1.4) records current
+  totals for all three packages for trend tracking. Raise the website floors
+  toward the 80% target as unit coverage of testable (non-page) modules grows.
+
+## Alternatives Considered
+
+- **Force 80% on website now**: rejected — breaks the coverage command and
+  incentivizes low-value unit tests over the existing Playwright coverage.
+- **No threshold on website**: rejected — loses regression protection; a floor
+  at current level is cheap and meaningful.
+- **Count only shipped code on brand (exclude dev/)**: rejected — brand's
+  dev harness is genuinely unit-tested (`dev/App.test.tsx`), so including it
+  reflects real coverage; excluding it understates brand at 76.8%.
+
+## Consequences
+
+- `apps/website/vitest.config.ts` and `packages/brand/vitest.config.ts` gain a
+  `coverage` block extending `coverageConfigDefaults.exclude`, emitting a
+  `json-summary` reporter, with the thresholds above.
+- `reports/coverage-baseline.json` is the committed trend baseline.
+- Website's floor is a debt marker, not the target; revisit when raising it.
+
+## Adoption Impact
+
+- No change to `adoption/tech/tech-stack.md` (vitest + @vitest/coverage-v8
+  already adopted). This ADL is the record of the per-package threshold policy.
+
+## Update — 2026-07-12: thresholds now gate CI (reverses "bite only on explicit runs")
+
+Original text above ("Note: the quality-gate command runs `test`, not
+`test:coverage`, so these thresholds bite only on explicit coverage runs /
+trend tracking — they do not gate normal PRs.") is superseded per explicit
+user direction: silent coverage regression is not acceptable even for a low
+baseline floor.
+
+- `.github/workflows/ci.yml` gained a `Run coverage thresholds (brand,
+  website)` step running `pnpm turbo test:coverage --filter=@pair/brand
+  --filter=@pair/website` after the existing `Run tests` step. A regression
+  below either package's `coverage.thresholds` now fails the CI job (vitest's
+  v8 coverage provider exits non-zero when a configured threshold is
+  violated).
+- The baseline floors themselves (website: 9/9/40/60, brand: 80/80/80/80) are
+  unchanged by this update — only their enforcement moved from "trend-only"
+  to "PR-gating".
+- Adoption Impact update: `.pair/adoption/tech/way-of-working.md` Quality
+  Gates registry is unchanged (CI is a separate gate from `pnpm quality-gate`
+  in this project's convention); this ADL plus the CI diff is the record of
+  the enforcement change.

.pair/adoption/tech/tech-stack.md

@@ -88,6 +88,7 @@ Use `turbo` from the repository root to run cross-workspace tasks (e.g. `turbo b
   - globals v15.0.0
   - prettier v3.6.2 (configured via workspace `tools/prettier-config`)
   - markdownlint-cli v0.47.0 (configured via workspace `tools/markdownlint-config`, wired into `pnpm quality-gate` via `mdlint:check` / `mdlint:fix`)
+  - jscpd v5.0.12 (copy/paste duplication scanner, configured via `.jscpd.json`, wired into `pnpm quality-gate` via `dup:check`; current threshold 5% — baseline ~1.4%)
 
 ## Git hooks
 

.pair/adoption/tech/way-of-working.md

@@ -9,6 +9,7 @@
 - Team communication is informal and direct, with decisions validated collaboratively.
 - **Commit History Policy:**: All feature branches must be squashed into a single commit during the PR merge, unless otherwise specified by the story or epic. See [commit template](../../knowledge/guidelines/collaboration/templates/commit-template.md) for details. Unless specified, prefer commit per task (mark the commit title with the task number other than the user story number) where complete all tasks of the story without confirmation and update the body of the story at each commit without confirmation. At the end of the story raise a draft PR following the PR template.
 - Ensure use proper template for commit messages and PRs, see [commit template](../../knowledge/guidelines/collaboration/templates/commit-template.md) and [PR template](../../knowledge/guidelines/collaboration/templates/pr-template.md) for details.
+- **PR granularity — one PR per story (default):** A story's work lands in a single PR by default, even when the story is broken into multiple inline tasks/findings. Splitting into multiple PRs per story requires an explicit reason (e.g. unusually large story, or independently shippable/needed-sooner parts) — it is not the default. Per-task granularity within that one PR is expressed as commit-per-task (see Commit History Policy above), not as separate PRs. See ADL [2026-07-12-one-pr-per-story-default.md](../decision-log/2026-07-12-one-pr-per-story-default.md) for rationale.
 
 ## Manual Testing
 

.pair/knowledge/guidelines/code-design/code-organization/file-structure.md

@@ -50,7 +50,17 @@ UserProfile.stories.tsx (if using Storybook)
 
 **Tests spanning multiple modules**: when a test exercises more than one implementation file together (e.g. an integration test driving two collaborating functions end-to-end), it does not get its own standalone file — it goes in the test file already co-located with the *root* module of that call chain, the one whose exported entry point the test is actually verifying. A test that seeds its fixture through module B but asserts on module A's behavior belongs in `A.test.ts`, not a new `A-and-b-integration.test.ts`. This keeps the co-location rule intact (one root module → one test file) instead of a proliferation of ad hoc integration-test files that duplicate coverage or drift out of sync with the modules they actually exercise.
 
-This does not apply to two other, already-common test categories that are correctly named after what they validate rather than a source module: end-to-end/page-level tests (e.g. `landing.e2e.test.ts`, testing a user flow across many files by design) and content/asset-validation tests (e.g. asserting on a generated markdown file's content, where there is no single source module to co-locate against).
+This does not apply to two other, already-common test categories that are correctly named after what they validate rather than a source module: end-to-end/page-level tests (e.g. `cli.e2e.test.ts`, testing a real flow across many files by design) and content/asset-validation tests (e.g. asserting on a generated markdown file's content, where there is no single source module to co-locate against).
+
+**Amendment (2026-07-12, #199 test reorg)**: an earlier version of this rule set out three "checkable criteria" under which an application entry point's e2e coverage could be split into several `*.e2e.test.ts` files (one per command/flow). That split was applied to `apps/pair-cli` and looked compliant against the letter of those criteria, but a subsequent per-test classification of all 51 tests across the resulting 7 files found only 1 was genuinely e2e — depending on a real hand-off of state between independently invoked commands. The other 50 were single-module tests wearing e2e clothing: each exercised exactly one command's handler, with no real cross-command interaction, and would have been (and, per this reorg, now are) equally at home as regular tests in that handler's own co-located test file. The three-criteria version of the rule was too easy to satisfy on paper without the underlying flows actually being cross-module.
+
+The rule is corrected to this, tighter form:
+
+- **e2e tests default to one file per application entry point** (e.g. `cli.e2e.test.ts` for `apps/pair-cli`, `app.e2e.test.ts` for a web app). Do not split it by module or command as a matter of course.
+- **Splitting an entry point's e2e file is valid only when the production code itself has been genuinely refactored into isolated modules, and the corresponding tests for those modules have become true unit tests** (asserting a single module's behavior, not staying e2e-shaped with a full command/flow setup). If the production code was not restructured, the tests should not be either — that's a signal the "split" is just moving tests around, not truly isolating them.
+- A test belongs in the one e2e file if it depends on real, observable hand-off of state between independently invoked units (e.g. command A's output becoming command B's input against the same target). If a test only exercises one command/module — even if named `*.e2e.test.ts` — it is a unit test for that module and belongs in its co-located `*.test.ts` file instead.
+
+Concrete precedent: `apps/pair-cli`'s prior 7-file e2e split (`cli-errors`, `cli-install`, `cli-kb-validate`, `cli-link`, `cli-packaging`, `cli-update`, `cli-validate-config` — all `.e2e.test.ts`) was consolidated back into a single `cli.e2e.test.ts` containing only the one test with a genuine cross-command dependency (install → update → update-link against the same disjoint target). Duplicates of existing unit tests were deleted; the remaining genuine coverage gaps were moved into the relevant handler/module `*.test.ts` files (e.g. `commands/install/handler.test.ts`, `commands/update/handler.test.ts`, `registry/validation.test.ts`).
 
 **Types**: Co-locate types when feature-specific:
 ```

.pair/knowledge/guidelines/testing/e2e-testing/README.md

@@ -30,6 +30,8 @@ E2E tests are the most comprehensive but also the most expensive to maintain. Th
 
 Effective E2E testing requires careful test design, robust test infrastructure, and clear understanding of user behavior patterns. These tests serve as living documentation of how the system should behave from a user's perspective.
 
+**File organization**: a large e2e file, or many of them, is a signal — not a target. Genuine e2e tests depend on a real hand-off between independently-invoked components; anything that only exercises one component belongs in that component's own unit tests. See [file-structure.md](../../code-design/code-organization/file-structure.md)'s co-location rules for the file-per-entry-point default and when splitting is actually justified.
+
 ## Framework Comparison
 
 | Framework      | Best For                             | Learning Curve | Debugging | Ecosystem       |