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

.github/workflows/security-pr.yml

@@ -7,6 +7,12 @@ name: security-pr
 # (<~3 min) and can be a required status check without throttling the inner loop.
 # The heavy image + SAST suite (trivy/grype/semgrep) runs in security.yml and is
 # tracked separately by #235; the full-history secret sweep also lives there.
+#
+# The `dead-code` job (issue #282, cairn MVG gate #6) is intentionally ADVISORY:
+# its steps use continue-on-error so findings surface as annotations without
+# blocking the merge queue. It flips to a required/blocking gate once the knip
+# baseline (knip-baseline.json) is driven to zero. The eslint no-unused-vars
+# half of the gate is already blocking via the `build` check.
 on:
   pull_request: {}
   # Must also run on merge_group so this check reports inside the merge queue;
@@ -86,3 +92,49 @@ jobs:
 
       - name: Workflow static analysis (zizmor)
         run: mise run security:gh-actions
+
+  dead-code:
+    name: Dead-code detection (advisory)
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    permissions:
+      contents: read
+    env:
+      CI: "true"
+      MISE_EXPERIMENTAL: "1"
+    steps:
+      - name: Checkout
+        uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
+        with:
+          persist-credentials: false
+
+      - name: Install mise
+        uses: jdx/mise-action@dba19683ed58901619b14f395a24841710cb4925 # v4.1.0
+        with:
+          cache: true
+
+      - name: Setup Node.js
+        uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
+        with:
+          node-version: 22.x
+
+      - name: Install dependencies
+        run: mise run install
+
+      - name: TS dead-code ratchet (knip, advisory)
+        continue-on-error: true
+        run: |
+          if mise run check:deadcode-ratchet; then
+            echo "::notice title=knip ratchet::Dead-code count is at or below baseline."
+          else
+            echo "::warning title=knip ratchet::Dead-code count increased above the baseline (knip-baseline.json). Run 'yarn knip' locally and remove the new dead code, or suppress a false positive in knip.json."
+          fi
+
+      - name: Python dead-code scan (vulture, advisory)
+        continue-on-error: true
+        run: |
+          if mise //agent:lint:deadcode; then
+            echo "::notice title=vulture::No Python dead code above the confidence threshold."
+          else
+            echo "::warning title=vulture::vulture found unused Python code. Remove it, or add an intentional keep to agent/.vulture_allowlist.py."
+          fi

agent/.vulture_allowlist.py

@@ -0,0 +1,23 @@
+# Vulture allowlist (issue #282, cairn MVG gate #6).
+#
+# Names here are reported by vulture as unused but are intentionally kept.
+# vulture "uses" every attribute access in this file, so listing a name
+# suppresses its finding. Only entries that fire at `--min-confidence 80`
+# (the CI threshold) belong here — lower-confidence noise (Pydantic
+# `model_config`, FastAPI route handlers, dynamically-dispatched methods)
+# is below the threshold and must NOT be blanket-allowlisted, so that real
+# dead code in those forms still surfaces.
+#
+# Regenerate/extend (then hand-review the diff):
+#   cd agent && uvx vulture src --make-whitelist
+#
+# Keep this list MINIMAL. Prefer deleting dead code over allowlisting it.
+
+# claude-agent-sdk invokes hooks positionally as
+# (hook_input, tool_use_id, hook_context). The dispatcher in hooks.py passes
+# its own `ctx`, so the `hook_context` positional is part of the required
+# signature but unread in these three hook bodies. Cannot be removed without
+# breaking the SDK calling convention.
+hook_context  # unused variable (src/hooks.py:132)
+hook_context  # unused variable (src/hooks.py:918)
+hook_context  # unused variable (src/hooks.py:1258)

agent/mise.toml

@@ -44,6 +44,10 @@ run = "uvx ruff format --check"
 description = "Type check with ty"
 run = "uv run ty check"
 
+[tasks."lint:deadcode"]
+description = "Dead-code detection with vulture (#282, cairn MVG gate #6): unused functions/classes/methods ruff F cannot see. Config + allowlist in pyproject.toml / .vulture_allowlist.py."
+run = "uvx vulture"
+
 [tasks.test]
 description = "Run tests with pytest (coverage floor enforced; use `uv run pytest --no-cov` for focused runs)"
 run = "uv run pytest --cov=src --cov-branch --cov-fail-under=72 --cov-report=term-missing:skip-covered --cov-report=lcov:coverage/lcov.info --cov-report=json:coverage/coverage.json"

