Skip to content

refactor: extract contracts workspace package - #1499

Merged
thymikee merged 3 commits into
mainfrom
agent/1490-w1a-coordinator
Jul 30, 2026
Merged

refactor: extract contracts workspace package#1499
thymikee merged 3 commits into
mainfrom
agent/1490-w1a-coordinator

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

Extract shared contracts into the private @agent-device/contracts workspace package behind 12 domain-oriented subpaths.

Keep mutable runtime composition and registries in the root package, enforce zero package back-imports, and reduce the largest type SCC from 102 to 76.

This is W1a of #1490. The large file count is predominantly contract moves and mechanical import rewrites.

Validation

  • pnpm check
  • VITEST_MAX_WORKERS=1 pnpm check:affected --run
  • package artifact check: no unresolved internal package specifiers or internal runtime dependencies
  • normalized size delta vs origin/main: +1.1 kB raw JS, -502 B gzip, -1.7 kB tarball, -4.9 kB unpacked

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.90 MB 1.90 MB +1.1 kB
JS gzip 610.1 kB 609.7 kB -466 B
npm tarball 729.2 kB 727.4 kB -1.7 kB
npm unpacked 2.55 MB 2.55 MB -4.8 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 18.1 ms 18.4 ms +0.3 ms
CLI --help 37.8 ms 39.3 ms +1.5 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/viewport-dimension.js +73.2 kB +23.9 kB
dist/src/device2.js +62.5 kB +18.5 kB
dist/src/screenshot-result.js +52.5 kB +16.9 kB
dist/src/internal/daemon.js -1.6 kB -339 B
dist/src/sdk-batch-runner.js +886 B +286 B

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-30 15:08 UTC

@thymikee

Copy link
Copy Markdown
Member Author

Review finding — not ready yet:

packages/contracts/src/diff.ts replaces the exact ArtifactDescriptor and ScreenshotDiffResult aliases with Record<string, unknown> and a truncated region shape. The production diff screenshot route still returns the full result (dimension mismatch, complete region diagnostics, OCR/non-text fields where present, and discriminated artifact descriptors), so DiffScreenshotCommandResult no longer accurately describes its runtime output. That weakens the typed contract boundary W1a is intended to enforce.

Please keep the screenshot-diff result contract local and exact for now, or first move its complete neutral vocabulary below the package boundary. DiffSnapshotCommandResult can still move independently. Validate the corrected contract through typecheck/build and the packaged CLI/contract gates.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed current head 4d925ae. The earlier screenshot-diff contract finding is resolved: the full ScreenshotDiffResult & { artifacts? } remains local to the runtime route, while the contracts package exports only the closed snapshot-diff contract. W1a keeps mutable provider/runtime composition in root, preserves ADR 0003/0008/0009 ownership boundaries, and adds non-vacuous package-boundary regression coverage. No blocking code findings; ready for human review while iOS Smoke and Coverage finish. Residual risk: this is a 509-file mechanical move, and no separately authorized cross-vendor review was performed.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 30, 2026
@thymikee
thymikee force-pushed the agent/1490-w1a-coordinator branch from 4d925ae to e4cdf8d Compare July 30, 2026 13:33
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in e4cdf8d16.

  • DiffScreenshotCommandResult is local again and composes directly from the implementation-owned ScreenshotDiffResult and exact discriminated ArtifactDescriptor.
  • @agent-device/contracts/capture now exports only the independently movable snapshot-diff vocabulary for this area.
  • The root snapshot-diff implementation imports and re-exports the package-owned snapshot aliases, retaining one source of truth.
  • The final rebase also migrated two newer main call sites that still referenced the removed internal contract paths.

Validation:

  • pnpm check:affected --run (all runnable checks passed)
  • 5,129 coverage tests, 4,891 unit tests, and 150 provider-integration tests passed
  • typecheck, declaration build, package-boundary enforcement, packaged Node/CLI smoke, and replay compatibility passed

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 30, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed e4cdf8d: W1a's code delta remains clean after the rebase, and the earlier screenshot-diff finding stays fixed—the exact result type is local again and the contracts package exposes only closed snapshot-diff vocabulary. Android Smoke is nevertheless red: smoke:keyboard-ime runs for about 130 seconds, hits the 120-second subprocess timeout, and cleanup also fails, so capture/close and replay never execute. The identical failure occurs on docs-only #1500 and on main cd9a7ce, making this a merged-main Android lifecycle blocker rather than a contracts-extraction regression. ready-for-human was removed until Android Smoke is green. All other #1499 checks, including iOS, Coverage, package smoke, type/layering, and integration, are green. Residual risk: this remains a 510-file mechanical move, and no separately authorized cross-vendor review was performed.

@thymikee

Copy link
Copy Markdown
Member Author

Follow-up complete:

  • rebased/verified against current origin/main
  • no unresolved review threads remain
  • stabilized the Android keyboard smoke by focusing from the prepared snapshot geometry and polling the structured keyboard status, avoiding the unrelated click --settle path that hung in CI
  • preserved the screenshot-diff result contract from the prior review fix

Validation: pnpm check:affected --run passed locally; all GitHub checks are now green, including Android smoke and the Coverage rerun.

@thymikee

Copy link
Copy Markdown
Member Author

Re-review at dca364547: clean. The Android regression now focuses the snapshot-derived email rect and waits for the actual system IME instead of treating best-effort UI settling as readiness; it still proves visible-to-dismissed keyboard state, pixel change, and preserved form navigation. Exact-head Android Smoke—including keyboard-ime and replay—and every other check are green. Ready for human review. Residual risk: no separately authorized cross-vendor pass.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 30, 2026
@thymikee
thymikee merged commit 0ee2a86 into main Jul 30, 2026
46 of 47 checks passed
@thymikee
thymikee deleted the agent/1490-w1a-coordinator branch July 30, 2026 15:07
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