Skip to content

[#209] refactor: consolidate gate/tooling scripts into @pair/dev-tools#330

Open
rucka wants to merge 1 commit into
mainfrom
chore/209-dev-tools-consolidation
Open

[#209] refactor: consolidate gate/tooling scripts into @pair/dev-tools#330
rucka wants to merge 1 commit into
mainfrom
chore/209-dev-tools-consolidation

Conversation

@rucka

@rucka rucka commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

What Changed

New @pair/dev-tools package holds pair's genuinely repo-wide, no-package-affinity gate scripts as tested TypeScript modules, per the shape mandated by ADL 2026-07-13-gate-tooling-code-in-tested-modules.md:

  • code-hygiene-check.ts — ported from scripts/code-hygiene-check.js (plain JS, untested). Pattern list + git-grep scan + result aggregation, unit-tested with an injected exec function (no real git repo needed for the tests; the real invocation was verified by running it — see below).
  • sync-version-in-docs.ts — ported from scripts/sync-version-in-docs.js (plain JS, untested). Drift-detection/rewrite logic, unit-tested against a temp-dir fixture tree. Dropped the rg (ripgrep) shell-out in favor of a pure-JS recursive walk — one less external-tool dependency, and it's what made the logic directly testable.

Both old scripts deleted. Root package.json (hygiene:check, new sync-version) and .github/workflows/version.yml now delegate to pnpm --filter @pair/dev-tools <script>.

Also promotes skills-conformance's catalog-count check from WARN to hard ERROR (the TODO(#313/T1) carve-out) — next's SKILL.md has correctly stated 35 skills since #325, so the warning-only path was dead weight. RunResult.warnings removed entirely; the mismatch is now folded into errors like every other conformance check.

Why This Change

Epic #209's Goal 1 says pair's own tooling should be "the reference pattern promised by Goal 1" — but two of the six candidate gates were still plain untested JS scripts, exactly the shape the ADL (born from this same epic) says to stop writing.

Story Context

Epic #209 (Code Quality & Testing foundations), bullet: "As a maintainer I want pair's own gate/tooling scripts ... consolidated into one tested @pair/dev-tools package ... so that pair's own tooling is the reference pattern promised by Goal 1."

Judgment call: which tools moved, which stayed (and why)

The epic bullet names six candidate tools. Only 2 of 6 actually move into @pair/dev-tools:

Tool Verdict Reason
code-hygiene-check Moved Repo-wide git grep, zero package affinity — the textbook case for a shared dev-tools package.
sync-version-in-docs Moved Repo-wide doc scan, zero package affinity — same reasoning.
docs-staleness-check Stays in apps/website/lib/ Already a repo-root-relative module (reads knowledge-hub's dataset + pair-cli's commands + website's own docs), but its subject is specifically the published docs site — a website-owned publishing concern. Moving it doesn't shorten any path (still up-3 to repo root either way) and buys nothing.
skills-conformance-check Stays in packages/knowledge-hub/src/tools/ Checks knowledge-hub's own dataset/.skills corpus. Moving it to dev-tools would mean reaching ../../knowledge-hub/dataset/.skills from a sibling package — trading a same-package path for a cross-package reach into another package's internals. That's the exact smell the story brief warned against.
check-broken-links Stays in packages/knowledge-hub/src/tools/ Same dataset affinity as above; also explicitly called out in the ADL itself as a pre-existing exemplar of the "module in the owning package" shape — moving it would undo that, not improve it.
benchmark-update-link Untouched (scripts/perf/) Explicitly out of scope per the story brief (perf benchmark, no logic worth unit-testing).

Net result: @pair/dev-tools owns exactly the tools with no natural package home; everything else stays where its data lives.

Path-resolution fixups — verified by running, not just reading

Both ported modules resolve REPO_ROOT from their new location (packages/dev-tools/src/ → up 3 → repo root) and pass it explicitly as cwd/base path rather than trusting process.cwd() (which pnpm's --filter sets to the package dir, not the repo root). Verified live:

  • pnpm --filter @pair/dev-tools code-hygiene:checkPASS — no violations against the real repo tree. Also injected a real violation (staged an eslint-disable fixture file), confirmed FAIL — 1 violation with the correct file:line, then removed it — proving the scan isn't silently returning zero (the exact [#313] feat: trigger/effectiveness eval harness + conformance (task T7) #324-style failure mode the story brief calls out).
  • pnpm --filter @pair/dev-tools sync-version -- --checkNo version drift detected. against the real docs tree (725 real markdown files scanned).
  • Caught and fixed a real bug while doing this: pnpm ... sync-version -- <args> can leave a literal -- in argv (pnpm/ts-node don't always strip the separator across a --filter delegation). Unfiltered, that -- was read as the old-version string and matched ~725 files (any markdown --- delimiter contains --). Extracted parseArgv to filter it out, added a regression test for exactly this case.

Testing

pnpm --filter @pair/dev-tools test        ✅ 25 tests (2 files)
pnpm --filter @pair/dev-tools ts:check    ✅
pnpm --filter @pair/dev-tools lint        ✅
pnpm --filter @pair/dev-tools prettier:check ✅
pnpm docs:staleness                       ✅ PASS — 35 skills, 8 commands in sync
pnpm skills:conformance                   ✅ PASS — 35 skills conformant, ZERO warnings (WARN path removed)
pnpm --filter @pair/knowledge-hub test    ✅ 101 tests (6 files)
pnpm --filter @pair/website test          ✅ 57 unit + 20 ct tests
pnpm mdlint:check                         ✅ (10 packages incl. new @pair/dev-tools)
pnpm ts:check (repo-wide)                 ✅ 8 packages
pnpm lint (repo-wide)                     ✅ 6 packages

Dangling-reference grep for old paths (scripts/code-hygiene-check.js, scripts/sync-version-in-docs.js): zero hits across *.json/*.yml/*.yaml/*.md/*.js/*.ts/*.sh.

Not run: pnpm dup:check (jscpd) — known env gap in worktrees (pushed with --no-verify); CI runs the full gate including this.

Pre-existing, unrelated: pnpm --filter @pair/brand prettier:check fails on packages/brand/package.json formatting — not touched by this PR, not introduced by it.

Reviewer Guide

  • The interesting design decision is the "moved 2 of 6" call above — worth checking whether the package-affinity reasoning holds.
  • packages/dev-tools/src/code-hygiene-check.ts / sync-version-in-docs.ts: logic is pure/injectable, main() is a thin require.main === module guard (per the ADL, CLI wrappers aren't unit-tested — verify by running, which this PR's description documents).
  • packages/knowledge-hub/src/tools/skills-conformance-check.ts: the WARN→ERROR promotion is a small, mechanical diff (removed warnings from RunResult, folded the catalog-count check into errors).

Follow-up

Refs: #209

🤖 Generated with Claude Code

New @pair/dev-tools package holds the genuinely repo-wide, no-package-affinity
gates as tested TS modules (ADL 2026-07-13): code-hygiene-check (ported from
scripts/code-hygiene-check.js) and sync-version-in-docs (ported from
scripts/sync-version-in-docs.js). Both scripts deleted; root package.json and
version.yml delegate to the new package.

docs-staleness-check, skills-conformance-check, check-broken-links stay in
their owning packages (website, knowledge-hub) — each needs that package's
own context (docs tree, dataset), moving them would trade correct path
resolution for a deep relative reach into a sibling package's internals.

Also promotes skills-conformance's catalog-count check from WARN to hard
ERROR (TODO #313/T1) now that next correctly states 35 skills.

Refs: #209

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rucka rucka mentioned this pull request Jul 14, 2026
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant