feat(ci): dead-code detection gate — knip + no-unused-vars + vulture (#282)

[ClintEastman02]

Closes #282 (cairn MVG gate #6 — dead-code detection).

What this does

Closes the unused-symbol detection asymmetry: Python already fails on unused imports (ruff F401), but unused TS symbols passed silently in cdk/. Adds three detection layers plus advisory CI wiring.

• TS lint-level — @typescript-eslint/no-unused-vars (error) in cdk/ + cli/ flat configs with ^_ ignore patterns; cdk/tsconfig.json flips noUnusedLocals/noUnusedParameters to true for parity with cli/.
• TS project-graph — knip v6 with a per-workspace knip.json (cdk/cli/docs), plus a count-only ratchet (scripts/check-deadcode-ratchet.mjs + knip-baseline.json) that fails only when the count rises — the cairn "dead-code count should not increase" metric.
• Python module-level — vulture 2.16 ([tool.vulture] in pyproject.toml, --min-confidence 80) with a minimal reviewed .vulture_allowlist.py for the three SDK-mandated hook_context params.
• CI — advisory dead-code job in security-pr.yml (continue-on-error, emits ::notice::/::warning:: annotations; not blocking yet).

Resolves the three known live instances: drops the dead DEFAULT_MAX_TURNS import, removes the orphaned cdk/src/bootstrap/index.ts barrel (consumers import the submodules directly), and removes the unused LINEAR_SECRET_PREFIX export. Also clears the unused symbols the new rule surfaces across cdk/cli.

The eslint half is blocking via the existing build check; knip + vulture stay advisory until the knip baseline is driven to zero (deferred AC, per the issue's phased rollout).

Acceptance criteria

• @typescript-eslint/no-unused-vars as error in cdk/cli flat configs with ^_ patterns; cdk tsconfig flags flipped to true
• knip added with per-workspace knip.json (cdk/cli/docs); cli/src/index.ts not flagged (entry-point caveat handled)
• vulture for agent/ with reviewed allowlist + --min-confidence 80
• Three known instances resolved (DEFAULT_MAX_TURNS, bootstrap barrel, LINEAR_SECRET_PREFIX)
• Gate runs in fast CI, advisory first
• Gate becomes blocking once baseline is clean (deferred per issue's "advisory first, then ratchet" rollout)

Test plan

Verified locally on a fresh checkout:

• mise //cdk:build — compile, tests, eslint (no-unused-vars active), synth all pass
• mise //cli:build — compile, 565 tests, eslint all pass
• yarn knip + node scripts/check-deadcode-ratchet.mjs — ratchet holds at baseline (78; ratcheted down from 79 after merging main chore: repo cleanup — roadmap removal, CLI dedupe, doc hygiene #505)
• uvx vulture (agent) — clean, exit 0
• mise //cdk:eslint, mise //cli:eslint — pass

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@37a5c88). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #520   +/-   ##
=======================================
  Coverage        ?   88.92%           
=======================================
  Files           ?      222           
  Lines           ?    52547           
  Branches        ?     5440           
=======================================
  Hits            ?    46730           
  Misses          ?     5817           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ayushtr-aws]

Review

Overview

This PR closes the unused-symbol detection asymmetry between Python and TypeScript by adding three dead-code detection layers — @typescript-eslint/no-unused-vars (blocking, via the existing build check), knip + a count-only ratchet (advisory), and vulture for Python (advisory) — plus an advisory dead-code CI job. It also clears the symbols the new rules surface: dropping the dead DEFAULT_MAX_TURNS import, the orphaned bootstrap/index.ts barrel, the unused LINEAR_SECRET_PREFIX export, and a scattering of ^_-prefix renames.

The bulk of the diff is inert and safe: config, lockfile churn, and parameter/catch-variable renames. Two changes carry real logic, and one of them regresses a compile-time guard. Findings below, most-severe first.

---

Findings

1. cdk/src/handlers/shared/validation.ts:316 — refactor silently defeats the AttachmentType exhaustiveness check (CONFIRMED)

The change rewrote a typed assignment into a type assertion:

-const _attachmentExhaustiveCheck: _AssertAttachmentExhaustive = true;
+void (true as _AssertAttachmentExhaustive);

These are not equivalent. The guard relies on _AssertAttachmentExhaustive resolving to never when AttachmentType gains a member missing from ATTACHMENT_TYPE_LIST:

• Old (assignment): const x: never = true is a hard compile error — true is not assignable to never. That error is the guard.
• New (assertion): true as never is an assertion, and TypeScript always permits X as Y when either type is assignable to the other. never is assignable to true, so true as never never errors.

Failure scenario: someone adds 'video' to AttachmentType without updating ATTACHMENT_TYPE_LIST. The old code fails the build; the new code compiles clean, and VALID_ATTACHMENT_TYPES silently omits the new type — the exact regression the assertion was meant to catch ships unnoticed.

Suggested fix — restore the assignment and consume the binding so it survives both noUnusedLocals (which this PR flips to true and which does not honor eslint's ^_ pattern) and @typescript-eslint/no-unused-vars:

/** Compile-time exhaustiveness check for AttachmentType. */
const ATTACHMENT_TYPE_LIST = ['image', 'file', 'url'] as const satisfies readonly AttachmentType[];
type _AssertAttachmentExhaustive = Exclude<AttachmentType, (typeof ATTACHMENT_TYPE_LIST)[number]> extends never ? true : never;
// The ASSIGNMENT is the guard: if AttachmentType gains a member missing from
// ATTACHMENT_TYPE_LIST, _AssertAttachmentExhaustive resolves to `never` and
// assigning `true` fails to compile. `void` consumes the binding so
// noUnusedLocals stays satisfied without weakening the check.
const _attachmentExhaustiveCheck: _AssertAttachmentExhaustive = true;
void _attachmentExhaustiveCheck;

---

2. scripts/check-deadcode-ratchet.mjs:606 — classMembers is not a valid knip 6.x key; namespaceMembers is missing (low severity)

COUNTED_KEYS lists classMembers, which is not an issue key knip 6.x emits — it's always undefined, so it's silently dropped (harmless). Meanwhile the real member-issue key namespaceMembers is absent from the list, so dead namespace members never move the ratchet count, despite the script's comment claiming it counts "enum/class members." Separately, duplicates is emitted as an array-of-arrays (each inner array is one duplicate group), so issue.duplicates.length counts groups rather than individual duplicate symbols.

Impact is low because the ratchet step is continue-on-error (advisory), but it should be reconciled before the gate flips to blocking.

Suggested fix:

const COUNTED_KEYS = [
  'dependencies',
  'devDependencies',
  'optionalPeerDependencies',
  'unlisted',
  'binaries',
  'unresolved',
  'exports',
  'types',
  'nsExports',
  'nsTypes',
  'duplicates',
  'enumMembers',
  'namespaceMembers',  // was 'classMembers' — not a knip 6.x key
];

---

Minor note (not a finding)

The PR description's test plan says the ratchet "holds at baseline (79)", but knip-baseline.json commits "count": 78. If the live count is 79, the gate emits a ::warning:: from day one — harmless while advisory, but reconcile the baseline before it becomes blocking.

---

Verdict

One change blocks: the validation.ts exhaustiveness regression (# 1) reintroduces exactly the class of silent drift this PR's tooling is meant to prevent, so it's worth fixing in-PR. The ratchet key fix (# 2) and the baseline note are low-severity cleanups that can ride along or follow before the gate goes blocking. Everything else — the eslint/tsconfig wiring, vulture setup, dead-symbol removals, and renames — looks correct and well-scoped.

[krokoko]

Thanks for this — it's a clean, well-scoped implementation of the dead-code gate (#282), and the advisory-first rollout is exactly the right call. Approving with a few nits, none blocking — contingent on the fixes identified by the other reviewers in the comments above being addressed as well.

What's solid:

• All three known dead symbols (DEFAULT_MAX_TURNS, the bootstrap barrel, LINEAR_SECRET_PREFIX) are correctly removed and verified unreferenced.
• The _-prefix param renames and test cleanups are safe — no assertions weakened.
• Comments are accurate where it counts (the exhaustiveness-guard rationale and the hook_context line numbers both check out).
• Mirrors the existing CA-10/feat(ci): no-magic-numbers (eslint) + PLR2004 (ruff) with allowlist (CA-10) #258 parity pattern nicely; deps land in the right manifests.

Worth a look (non-blocking):

1. Ratchet can fail open. In scripts/check-deadcode-ratchet.mjs, if a future knip (^6.20.0) renames a category or reshapes its JSON, the count silently drops below baseline and the script reports a passing build — quietly disabling the gate. Fine while advisory, but please pin knip exactly (or assert report.issues is present) before flipping to blocking.
2. COUNTED_KEYS omits nsExports/nsTypes — harmless today, but the comment claims completeness; a one-line note would clear it up.
3. Small comment nit in validation.ts:294 — the void is really there for noUnusedLocals, not eslint (which already exempts ^_).
4. A countIssues fixture test would pair well with the eventual blocking flip.

Everything else — docs sync, CLI/API type sync, bootstrap policies — is N/A for this diff. Nice work; happy to see this land once the above and the other reviewers' points are folded in. 🚀

[ClintEastman02]

Thanks for the thorough review @ayushtr-aws — all three addressed in 384efa8.

#1 — validation.ts exhaustiveness guard (confirmed, fixed). You're right that true as never is an assertion and never errors. Restored the assignment form and consumed the binding with void so it survives both noUnusedLocals (now true) and @typescript-eslint/no-unused-vars:

const _attachmentExhaustiveCheck: _AssertAttachmentExhaustive = true;
void _attachmentExhaustiveCheck;

Verified empirically: adding a 'video' member to AttachmentType without updating ATTACHMENT_TYPE_LIST now fails tsc again (TS2322: Type 'true' is not assignable to type 'never'), and compiles clean when exhaustive. Comment updated to spell out why it must stay an assignment.

#2 — ratchet COUNTED_KEYS (confirmed, fixed). Dumped knip 6.x's actual JSON keys and corrected the list: dropped the non-existent classMembers/nsExports/nsTypes, added the real namespaceMembers (and catalog). Also found a related bug while in there — the script counted unused files from a non-existent top-level report.files; knip nests them in issues[].files[], so files is now in COUNTED_KEYS and unused files actually move the count. Live count unchanged at 78 (1 devDep + 37 exports + 40 types).

Baseline note. Good catch on the description drift — the committed knip-baseline.json was already 78 (the merge of main #505 removed a dead symbol, so I ratcheted 79→78 in ea6d75a). Updated the stale "79" in the PR test plan.

Everything stays advisory as designed; these were the correctness fixes needed before the gate can flip to blocking.

[ClintEastman02]

Thanks @krokoko! Folded in the non-blocking nits in 2a7555c (ayushtr-aws's three findings were already addressed in 384efa8):

1. Ratchet fail-open — good catch, this was the one worth hardening now. Pinned knip exactly (6.20.0, dropped the caret) and countIssues now requires report.issues to be an array: if a future knip reshapes its JSON the script exits 2 (fails loud) instead of counting 0 and reporting a false pass. So a schema change becomes a deliberate, reviewed bump rather than a silently-disabled gate.
2. nsExports/nsTypes — clarified in the COUNTED_KEYS comment that the list is complete for the pinned knip and those keys aren't part of its schema (namespace/enum members surface as namespaceMembers/enumMembers, both counted). The fail-loud guard backstops any future drift.
3. validation.ts comment — you're right, the void is there for noUnusedLocals, not eslint (the ^_ binding is already exempt via varsIgnorePattern). Reworded to attribute it correctly.
4. countIssues fixture test — agree this pairs with the blocking flip; I've left it as a deferred item for that follow-up rather than expanding this PR's scope, since the count is exercised end-to-end by the advisory CI job today.

Baseline is holding at 78 through the recent main merges. All checks green. Ready for another look whenever you have a moment. 🙏