agent/pyproject.toml

@@ -55,6 +55,14 @@ constraint-dependencies = [
     "pydantic-settings>=2.14.2",  # GHSA-4xgf-cpjx-pc3j — transitive via mcp; remove when mcp bumps floor
 ]
 
+# Dead-code detection (#282, cairn MVG gate #6). ruff "F" catches unused
+# imports/locals; vulture catches unused module-level functions/classes/methods
+# that ruff cannot see. min_confidence 80 keeps the signal high; intentional
+# keeps (e.g. SDK-mandated hook params) live in .vulture_allowlist.py.
+[tool.vulture]
+paths = ["src", ".vulture_allowlist.py"]
+min_confidence = 80
+
 [tool.bandit]
 exclude_dirs = ["tests", ".venv"]
 skips = [
@@ -73,11 +81,16 @@ dev = [
     "pytest",
     "pygments==2.20.0",
     "pytest-cov==7.1.0",
+    "vulture==2.16",  # dead-code detection (#282): unused functions/classes ruff F can't see
 ]
 
 [tool.ruff]
 target-version = "py313"
 line-length = 100
+# The vulture allowlist is bare-name expressions by design (that is how vulture
+# "uses" a name to suppress it). ruff would flag every line F821/B018, so it is
+# not Python source ruff should lint. See .vulture_allowlist.py (#282).
+extend-exclude = [".vulture_allowlist.py"]
 
 [tool.ruff.lint]
 select = [
@@ -124,3 +137,9 @@ skip_covered = true
 [tool.ty.environment]
 python-version = "3.13"
 extra-paths = ["src"]
+
+[tool.ty.src]
+# The vulture allowlist is bare-name expressions by design, not type-checkable
+# Python — ty flags each line unresolved-reference. Excluded here for the same
+# reason ruff excludes it via extend-exclude (#282).
+exclude = [".vulture_allowlist.py"]

cdk/eslint.config.mjs

@@ -156,6 +156,19 @@ export default [
       }],
 
       // TypeScript rules
+      // Dead-code parity with Python ruff F401 (#282): unused imports/locals/params
+      // are errors. `^_` opts out an intentionally-unused binding. Base rule must be
+      // off so the typescript-eslint variant (type-aware) is the only one active.
+      'no-unused-vars': 'off',
+      '@typescript-eslint/no-unused-vars': ['error', {
+        args: 'all',
+        argsIgnorePattern: '^_',
+        varsIgnorePattern: '^_',
+        caughtErrors: 'all',
+        caughtErrorsIgnorePattern: '^_',
+        ignoreRestSiblings: true,
+        destructuredArrayIgnorePattern: '^_',
+      }],
       // AI007 guard (#258): inline numeric literals should be named constants.
       // Values shared across Python/TypeScript belong in contracts/constants.json
       // (see contracts/constants.md). Baseline cleaned in #312; blocking like cli/.

cdk/src/handlers/confirm-uploads.ts

@@ -294,9 +294,9 @@ export async function handler(event: APIGatewayProxyEvent, context: Context): Pr
 
 async function screenSingleAttachment(
   att: AttachmentRecord,
-  task: TaskRecord,
+  _task: TaskRecord,
   screeningConfig: ScreeningConfig,
-  taskId: string,
+  _taskId: string,
   meta: S3ObjectMeta,
 ): Promise<AttachmentRecord> {
   const { s3Key, versionId, sizeBytes } = meta;
@@ -530,7 +530,7 @@ async function transitionToSubmitted(
 // ---------------------------------------------------------------------------
 
 async function failTaskOnScreening(
-  task: TaskRecord,
+  _task: TaskRecord,
   taskId: string,
   filename: string,
   reason: string,

cdk/src/handlers/shared/create-task-core.ts

@@ -53,7 +53,7 @@ import {
   type TaskRecord,
   toTaskDetail,
 } from './types';
-import { computeTtlEpoch, DEFAULT_MAX_TURNS, hasTaskSpec, isValidIdempotencyKey, isValidRepo, isValidTaskDescriptionLength, MAX_ATTACHMENT_SIZE_BYTES, MAX_TASK_DESCRIPTION_LENGTH, validateAttachments, validateMaxBudgetUsd, validateMaxTurns, validatePrNumber } from './validation';
+import { computeTtlEpoch, hasTaskSpec, isValidIdempotencyKey, isValidRepo, isValidTaskDescriptionLength, MAX_ATTACHMENT_SIZE_BYTES, MAX_TASK_DESCRIPTION_LENGTH, validateAttachments, validateMaxBudgetUsd, validateMaxTurns, validatePrNumber } from './validation';
 import { disallowedWorkflowModel, getWorkflowDescriptor, isValidWorkflowRef, resolveWorkflowRef, resolveWorkflowRefError } from './workflows';
 import { ATTACHMENT_OBJECT_KEY_PREFIX } from '../../constructs/attachments-bucket';
 import { TaskStatus } from '../../constructs/task-status';
@@ -522,7 +522,6 @@ export async function createTaskCore(
       if (att.delivery !== 'presigned') continue;
       const presignedAtt = att as PresignedAttachment;
       const attachmentId = ulid();
-      const s3Key = `${ATTACHMENT_OBJECT_KEY_PREFIX}${context.userId}/${taskId}/${attachmentId}/${presignedAtt.filename}`;
 
       attachmentRecords.push(createAttachmentRecord({
         attachment_id: attachmentId,

cdk/src/handlers/shared/linear-verify.ts

@@ -28,9 +28,6 @@ import { logger } from './logger';
 const sm = new SecretsManagerClient({});
 const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({}));
 
-/** Prefix for Linear-related secrets in Secrets Manager. */
-export const LINEAR_SECRET_PREFIX = 'bgagent/linear/';
-
 // In-memory secret cache with 5-minute TTL (same pattern as slack-verify.ts).
 const secretCache = new Map<string, { secret: string; expiresAt: number }>();
 const CACHE_TTL_MINUTES = 5;

cdk/src/handlers/shared/validation.ts

@@ -286,7 +286,9 @@ export const MAX_TOTAL_ATTACHMENT_SIZE_BYTES = MAX_TOTAL_ATTACHMENT_SIZE_MB * 10
 /** 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;
-const _attachmentExhaustiveCheck: _AssertAttachmentExhaustive = true;
+// `void` keeps the compile-time exhaustiveness assertion without an unused binding
+// (tsconfig noUnusedLocals does not honor eslint's ^_ ignore pattern).
+void (true as _AssertAttachmentExhaustive);
 const VALID_ATTACHMENT_TYPES = new Set<string>(ATTACHMENT_TYPE_LIST);
 
 /** Allowed image MIME types (PNG and JPEG only — passed directly to Bedrock). */
@@ -411,7 +413,7 @@ export function isValidFilename(filename: string): boolean {
 }
 
 /** Generate a default filename when none was provided. */
-function generateFilename(type: string, contentType: string, index: number): string {
+function generateFilename(_type: string, contentType: string, index: number): string {
   const ext = MIME_TO_EXTENSION[contentType] ?? 'bin';
   return `attachment_${index}.${ext}`;
 }

cdk/test/handlers/confirm-uploads.test.ts

@@ -234,9 +234,7 @@ describe('confirm-uploads handler', () => {
     });
 
     // Use mockImplementation for S3 calls to handle interleaving
-    let s3CallCount = 0;
     s3Send.mockImplementation((cmd: any) => {
-      s3CallCount++;
       if (cmd._type === 'S3Head') {
         // HeadObject — return valid metadata
         const isAtt1 = cmd.input.Key?.includes('att-1');

cdk/test/handlers/shared/context-hydration.test.ts

@@ -54,7 +54,6 @@ import {
   hydrateContext,
   resolveGitHubToken,
   screenWithGuardrail,
-  type ContentTrustLevel,
   type GitHubIssueContext,
   type GuardrailScreeningResult,
   type IssueComment,

cdk/test/handlers/shared/resolve-url-attachments.test.ts

@@ -166,7 +166,7 @@ jest.mock('../../../src/handlers/shared/logger', () => ({
 describe('resolveUrlAttachments', () => {
   const dns = jest.requireMock('dns').promises;
   const { screenImage } = jest.requireMock('../../../src/handlers/shared/attachment-screening');
-  const { isAllowedMimeType, validateMagicBytes } = jest.requireMock('../../../src/handlers/shared/validation');
+  const { isAllowedMimeType } = jest.requireMock('../../../src/handlers/shared/validation');
 
   const PNG_MAGIC = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
 

cdk/test/handlers/slack-command-processor.test.ts

@@ -121,7 +121,7 @@ describe('slack-command-processor handler', () => {
   test('slash submit tells user to use @mention', async () => {
     await handler(slashCommand({ text: 'submit org/repo fix' }));
     const posted = fetchMock.mock.calls.find(
-      ([url, opts]) => String((opts as { body: string }).body).includes('Use `@Shoof` to submit tasks'),
+      ([_url, opts]) => String((opts as { body: string }).body).includes('Use `@Shoof` to submit tasks'),
     );
     expect(posted).toBeTruthy();
     expect(createTaskCoreMock).not.toHaveBeenCalled();
@@ -215,15 +215,15 @@ describe('slack-command-processor handler', () => {
     expect(putCall![0].input.Item.slack_identity).toMatch(/^pending#/);
     expect(putCall![0].input.Item.status).toBe('pending');
     const posted = fetchMock.mock.calls.find(
-      ([url, opts]) => String((opts as { body: string }).body).includes('bgagent slack link'),
+      ([_url, opts]) => String((opts as { body: string }).body).includes('bgagent slack link'),
     );
     expect(posted).toBeTruthy();
   });
 
   test('help subcommand replies with usage text', async () => {
     await handler(slashCommand({ text: 'help' }));
     const posted = fetchMock.mock.calls.find(
-      ([url, opts]) => String((opts as { body: string }).body).includes('Using Shoof'),
+      ([_url, opts]) => String((opts as { body: string }).body).includes('Using Shoof'),
     );
     expect(posted).toBeTruthy();
   });

cdk/tsconfig.json

@@ -18,8 +18,8 @@
     "noImplicitAny": true,
     "noImplicitReturns": true,
     "noImplicitThis": true,
-    "noUnusedLocals": false,
-    "noUnusedParameters": false,
+    "noUnusedLocals": true,
+    "noUnusedParameters": true,
     "resolveJsonModule": true,
     "strict": true,
     "strictNullChecks": true,

cli/eslint.config.mjs

@@ -153,6 +153,19 @@ export default [
       }],
 
       // TypeScript rules
+      // Dead-code parity with Python ruff F401 (#282): unused imports/locals/params
+      // are errors. `^_` opts out an intentionally-unused binding. Base rule must be
+      // off so the typescript-eslint variant (type-aware) is the only one active.
+      'no-unused-vars': 'off',
+      '@typescript-eslint/no-unused-vars': ['error', {
+        args: 'all',
+        argsIgnorePattern: '^_',
+        varsIgnorePattern: '^_',
+        caughtErrors: 'all',
+        caughtErrorsIgnorePattern: '^_',
+        ignoreRestSiblings: true,
+        destructuredArrayIgnorePattern: '^_',
+      }],
       // AI007 guard (#258): inline numeric literals should be named constants.
       // Values shared across Python/TypeScript belong in contracts/constants.json
       // (see contracts/constants.md).

cli/src/linear-oauth.ts

@@ -253,7 +253,7 @@ async function parseTokenResponse(
   let body: unknown;
   try {
     body = await response.json();
-  } catch (err) {
+  } catch (_err) {
     throw new CliError(
       `Linear /oauth/token returned non-JSON during ${contextLabel}: HTTP ${response.status}`,
     );

cli/test/commands/github-token.test.ts

@@ -17,7 +17,7 @@
  *  SOFTWARE.
  */
 
-import { GetSecretValueCommand, PutSecretValueCommand } from '@aws-sdk/client-secrets-manager';
+import { PutSecretValueCommand } from '@aws-sdk/client-secrets-manager';
 import { GetCommand } from '@aws-sdk/lib-dynamodb';
 import { CliError } from '../../src/errors';
 import {

knip-baseline.json

@@ -0,0 +1,4 @@
+{
+  "count": 78,
+  "comment": "Dead-code ratchet baseline for issue #282 (cairn MVG gate #6). This is the knip issue count at the time the gate was introduced — pre-existing unused exports/types that are out of scope to remove in the gate PR. The ratchet (scripts/check-deadcode-ratchet.mjs) fails the build only if the count rises above this number. When dead code is removed and the count drops, lower this value in the same PR to lock in the gain. Per-category false positives belong in knip.json, not here."
+}