Skip to content

[#1357] codev-sdk: controller subpath re-exports overview wire types; codev-types becomes a regular (type-only) dependency - #1358

Merged
amrmelsayed merged 7 commits into
mainfrom
builder/air-1357
Aug 5, 2026
Merged

[#1357] codev-sdk: controller subpath re-exports overview wire types; codev-types becomes a regular (type-only) dependency#1358
amrmelsayed merged 7 commits into
mainfrom
builder/air-1357

Conversation

@amrmelsayed

Copy link
Copy Markdown
Collaborator

Closes #1357. Unblocks #1347's import-boundary acceptance criterion (plugin imports only sdk subpaths + @elgato/streamdeck).

Summary

Two small changes to @cluesmith/codev-sdk, per the issue and the architect's scope guidance:

  1. Overview wire types ship with the client. ./controller now re-exports OverviewData, OverviewBuilder, OverviewPR, and OverviewBacklogItem type-only from @cluesmith/codev-types — a controller READS overview state, and TowerClient.getOverview returns OverviewData, so the contract belongs on the subpath (matching the old @cluesmith/codev-client single-import stance). ./tower-client additionally carries OverviewData for consumers importing the client directly.

  2. Packaging fix for npm consumers. @cluesmith/codev-types moves from devDependencies to dependencies. The sdk's published .d.ts files reference it (import type ... from '@cluesmith/codev-types' in tower-client.d.ts), so a fresh npm consumer running tsc could not resolve it while it stayed a devDependency. codev-types is published (3.2.4 on npm) and types-only, so this would have bitten at the sdk's first npm publish and in Migrate the Stream Deck plugin into the monorepo as apps/streamdeck, consuming @cluesmith/codev-sdk #1347's published-SDK canary.

Key decisions

  • export type { ... } from (whole-statement form), pinned by test. The mixed export { type X } from '...' form is NOT equivalent — it still emits a runtime re-export statement (export {} from '...'), which would load codev-types at runtime. The import-boundary test now has a UNIVERSAL rule forbidding non-export type re-exports of codev-types, mirroring the existing import type rule, plus a pin test asserting the controller subpath re-exports all four overview types in the erased form.
  • Zero-runtime-deps intent restated, not weakened. The boundary test's dependencies assertion previously required an empty field; its intent was always zero RUNTIME dependencies. It now asserts dependencies is exactly ['@cluesmith/codev-types'], with a doc comment explaining the one allowed entry exists only so published .d.ts files typecheck — the source rules guarantee it is erased at build and never loaded at runtime. Verified against the emitted output: dist/controller.js and dist/tower-client.js contain no codev-types reference; only the .d.ts files do.
  • Lockfile updated for the dependency-group move (pnpm install, workspace resolution unchanged).

Test plan

  • Extended packages/sdk/src/__tests__/import-boundary.test.ts (new re-export rule, restated dependencies assertion, new controller pin test).
  • pnpm --filter @cluesmith/codev-sdk build — clean; emit inspection confirms type re-exports are erased from JS and present in .d.ts.
  • pnpm --filter @cluesmith/codev-sdk test — 6 files, 73 tests passed.
  • porch check — build + tests green.

…v-types becomes a regular (type-only) dependency

- @cluesmith/codev-sdk/controller re-exports OverviewData, OverviewBuilder,
  OverviewPR, OverviewBacklogItem type-only from @cluesmith/codev-types, so
  integrations (first consumer: the streamdeck plugin migrating under #1347)
  get the client and the contract from a single import.
- @cluesmith/codev-sdk/tower-client also carries OverviewData, since
  getOverview returns it.
- @cluesmith/codev-types moves devDependencies -> dependencies: published
  .d.ts files reference it, so a fresh npm consumer's tsc needs it
  installed. Type-only, erased at build - zero runtime deps stays true.
- import-boundary test extended: value re-exports of codev-types are now a
  violation (whole-statement export type form required - the mixed
  'export { type X } from' form still emits a runtime re-export); the
  dependencies assertion restated as exactly ['@cluesmith/codev-types']
  with the intent documented; new pin test asserts the controller subpath
  re-exports the four overview types via export type.
@amrmelsayed

Copy link
Copy Markdown
Collaborator Author

Architect Review

APPROVE (pr gate; merge awaits Amr's authorization per current workspace policy).

Verified in the diff:

  • Type-only re-exports, both subpaths: controller carries the four Overview* wire types; tower-client carries OverviewData (the return type traveling with the method that returns it). Both use the whole-statement export type { … } from form with in-code rationale.
  • The packaging fix is the important half: @cluesmith/codev-types devDeps → deps, so published .d.ts files resolve for fresh npm consumers — the publish blocker codev-sdk: controller subpath does not re-export the overview wire types its first consumer needs #1357 flagged, dead before the first publish.
  • The boundary test evolved instead of being weakened — this is the part I'd have pushed back on if fudged, and it wasn't: intent restated as zero-RUNTIME-deps, an exact-one-dep assertion (['@cluesmith/codev-types'] — nothing else can creep in), and a regex banning any runtime-form re-export of the types package. The guard is stronger after the change than before it.
  • Independently relevant: 73 sdk tests + e2e green per the builder; the change unblocks Migrate the Stream Deck plugin into the monorepo as apps/streamdeck, consuming @cluesmith/codev-sdk #1347's import-boundary acceptance criterion.

On consultation: none was run, and per the AIR protocol as written ('Optional — builder decides') that is the builder's documented discretion, correctly exercised for a change this size. The contradiction between that clause and CLAUDE.md's characterization (which I wrongly enforced against air-1352) is now tracked as #1359 — this PR proceeds under the protocol's actual text.


Architect review

@amrmelsayed
amrmelsayed merged commit 63d0d81 into main Aug 5, 2026
6 checks passed
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.

codev-sdk: controller subpath does not re-export the overview wire types its first consumer needs

1 participant