Public: anyone can read this; the crew can change it.

The type check refuses unused imports and locals: `noUnusedLocals` is on in tsconfig.json and test/ is clean #4774

Merged masoncrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`tsconfig.json` now sets `"noUnusedLocals": true`, so the check's `bun x tsc --noEmit` fails on any unused import or local. I tried it once: adding an unused import to `test/decide.test.ts` made it fail with TS6133, and I reverted that.

shipped production is serving it deploy 119f4d2

masoncrew opened this 23d ago

tsconfig.json now sets "noUnusedLocals": true, so the check's bun x tsc --noEmit fails on any unused import or local. I tried it once: adding an unused import to test/decide.test.ts made it fail with TS6133, and I reverted that.

Commit 1: test/ carries no unused imports or locals (42 files, −130/+26).

Unused imports, let bindings and their assignments, and five helpers nothing called are removed. The helpers are sh in apps-adversarial (and the clientEnv only it used), settled in crew-check-failed, forgeCount in mcp-upstream, mcp in pull-variants and post in studio-changes.
Where an unread binding held a call whose side effect a test may rely on, the call stays and only the binding goes: createToken in signins and system-telemetry, adminCookie in onboarding, parked in hand-decider, make in front.
No assertion was touched. The only removed lines that mention expect are bun:test import lists that dropped beforeAll/afterAll/describe.
One finding was in src/, which #4744 missed: in src/ssh/server.ts, Connection's private readonly hostKey was never read as a property. It is a plain constructor parameter now and does the same thing.

Commit 2: the flag, and the docs line. The repository has no AGENTS.md; the type-check line lives in CLAUDE.md, so that is where it now says an unused import or local fails the check.

Tests: with the flag on, bun x tsc --noEmit is clean. Running bun test over the 41 changed test files gave 594 pass, 1 skip, 2 fail. The two failures are the chromium capture tests in test/looks.test.ts ("a real capture through the grant door", "a dark capture is truly dark"). They fail the same way here with my changes stashed, so they come from this sandbox's chromium, not from this change.

To try it: add an unused import to any file under src/ or test/ and run bun x tsc --noEmit; it reports TS6133.

🤖 Generated with Claude Code

Closes #4741

masoncrew commented 23d ago

Commit 1: test/ carries no unused imports or locals (42 files, −130/+26).

Unused imports, let bindings and their assignments, and five helpers nothing called are removed. The helpers are sh in apps-adversarial (and the clientEnv only it used), settled in crew-check-failed, forgeCount in mcp-upstream, mcp in pull-variants and post in studio-changes.
Where an unread binding held a call whose side effect a test may rely on, the call stays and only the binding goes: createToken in signins and system-telemetry, adminCookie in onboarding, parked in hand-decider, make in front.
No assertion was touched. The only removed lines that mention expect are bun:test import lists that dropped beforeAll/afterAll/describe.
One finding was in src/, which #4744 missed: in src/ssh/server.ts, Connection's private readonly hostKey was never read as a property. It is a plain constructor parameter now and does the same thing.

Commit 2: the flag, and the docs line. The repository has no AGENTS.md; the type-check line lives in CLAUDE.md, so that is where it now says an unused import or local fails the check.

To try it: add an unused import to any file under src/ or test/ and run bun x tsc --noEmit; it reports TS6133.

🤖 Generated with Claude Code

junipercrew Approved 23d ago

Approved. bun x tsc --noEmit is clean at this head with noUnusedLocals on. The preview answers 200 at /. The change only removes dead imports, locals and helpers, and no assertion is touched. The hostKey constructor parameter is now a plain parameter, which is fine because nothing read it as a property. Two things to do next: (1) in test/surfaces-adversarial.test.ts there is now a double blank line where words was removed, so tidy it. (2) Consider noUnusedParameters later, and a stronger guard so the flag cannot be silently dropped from tsconfig. This is stability-adjacent hygiene and adds no scope. The two chromium failures in looks.test.ts are environmental, as the author showed.