Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .claude/skills/ci-prep/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: ci-prep
description: Prepares the current branch for CI by running the exact same steps locally and fixing issues. If CI is already failing, fetches the GH Actions logs first to diagnose. Use before pushing, when CI is red, or when the user says "fix ci".
argument-hint: "[--failing] [optional job name to focus on]"
---
<!-- agent-pmo:5547fd2 -->
<!-- agent-pmo:424c8f8 -->

# CI Prep

Expand Down Expand Up @@ -42,12 +42,12 @@ Read **every line** of `--log-failed` output. For each failure note the exact fi

## Step 2 — Analyze the CI workflow

1. Find the CI workflow file. Look in `.github/workflows/` for `ci.yml`.
1. Find the CI workflow file. Look in `.github/workflows/` for `ci.yml` (this repo's CI workflow).
2. Read the workflow file completely. Parse every job and every step.
3. Extract the ordered list of commands the CI actually runs (e.g., `make fmt-check`, `make lint`, `make spellcheck`, `make test EXCLUDE_CI=true`, `make build`, `make package`).
4. Note any environment variables, matrix strategies, or conditional steps that affect execution.
3. Extract the ordered list of commands the CI actually runs. In this spec-compliant repo that is `make fmt CHECK=1 → make lint → make test make buildmake package` (REPO-STANDARDS-SPEC [MAKE-TARGETS] plus the repo-specific `package` target).
4. Note any environment variables, matrix strategies, or conditional steps that affect execution. The separate `website` job runs Playwright tests against the 11ty site under `website/`.

**Do NOT assume the steps.** Extract what the CI *actually does*.
**Do NOT assume the steps.** Extract what the CI *actually does*. If you find extra targets beyond the 7 in [MAKE-TARGETS] that are not already in the `Repo-Specific Targets` section, flag them — they should be consolidated by the agent-pmo skill.

## Step 3 — Run each CI step locally, in order

Expand All @@ -60,15 +60,15 @@ Work through failures in this priority order:

For each command extracted from the CI workflow:

1. Run the command exactly as CI would run it.
1. Run the command exactly as CI would run it (adjusting only for local environment differences like not needing `actions/checkout`).
2. If the step fails, **stop and fix the issues** before continuing to the next step.
3. After fixing, re-run the same step to confirm it passes.
4. Move to the next step only after the current one succeeds.

### Hard constraints

- **NEVER modify test files** — fix the source code, not the tests
- **NEVER add suppressions** (`// eslint-disable`, `// @ts-ignore`)
- **NEVER add suppressions** (`// eslint-disable`, `@ts-ignore`, `@ts-nocheck`)
- **NEVER use `any` in TypeScript** to silence type errors
- **NEVER delete or ignore failing tests**
- **NEVER remove assertions**
Expand Down Expand Up @@ -97,7 +97,7 @@ Once all CI steps pass locally:
- Fix issues found in each step before moving to the next
- Never skip steps or suppress errors
- If the CI workflow has multiple jobs, run all of them (respecting dependency order)
- Skip steps that are CI-infrastructure-only (checkout, setup-node, cache steps, artifact uploads) — focus on the actual build/test/lint commands
- Skip steps that are CI-infrastructure-only (checkout, setup-node actions, cache steps, artifact uploads) — focus on the actual build/test/lint commands

## Success criteria

Expand Down
54 changes: 29 additions & 25 deletions .claude/skills/code-dedup/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name: code-dedup
description: Searches for duplicate code, duplicate tests, and dead code, then safely merges or removes them. Use when the user says "deduplicate", "find duplicates", "remove dead code", "DRY up", or "code dedup". Requires test coverage — refuses to touch untested code.
---
<!-- agent-pmo:5547fd2 -->
<!-- agent-pmo:424c8f8 -->

# Code Dedup

Expand All @@ -13,8 +13,8 @@ Carefully search for duplicate code, duplicate tests, and dead code across the r
Before touching ANY code, verify these conditions. If any fail, stop and report why.

1. Run `make test` — all tests must pass. If tests fail, stop. Do not dedup a broken codebase.
2. Run `make coverage-check` — coverage must meet the repo's threshold. If it doesn't, stop.
3. Verify the project uses **static typing**. Check `tsconfig.json` has `"strict": true` — proceed.
2. `make test` is fail-fast AND enforces the coverage threshold from `coverage-thresholds.json`. If anything fails, stop and fix it before deduping.
3. This is a TypeScript repo. Confirm `tsconfig.json` has `"strict": true` (it does). Proceed.

## Steps

Expand All @@ -34,16 +34,19 @@ Dedup Progress:

Before deciding what to touch, understand what is tested.

1. Run `make test` and `make coverage-check` to confirm green baseline
2. Note the current coverage percentagethis is the floor. It must not drop.
1. Run `make test` to confirm green baseline. `make test` is fail-fast AND enforces the coverage thresholds from `coverage-thresholds.json` (REPO-STANDARDS-SPEC [TEST-RULES], [COVERAGE-THRESHOLDS-JSON]). It exits non-zero on any test failure OR coverage shortfall.
2. Note the current coverage percentagesthese are the floor. They must not drop.
3. Identify which files/modules have coverage and which do not. Only files WITH coverage are candidates for dedup.

### Step 2 — Scan for dead code

Search for code that is never called, never imported, never referenced.

1. Look for unused exports, unused functions, unused classes, unused variables
2. Check for `noUnusedLocals`/`noUnusedParameters` in tsconfig, look for unexported functions with zero references
2. TypeScript-specific tooling:
- `tsconfig.json` has `noUnusedLocals`/`noUnusedParameters` — the compiler already warns on unused locals/params
- ESLint flags unused exports via `@typescript-eslint/no-unused-vars`
- Look for unexported functions with zero references within the module
3. For each candidate: **grep the entire codebase** for references (including tests, scripts, configs). Only mark as dead if truly zero references.
4. List all dead code found with file paths and line numbers. Do NOT delete yet.

Expand All @@ -54,7 +57,7 @@ Search for code blocks that do the same thing in multiple places.
1. Look for functions/methods with identical or near-identical logic
2. Look for copy-pasted blocks (same structure, maybe different variable names)
3. Look for multiple implementations of the same algorithm or pattern
4. Check across module boundaries — duplicates often hide in different packages
4. Check across discovery providers in `src/discovery/` — duplicates often hide in similar providers
5. For each duplicate pair: note both locations, what they do, and how they differ (if at all)
6. List all duplicates found. Do NOT merge yet.

Expand All @@ -63,8 +66,8 @@ Search for code blocks that do the same thing in multiple places.
Search for tests that verify the same behavior.

1. Look for test functions with identical assertions against the same code paths
2. Look for test fixtures/helpers that are duplicated across test files
3. Look for integration tests that fully cover what a unit test also covers (keep the integration test, mark the unit test as redundant)
2. Look for test fixtures/helpers duplicated across test files
3. Look for integration tests that fully cover what a unit test also covers (keep the integration/E2E test, mark the unit test as redundant per CLAUDE.md rules)
4. List all duplicate tests found. Do NOT delete yet.

### Step 5 — Apply changes (one at a time)
Expand All @@ -73,32 +76,33 @@ For each change, follow this cycle: **change → test → verify coverage → co

#### 5a. Remove dead code
- Delete dead code identified in Step 2
- After each deletion: run `make test` and `make coverage-check`
- If tests fail or coverage drops: **revert immediately** and investigate
- After each deletion: run `make test` (fail-fast + coverage + threshold all in one)
- If `make test` exits non-zero: **revert immediately** and investigate
- Dead code removal should never break tests or drop coverage

#### 5b. Merge duplicate code
- For each duplicate pair: extract the shared logic into a single function/module
- Update all call sites to use the shared version
- After each merge: run `make test` and `make coverage-check`
- If tests fail: **revert immediately**
- After each merge: run `make test`
- If tests fail: **revert immediately**. The duplicates may have subtle differences you missed.
- If coverage drops: the shared code must have equivalent test coverage. Add tests if needed before proceeding.

#### 5c. Remove duplicate tests
- Delete the redundant test (keep the more thorough one)
- After each deletion: run `make coverage-check`
- If coverage drops: **revert immediately**
- After each deletion: run `make test`
- If coverage drops below threshold, `make test` exits non-zero — **revert immediately**. The "duplicate" test was covering something the other wasn't.

### Step 6 — Final verification

1. Run `make test` — all tests must still pass
2. Run `make coverage-check` — coverage must be >= the baseline from Step 1
3. Run `make lint` and `make fmt-check` — code must be clean
4. Report: what was removed, what was merged, final coverage vs baseline
1. Run `make lint` — ESLint + cspell must pass
2. Run `make test` — tests must pass AND coverage must remain ≥ the baseline from Step 1
3. Report: what was removed, what was merged, final coverage vs baseline

## Rules

- **No test coverage = do not touch.** If a file has no tests covering it, leave it alone entirely.
- **Coverage must not drop.** The coverage floor from Step 1 is sacred.
- **One change at a time.** Make one dedup change, run tests, verify coverage. Never batch.
- **When in doubt, leave it.** If two code blocks look similar but you're not 100% sure they're functionally identical, leave both.
- **Preserve public API surface.** Do not change function signatures, class names, or module exports that external code depends on.
- **Three similar lines is fine.** Only dedup when the shared logic is substantial (>10 lines) or when there are 3+ copies.
- **No test coverage = do not touch.** If a file has no tests covering it, leave it alone entirely. You cannot safely dedup what you cannot verify.
- **Coverage must not drop.** If removing or merging code causes coverage to decrease, revert and investigate. The coverage floor from Step 1 is sacred.
- **One change at a time.** Make one dedup change, run tests, verify coverage. Never batch multiple dedup changes before testing.
- **When in doubt, leave it.** If two code blocks look similar but you're not 100% sure they're functionally identical, leave both. False dedup is worse than duplication.
- **Preserve public API surface.** Do not change exported function signatures, class names, or module exports. Internal refactoring only.
- **Three similar lines is fine.** Do not create abstractions for trivial duplication. Only dedup when the shared logic is substantial (>10 lines) or when there are 3+ copies.
136 changes: 130 additions & 6 deletions .claude/skills/spec-check/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: spec-check
description: Audit spec/plan documents against the codebase. Ensures every spec section has implementing code, tests, and matching logic. Use when the user says "check specs", "spec audit", or "verify specs".
argument-hint: "[optional spec ID or filename filter]"
---
<!-- agent-pmo:5547fd2 -->
<!-- agent-pmo:424c8f8 -->

# spec-check

Expand Down Expand Up @@ -69,6 +69,21 @@ Use Glob to find candidate files, then use Grep to confirm they contain spec IDs

Spec IDs are **hierarchical descriptive slugs, NEVER numbered.** The format is `[GROUP-TOPIC]` or `[GROUP-TOPIC-DETAIL]`. The first word is the **group** — all sections sharing the same group MUST appear together in the spec's table of contents. IDs are uppercase, hyphen-separated, unique across the repo, and MUST NOT contain sequential numbers.

The hierarchy depth varies by repo: two words for simple repos (`[AUTH-LOGIN]`), three for most (`[AUTH-TOKEN-VERIFY]`), four for complex domains (`[AUTH-OAUTH-REFRESH-FLOW]`). The hierarchy mirrors the spec document's heading structure.

Examples of valid spec IDs (note how groups cluster):
- `[AUTH-LOGIN]`, `[AUTH-TOKEN-VERIFY]`, `[AUTH-TOKEN-REFRESH]` — all in the AUTH group
- `[CI-TIMEOUT]`, `[CI-LINT]`, `[CI-RELEASE]` — all in the CI group
- `[LINT-ESLINT]`, `[LINT-RUFF]` — all in the LINT group
- `[FEAT-DARK-MODE]`, `[FEAT-SEARCH-FILTER]` — all in the FEAT group

Examples of INVALID spec IDs:
- `[SPEC-001]` — numbered, meaningless
- `[FEAT-AUTH-01]` — trailing number
- `[REQ-003]` — sequential index, no group hierarchy
- `[CI-004]` — numbered, tells the reader nothing
- `[TIMEOUT]` — no group prefix, ungrouped

For each file, extract every spec ID and its associated section title (the heading text after the ID) and the full section content (everything until the next heading of equal or higher level).

---
Expand Down Expand Up @@ -101,6 +116,34 @@ Search the entire codebase for the spec ID string, **excluding** these directori

Use Grep with the literal spec ID (e.g., `[AUTH-TOKEN-VERIFY]`) to find references in code files.

Code files should contain comments referencing the spec ID. The search must catch **all** comment styles across languages:

**C-style `//` comments** (JavaScript, TypeScript, Rust, C#, F#, Java, Kotlin, Go, Swift, Dart):
- `// Implements [AUTH-TOKEN-VERIFY]`
- `// [AUTH-TOKEN-VERIFY]`
- `// Tests [AUTH-TOKEN-VERIFY]` (also counts as a code reference)
- `/// Implements [AUTH-TOKEN-VERIFY]` (doc comments)

**Hash `#` comments** (Python, Ruby, Shell/Bash, YAML, TOML):
- `# Implements [AUTH-TOKEN-VERIFY]`
- `# [AUTH-TOKEN-VERIFY]`
- `# Tests [AUTH-TOKEN-VERIFY]`

**HTML/XML comments** (HTML, CSS, SVG, XML, XAML, JSX templates):
- `<!-- Implements [AUTH-TOKEN-VERIFY] -->`
- `<!-- [AUTH-TOKEN-VERIFY] -->`

**ML-style comments** (F#, OCaml):
- `(* Implements [AUTH-TOKEN-VERIFY] *)`

**Lua comments:**
- `-- Implements [AUTH-TOKEN-VERIFY]`

**CSS comments:**
- `/* Implements [AUTH-TOKEN-VERIFY] */`

**The key rule:** any comment in any language containing the exact spec ID string (e.g., `[AUTH-TOKEN-VERIFY]`) counts as a valid code reference. The Grep search uses the literal spec ID string, so it naturally matches all comment styles. Do NOT restrict the search to specific comment prefixes — just search for the spec ID string itself.

**If NO code files reference the spec ID:**

```
Expand All @@ -118,12 +161,72 @@ this spec section, then re-run spec-check.
#### Check B: Tests reference the spec ID

Search test files for the spec ID. Test files are found in:
- `src/test/`
- `test/`
- `tests/`
- `**/*.test.*`
- `**/*.spec.*`
- `**/*_test.*`
- `**/test_*.*`
- `**/*Tests.*`
- `**/*Test.*`

Use Grep to search these locations for the literal spec ID string.

Tests should contain the spec ID in comments, test names, or annotations. The search must catch **all** test frameworks across languages:

**JavaScript/TypeScript** (Jest, Mocha, Vitest, Playwright):
- `// Tests [AUTH-TOKEN-VERIFY]`
- `describe('[AUTH-TOKEN-VERIFY] Authentication flow', () => ...)`
- `test('[AUTH-TOKEN-VERIFY] should verify token', () => ...)`
- `it('[AUTH-TOKEN-VERIFY] verifies token', () => ...)`

**Python** (pytest, unittest):
- `# Tests [AUTH-TOKEN-VERIFY]`
- `def test_auth_token_verify_flow():`
- `class TestAuthTokenVerify:`

**Rust:**
- `// Tests [AUTH-TOKEN-VERIFY]`
- `#[test] // Tests [AUTH-TOKEN-VERIFY]`

**C#** (xUnit, NUnit, MSTest):
- `// Tests [AUTH-TOKEN-VERIFY]`
- `[Fact] // Tests [AUTH-TOKEN-VERIFY]`
- `[Test] // Tests [AUTH-TOKEN-VERIFY]`
- `[TestMethod] // Tests [AUTH-TOKEN-VERIFY]`

**F#** (xUnit, Expecto):
- `// Tests [AUTH-TOKEN-VERIFY]`
- `[<Fact>] // Tests [AUTH-TOKEN-VERIFY]`
- `testCase "[AUTH-TOKEN-VERIFY] description" <| fun () ->`

**Java/Kotlin** (JUnit, TestNG):
- `// Tests [AUTH-TOKEN-VERIFY]`
- `@Test // Tests [AUTH-TOKEN-VERIFY]`

**Go:**
- `// Tests [AUTH-TOKEN-VERIFY]`
- `func TestAuthTokenVerify(t *testing.T) { // Tests [AUTH-TOKEN-VERIFY]`

**Swift** (XCTest):
- `// Tests [AUTH-TOKEN-VERIFY]`
- `func testAuthTokenVerify() { // Tests [AUTH-TOKEN-VERIFY]`

**Dart** (flutter_test):
- `// Tests [AUTH-TOKEN-VERIFY]`
- `test('[AUTH-TOKEN-VERIFY] description', () { ... });`

**Ruby** (RSpec, Minitest):
- `# Tests [AUTH-TOKEN-VERIFY]`
- `describe '[AUTH-TOKEN-VERIFY] Authentication' do`
- `it '[AUTH-TOKEN-VERIFY] verifies token' do`

**Shell** (bats, shunit2):
- `# Tests [AUTH-TOKEN-VERIFY]`
- `@test "[AUTH-TOKEN-VERIFY] description" {`

**The key rule:** same as Check A — search for the literal spec ID string in test files. Any occurrence of the exact spec ID in a test file counts. Do NOT restrict to specific patterns — just search for the spec ID string itself.

**If NO test files reference the spec ID:**

```
Expand Down Expand Up @@ -153,7 +256,26 @@ This is the most critical check. You must:
- **Missing steps** — If the spec describes 5 steps but code only implements 3, that's a violation.
- **Wrong defaults** — If the spec says "default to X" but code defaults to Y, that's a violation.

4. **If the code deviates from the spec**, report a detailed error with spec quotes and code references.
4. **If the code deviates from the spec**, report a detailed error:

```
SPEC VIOLATION: [AUTH-TOKEN-VERIFY] Code does not match spec.

SPEC SAYS:
> "The authentication flow must verify the token expiry before checking permissions"
> (from docs/specs/AUTH-SPEC.md, line 42)

CODE DOES:
> `if (hasPermission(user)) { verifyToken(token); }` (src/auth.ts:42)

DEVIATION: The code checks permissions BEFORE verifying token expiry.
The spec explicitly requires token expiry verification FIRST.

ACTION REQUIRED: Reorder the logic in src/auth.ts to verify token expiry
before checking permissions, as specified in [AUTH-TOKEN-VERIFY].
```

**STOP HERE. Do not continue to other specs.**

5. **If the code matches the spec**, this check passes. Move to the next spec.

Expand All @@ -180,6 +302,8 @@ spec-check PASSED. All specs verified.
| Spec ID | Title | Code References | Test References | Logic Match |
|----------------|--------------------------|-----------------|-----------------|-------------|
| [AUTH-TOKEN-VERIFY] | Authentication flow | src/auth.ts | tests/auth.test.ts | PASS |
| [RATE-LIMIT-CONFIG] | Rate limiting | src/rate.ts | tests/rate.test.ts | PASS |
| ... | ... | ... | ... | ... |

Checked N spec sections across M files. All have implementing code, tests, and matching logic.
```
Expand All @@ -199,7 +323,7 @@ Checked N spec sections across M files. All have implementing code, tests, and m

- **Fail fast.** Stop on the first violation. One fix at a time.
- **Be pedantic.** If the spec says it, the code must do it. No "close enough".
- **Quote everything.** Always quote the spec text and the code in error messages.
- **Quote everything.** Always quote the spec text and the code in error messages so the developer sees exactly what's wrong.
- **Be actionable.** Every error must tell the developer what file to change and what to do.
- **Exclude docs from code search.** Markdown files are documentation, not implementation.
- **No numbered IDs.** Spec IDs are hierarchical descriptive slugs, NEVER sequential numbers.
- **Exclude docs from code search.** Markdown files are documentation, not implementation. Only search actual code files for spec references.
- **No numbered IDs.** Spec IDs are hierarchical descriptive slugs (`[AUTH-TOKEN-VERIFY]`), NEVER sequential numbers (`[SPEC-001]`). The first word is the group — sections sharing a group must be adjacent in the TOC. If you encounter numbered or ungrouped IDs, flag them as a violation.
Loading
Loading