Skip to content

refactor: import package types directly — no internal re-export laundering - #1640

Merged
thymikee merged 2 commits into
mainfrom
claude/direct-contracts-imports
Aug 6, 2026
Merged

refactor: import package types directly — no internal re-export laundering#1640
thymikee merged 2 commits into
mainfrom
claude/direct-contracts-imports

Conversation

@thymikee

@thymikee thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Follow-up to #1636 review feedback (the export type { ScreenshotResultData } from '@agent-device/contracts/capture' in utils/screenshot-result.ts): internal modules should not re-export package types — consumers import directly from the owning package. This PR applies that rule repo-wide.

The rule

Internal src modules import package types directly from @agent-device/*. Only published entry surfaces may re-export. The kept re-export surfaces (all feed the npm exports map or the public client type surface): src/sdk/* entries, src/client/client-types.ts, src/finders.ts, src/metro/{client-metro,metro-types}.ts, src/remote/remote-config-schema.ts.

What moved

Eleven internal re-exports removed; ~110 import sites across 115 files redirected to the packages (git diff is one-line import rewrites, −24 net):

removed from symbol(s) sites
core/dispatch.ts + core/dispatch-context.ts CommandFlags 39
daemon/types.ts SessionAction 19
request/progress.ts RequestProgressEvent/Sink 5
snapshot/snapshot-quality.ts SnapshotQualityVerdict 4
client/client-companion-tunnel-contract.ts CompanionTunnelScope, MetroBridgeScope 4
commands/cli-grammar/flag-types.ts CliFlags 3
utils/screenshot-result.ts ScreenshotResultData 1
commands/command-input.ts RepeatedInput 1
snapshot/snapshot-diff.ts SnapshotDiffLine/Summary 1
daemon/daemon-command-registry.ts RefFrameEffect dead
commands/capture/runtime/snapshot.ts DiffSnapshotCommandResult dead
core/batch.ts DaemonBatchStep 0 internal (entry takes runBatch only)

Entry-surface chains that previously laundered through a second internal module now re-export from the package (client-types.ts / client-metro.tsMetroBridgeScope from contracts/remote).

Measured side effect

The R9 type cycle shrinks again: 49 → 47 (daemon-server 19 → 17) — core/dispatch.ts's type re-export was itself a cycle edge. Ceilings lowered to the measured values.

Verification

  • Typecheck, lint, format, check:layering (47/17), fallow audit (115 changed files) + production-exports: green.
  • Full src suite: 609 files / 5,341 tests green. (One earlier run hit two pid-liveness assertion flakes in device-claim tests — pass in isolation and on rerun, pre-existing class per test: make pid-liveness fixtures deterministic under load #1556's own commit message; test-side hardening flagged separately, no product code involved.)

…internal modules

Post-#1636 review feedback: internal src modules were re-exporting package
types (export type { X } from '@agent-device/...'), giving one declaration
several import paths and hiding its provenance. New rule applied repo-wide:
internal modules import directly from the owning package; only published
entry surfaces (src/sdk/* entries, client-types, finders, metro composition,
remote-config-schema) may re-export.

Eleven internal re-exports removed and ~110 import sites redirected to the
packages, the big two being CommandFlags (core/dispatch chain, 39 sites) and
SessionAction (daemon/types.ts, 19 sites). Two were already dead
(RefFrameEffect via daemon-command-registry, DiffSnapshotCommandResult via
capture/runtime/snapshot). Entry-surface chains now re-export from the
package rather than laundering through a second internal module
(client-types/client-metro MetroBridgeScope).

Side effect: the R9 type cycle shrinks again, 49 -> 47 (daemon-server
19 -> 17); ceilings lowered to match.
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.99 MB 1.98 MB -428 B
JS gzip 634.8 kB 634.7 kB -105 B
npm tarball 769.1 kB 768.3 kB -823 B
npm unpacked 2.70 MB 2.70 MB -2.5 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.6 ms 29.5 ms +0.9 ms
CLI --help 67.7 ms 67.4 ms -0.3 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/interaction.js -223 B -47 B
dist/src/context.js -97 B -22 B

@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Reviewed exact head 7fe952e. One P2 remains: src/cli-schema/command-schema.ts:14 still re-exports CliFlags imported from @agent-device/contracts/command, although this is an internal module and no current consumer uses that re-export. That preserves exactly the internal type-laundering path this PR says it removes. Drop CliFlags from the export list (and the now-unneeded local import if applicable); internal consumers already import from the owning contracts package. The remaining rewrites preserve type-only semantics and deterministic checks are green. iOS Smoke is also red on testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden; the identical failure reproduces on main, so it is separate owner-action CI rather than a #1640 code finding. No readiness label while the code finding and confirmed failing CI remain.

The one consumer (cli/parser/args.ts, a multi-line import the sweep's
single-line scan missed) now imports CliFlags from contracts/command;
FlagDefinition/FlagKey stay — they are src-declared types, not package
laundering.
@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

P2 addressed at 586ab726bcommand-schema.ts no longer re-exports CliFlags. One correction to the finding: there was one consumer — cli/parser/args.ts pulled CliFlags through a multi-line import statement (the same single-line-scan blind spot that let my sweep miss this re-export in the first place); it now imports from @agent-device/contracts/command directly. FlagDefinition/FlagKey remain re-exported there deliberately: they're src-declared types from cli-grammar/flag-types.ts, not package laundering, so they're outside this PR's rule. Verified: typecheck, lint, format, layering (47/17 unchanged), fallow, and the cli/cli-schema suites green. The iOS Smoke red is the main-reproducing testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden failure you confirmed as separate owner-action CI.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 6, 2026
@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 586ab726. The prior P2 is fully resolved: CliFlags is no longer re-exported from internal src/cli-schema/command-schema.ts, and its sole missed consumer now imports directly from @agent-device/contracts/command. A head-wide search found no remaining internal CliFlags laundering path. The follow-up delta is limited to that import/export correction, passes diff checks, and all exact-head checks—including typecheck, package smoke, integration, coverage, layering, and iOS/macOS/Android smoke—are green. No device evidence is needed for this type-only refactor. No code findings; ready for human review.

@thymikee
thymikee merged commit d5f11f6 into main Aug 6, 2026
30 checks passed
@thymikee
thymikee deleted the claude/direct-contracts-imports branch August 6, 2026 13:22
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-06 13:23 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant