Skip to content

chore(ci): recursive typecheck across workspace packages#280

Merged
rz1989s merged 1 commit into
mainfrom
chore/recursive-typecheck
May 15, 2026
Merged

chore(ci): recursive typecheck across workspace packages#280
rz1989s merged 1 commit into
mainfrom
chore/recursive-typecheck

Conversation

@rz1989s
Copy link
Copy Markdown
Member

@rz1989s rz1989s commented May 15, 2026

Summary

Root pnpm typecheck was tsc --noEmit against the root tsconfig (includes only src/**/*.ts), silently skipping every workspace package:

  • packages/agent — 1602 tests, complex SSE + signing surface; not typechecked in CI
  • packages/sdk — had its own typecheck script, never invoked by CI
  • app — Vite-built React UI; not typechecked in CI

CI is .github/workflows/deploy.yml:36 which runs pnpm typecheck on every PR + push to main. The blind spot was real and silent.

Discovered during PR #277 (Spec 1 assertNever). Removing a switch case from agent-core.ts silently passed root pnpm typecheck but failed cd packages/agent && pnpm exec tsc --noEmit. The whole point of assertNever's compile-time guarantee was at risk of being defeated by a CI that wasn't actually running the check.

Changes

  • Add "typecheck": "tsc --noEmit" to packages/agent/package.json
  • Add "typecheck": "tsc --noEmit" to app/package.json
  • Split root scripts:
    • typecheck:root — original tsc --noEmit against root src/
    • typecheck — composite: pnpm typecheck:root && pnpm -r --filter='!sipher' run typecheck

--filter='!sipher' excludes the workspace root (sipher is the package name of the root entry . in pnpm-workspace.yaml) so the recursion doesn't infinite-loop into its own definition.

Verification

Manual fault-injection test:

echo "const x: number = 'not a number'" > packages/agent/src/_temp_inject.ts
pnpm typecheck

Output:

> tsc --noEmit
Scope: 3 of 4 workspace projects
app typecheck: Done
packages/sdk typecheck: Done
packages/agent typecheck: src/_temp_inject.ts(2,7): error TS2322: Type 'string' is not assignable to type 'number'.
packages/agent typecheck: Failed
ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL @sipher/agent@0.1.0 typecheck: `tsc --noEmit`

Removed the injection → clean run, all three packages report Done.

Out of scope

  • turbo.json migration. sipher uses pnpm workspaces without turborepo today; introducing turbo would be a larger architectural shift. The pnpm-recursive approach is sufficient for the immediate hygiene gap.

Test plan

  • pnpm typecheck runs against root + 3 packages, all clean
  • Injected type error in packages/agent surfaces via root command
  • After removing injection, clean again
  • All commits GPG-signed
  • No AI-attribution trailers

Related

This closes the followup tracked in the PR #277 / #278 / #279 chain — Root pnpm typecheck misleadingly skips packages. With this PR merged, the assertNever guard pattern from Spec 1 is reliably enforced in CI.

Root `pnpm typecheck` was `tsc --noEmit` against the root tsconfig, which
includes only `src/**/*.ts`. Packages and the app were silently skipped:
- packages/agent — 1602 tests, complex SSE + signing surface; not typechecked in CI
- packages/sdk — typechecked only via its own `typecheck` script, never invoked
- app — Vite-built React UI; not typechecked in CI

CI is the deploy.yml workflow at `.github/workflows/deploy.yml:36` which
runs `pnpm typecheck` on every PR + push to main. The blind spot was real.
Discovered during Spec 1 (PR #277) implementation when removing a switch
case from agent-core silently passed root `pnpm typecheck` but failed
`cd packages/agent && pnpm exec tsc --noEmit`.

Fix:
- Add `typecheck` script to packages/agent and app (sdk already had one)
- Split root `typecheck` into:
    typecheck:root  — original `tsc --noEmit` against root src
    typecheck       — composite: typecheck:root + `pnpm -r --filter='!sipher' run typecheck`
  `--filter='!sipher'` excludes the root from the recursion (root is named
  `sipher` in pnpm-workspace.yaml entry `.`) so we don't infinite-loop into
  the new script's own definition.

Verified: `Scope: 3 of 4 workspace projects` covers app + packages/sdk +
packages/agent. Injected `const x: number = 'not a number'` into
packages/agent/src/_temp_inject_typecheck_test.ts and confirmed root
`pnpm typecheck` reports:

  packages/agent typecheck: src/_temp_inject_typecheck_test.ts(2,7):
    error TS2322: Type 'string' is not assignable to type 'number'.

Then removed the injection and re-ran — clean.

Out of scope: turbo.json migration (sipher uses pnpm workspaces without
turborepo today; introducing turbo would be a larger architectural shift).
@vercel
Copy link
Copy Markdown

vercel Bot commented May 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sipher Ready Ready Preview, Comment May 15, 2026 8:27am

@rz1989s rz1989s merged commit ded380f into main May 15, 2026
7 of 8 checks passed
@rz1989s rz1989s deleted the chore/recursive-typecheck branch May 15, 2026 08:47
rz1989s added a commit that referenced this pull request May 17, 2026
PR #280's recursive typecheck (`pnpm -r --filter='!sipher' run typecheck`)
exposed a build-order issue that the previous root-only typecheck masked.
The agent package imports from @sipher/sdk (workspace dep) whose types live
in `dist/index.d.ts` — without building the SDK first, tsc fails with
'Cannot find module @sipher/sdk' for every consumer.

Local typecheck passes because dist/ exists from prior builds; CI does
fresh install without building. This has been failing on main since
PR #280 merged 2 days ago (last green: 7389a3e on PR #279 merge).

Add a Build SDK step between Install and Typecheck. Fixes both the
Typecheck and Test steps in one shot (tests also import the runtime
@sipher/sdk via dist/index.js).
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