refactor: isolate maestro engine package

[thymikee]

Summary

Package Maestro behind the single private @agent-device/maestro root facade.
Keep the root declaration/re-export-only, split implementation by capability, move daemon authority adapters under src/daemon/adapters/maestro, and keep ranking/conformance internals private.
Centralize Maestro-compatible discovery/format routing, require explicit --maestro for YAML, and remove superseded routing.

The follow-up also keeps yaml external as the baseline runtime dependency, removes the broad shared chunk, and consolidates fixture sealing and YAML-path recognition to one implementation each.

Part of #1478

Validation

• Maestro conformance: 46/46
• Focused facade/routing/discovery/failure/artifact tests: 49/49
• pnpm check:tooling
• Packed CLI smoke: clean install, --help, and --version
• Size versus base: +5.3 kB raw, +1.8 kB gzip, +1.7 kB tarball
• 31-run startup: +1.1 ms --version, +0.8 ms --help
• pnpm check:affected --run: format/lint/typecheck/layering/Fallow/MCP/build and 5,177 tests passed; five unrelated Android subprocess timeouts in two files then passed isolated 25/25

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.90 MB	1.91 MB	+5.2 kB
JS gzip	609.8 kB	611.6 kB	+1.7 kB
npm tarball	727.6 kB	729.3 kB	+1.7 kB
npm unpacked	2.55 MB	2.55 MB	+5.3 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.3 ms	28.1 ms	+0.8 ms
CLI --help	58.3 ms	58.1 ms	-0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/internal/daemon.js	+27.0 kB	+8.0 kB
dist/src/session.js	-21.7 kB	-6.0 kB
dist/src/cli.js	-18 B	-134 B
dist/src/sdk-selectors.js	-90 B	-30 B
dist/src/viewport-dimension.js	-7 B	-15 B

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-30 18:59 UTC

[thymikee]

Reviewed at 4e90f3dd. The extraction is solid and the hard boundary work is done correctly. One substantive concern about façade width, plus two smaller notes.

Verified against the P2 brief

I checked each "Done" criterion mechanically rather than by reading the summary:

• packages/maestro imports no root src/, daemon, platform, provider, or native replay — grep returns nothing. package.json depends only on @agent-device/contracts, @agent-device/kernel, and yaml, and exports only .. ✅
• .ad never enters Maestro — no .ad reference anywhere under packages/maestro/src. ✅
• runTypedReplayIfNeeded deleted — gone from the tree, not shimmed. ✅
• Format routing is a pure resolver — src/replay/format.ts is 17 lines, extension + explicit backend only, no content probing and no cross-engine fallback. The doc comment states both non-goals. ✅
• Discovery unified — src/daemon/replay-source-discovery.ts is the single expansion implementation, and session-test-discovery.ts drops 182 lines to consume it plus resolveReplayFormat at :53 and :142. ✅
• Typed failure identity from outcomes — session-replay-maestro-failure.ts:1 now imports MaestroFailedAction from the package instead of rebuilding it from observer state, and the file sheds most of its former weight. ✅

Also worth saying: splitting the 314-line runtime port into daemon-runtime-port.ts + daemon-runtime-tap.ts is a good call, not scope creep — the old src/compat/maestro/daemon-runtime-port.ts carried 17 tap-related references in one file.

Main concern: the façade exports four of the six things the brief says stay private

The brief is specific: "Add one Maestro façade and route non-test consumers through it. Keep engine IR, parser, plan, ranking, expression state, and compatibility policy private."

packages/maestro/src/index.ts currently exports, with the consumers I traced:

export	brief category	consumer
rankMaestroFailureCandidates	ranking	src/daemon/handlers/session-replay-maestro-failure.ts — a daemon handler, not an adapter
MAESTRO_COMPATIBILITY_PRESETS, MAESTRO_DEFAULT_SETTLE_TIMEOUT_MS	compatibility policy	4 files under src/daemon/adapters/maestro/
SUPPORTED_MAESTRO_COMMAND_NAMES, parseMaestroConformanceSource	parser	scripts/maestro-conformance/verify.ts only
MaestroPlatform, MaestroSelector (from internal/program-ir.ts)	engine IR	type-only

These aren't equivalent and I'd treat them differently:

• rankMaestroFailureCandidates is the one I'd push back on. Ranking is named explicitly as private, and its consumer is a daemon handler rather than an adapter — so the export exists to serve daemon-side failure presentation. Either the ranking call belongs inside executeMaestroFlow's outcome (so the daemon receives already-ranked candidates and never invokes the ranker), or session-replay-maestro-failure.ts moves under src/daemon/adapters/maestro/ where engine-adjacent knowledge is expected.
• The conformance exports are the weakest justification. SUPPORTED_MAESTRO_COMMAND_NAMES and parseMaestroConformanceSource exist solely for a script under scripts/. That's #1478's "expose implementation files merely to make imports compile", and a subpath isn't the escape hatch either — the issue reserves focused subpaths for leaf vocabulary packages and says the deep Maestro/replay-test/ad-replay packages export only their façade at .. Moving the conformance harness inside the package would keep the parser private and cost nothing externally.
• The policy constants and IR types are the most defensible, since the adapters genuinely need settle timeouts and selector/platform vocabulary to implement MaestroRuntimePort. But if they stay, it's worth a comment in index.ts recording why those specific ones are public, so the next extraction doesn't read the current surface as licence to widen further.

None of this is a STOP condition — there's no shared replay VM, no parser probing, no fallback engine, and DaemonRequest hasn't been promoted into contracts. It's a "does the façade match its stated contract" question, and right now it doesn't.

Two smaller notes

1. Bare .yaml routes to the native engine. resolveReplayFormat('flow.yaml', undefined) returns 'ad', which matches the brief exactly (".yaml/.yml means Maestro only when the Maestro backend was explicitly requested"). But it means a user running replay flow.yaml without --maestro hands a YAML file to the .ad parser. Could you confirm the resulting error names the missing --maestro flag rather than surfacing a raw parse failure? The routing is right; I'm only asking about the diagnostic.

2. Discovery ownership now overlaps P3. session-test-discovery.ts:5 imports inspectMaestroFlow from the package for Maestro titles. That's correct here, but #1478 also says replay-test owns discovery policy — so P3 will need a split rather than a move. The workable line looks like: host owns path expansion and per-file manifest inspection (this PR's replay-source-discovery.ts + resolveReplayFormat), replay-test owns filtering, ordering, skip decisions and skip-message text. Flagging so P3 doesn't try to move what you've just correctly placed in the daemon.

Testing

Test topology mirrors the moves, the conformance oracle is preserved, and android-test-suite.test.ts being substantially rewritten (669 lines) is expected given routing changed underneath it. Nothing here looks like coverage lost to the move.

---

Generated by Claude Code

[thymikee]

Correcting my earlier review: I raised the façade width as an architecture-contract question and missed that it has already caused a measured bundle and startup regression. The Bundle Size check is informational, so it stayed green.

The regression

From the size report on 4e90f3dd:

Metric	Base	Current	Diff
JS raw	1.90 MB	2.02 MB	+116.1 kB
JS gzip	609.8 kB	645.0 kB	+35.2 kB
npm tarball	727.6 kB	762.6 kB	+35.0 kB
CLI --help	61.0 ms	68.1 ms	+7.1 ms (~12%)

Chunk movement tells the story:

Chunk	Raw diff
dist/src/src.js	+181.7 kB
dist/src/session.js	−35.2 kB
dist/src/internal/daemon.js	−18.9 kB
dist/src/cli.js	−8.4 kB
dist/src/cli-help.js	−1.8 kB

Roughly 64 kB left chunks that were loaded lazily and ~182 kB landed in the shared chunk every entrypoint evaluates. --help now pays for the Maestro engine and yaml, which it has no reason to touch. --version is unchanged (−1.1 ms), consistent with it short-circuiting before the shared chunk matters.

This is a regression against a stated non-negotiable, not just a size preference — #1478 lists "lazy platform loading" in both the compatibility rules and the success metrics.

Why it happens

packages/maestro/src/index.ts is an implementation module rather than a re-export barrel:

• 17 static module-scope imports covering the parser, IR, runtime port, execution, conformance normalization, and compatibility policy;
• three runtime definitions in the file itself — MAESTRO_CONFORMANCE_CONSTANTS (:134), parseMaestroConformanceSource (:143, which calls parseMaestroProgram), and createMaestroRuntimePort (:155, a real function body).

So any import from @agent-device/maestro — including a type-only one that should erase — evaluates the entire engine graph. yaml is statically imported by six internal modules (program-ir-parser.ts, program-ir-values.ts, program-ir-command-parser.ts, program-ir-flow-parser.ts, program-ir-gesture-parser.ts, export-yaml.ts), all reachable from that graph, so it comes too.

This is the same root cause as my façade-width note, which I under-called. The conformance exports I described as "the weakest justification" are exactly what pins the parser into the static graph.

Suggested direction

1. Make index.ts a pure re-export barrel. Move createMaestroRuntimePort's body to internal/runtime-port-factory.ts and re-export it; the façade should contain no logic of its own.
2. Get the conformance surface out of the production entry. MAESTRO_CONFORMANCE_CONSTANTS, parseMaestroConformanceSource, and SUPPORTED_MAESTRO_COMMAND_NAMES serve only scripts/maestro-conformance/. Moving that harness inside the package removes the parser from the public graph entirely, which is also what my earlier note recommended on contract grounds.
3. Keep the engine behind a dynamic import at the daemon adapters, so it loads when a Maestro replay actually runs rather than at CLI start. refactor: isolate Limrun construction dependencies #1502 (W1c) has a good pattern for this, including a module-load regression test that counts mocked-module factory invocations to prove construction doesn't pull the helpers. The same test shape would pin --help against re-acquiring the engine.
4. Consider making the size check gate rather than inform, at least on a raw-bytes or startup-ms threshold. This regression was visible in CI the whole time and passed, and the remaining extractions (P3, P5, W1b) will each move code across chunk boundaries in the same way. A ratchet here would catch the next one automatically.

Verification for whichever fix lands: dist/src/src.js should return to roughly its base size, --help to ~61 ms, and yaml should not appear in the static import header of the shared chunk.

Apologies for the miss on the first pass — the façade observation was right there and I did not follow it to its runtime cost.

---

Generated by Claude Code

[thymikee]

Blocking follow-up resolved at 901b7aa80b (rebased onto current origin/main).

Review resolutions:

• packages/maestro/src/index.ts is now declaration/re-export-only. Execution/inspection, export, runtime-port construction, and support vocabulary live in focused internal facade modules; no public subpaths were added.
• Conformance-only parser/canonicalization/constants moved under packages/maestro/test/conformance and no longer widen the production facade.
• Ranking policy and types remain private. The daemon consumes a failure-only package capability through src/daemon/adapters/maestro/failure-snapshot.ts; normal target resolution carries no candidate/reporting data.
• The exported runtime adapter policy contains only values used by real daemon adapters. Parser, plan, expression, ranking, and broad compatibility-policy state stay private.
• Bare .yaml/.yml replay without explicit Maestro routing now returns the clear --maestro diagnostic; there is no content probing or fallback.
• yaml stays a baseline-compatible root runtime dependency and is external in emitted production chunks. The narrow Fallow exception documents that package source owns the import while the published root artifact retains it.
• sideEffects: false is safe because the private package has no registration/bootstrap side effects or observable top-level mutation; its root-facade test enforces the declaration/re-export-only shape.
• Fixture sealing now has one package-owned implementation used by verification and regeneration. The duplicate script helper and stale Fallow export exception were deleted.
• Test artifact handling now reuses central isMaestroYamlPath; the adjacent .yaml/.yml predicate was deleted.

Bundle/package A/B:

Metric	Delta
JS raw	+5.3 kB
JS gzip	+1.8 kB
npm tarball	+1.7 kB
CLI --version median, 31 runs	+1.1 ms
CLI --help median, 31 runs	+0.8 ms

dist/src/src.js is gone, dist/src/script.js is narrow, CLI/help do not import the Maestro parser chunk, and the daemon chunk retains the external yaml import. A clean packed install runs --help and --version.

Validation:

• Maestro conformance: 46/46
• Focused facade/routing/discovery/failure/artifact tests: 49/49
• Tooling gate: green
• Affected gate: format/lint/typecheck/layering/Fallow/MCP/build and 5,177 tests passed before five unrelated Android subprocess timeouts in two files
• Both timeout-affected files passed isolated: 25/25, confirming contention

The PR remains draft for independent review.

[thymikee]

Reviewed 901b7aa: clean P2 boundary/routing cutover. Maestro now exposes only its root facade; engine/parser/plan/ranking remain private, daemon authority stays in src/daemon/adapters/maestro, .ad remains native under --maestro, and YAML requires explicit --maestro. The central discovery resolver, failure-outcome provenance, conformance, layering, replay-compat, coverage, and bundle-size gates are green. Keep this draft/unlabeled until the exact-head package/device checks finish and Android+iOS Maestro-flow evidence for the moved adapter is attached. Residual risk: no separately authorized cross-vendor pass.

[thymikee]

Exact-head follow-up for 901b7aa80b5681e9587cc98901b3430aa142f2ba.

Live Maestro adapter evidence

iOS — iPhone 17 Pro simulator (C25DBB5B-9254-4293-A8D5-2785C78DE03A)

node bin/agent-device.mjs \
  --session pr1506-maestro-ios \
  --state-dir /private/tmp/agent-device-pr1506-ios \
  test examples/test-app/maestro/checkout-form.yaml \
  --maestro \
  --platform ios \
  --udid C25DBB5B-9254-4293-A8D5-2785C78DE03A \
  --metro-port 8081 \
  --artifacts-dir /private/tmp/pr1506-maestro-ios-artifacts \
  --timeout 180000

Result:

✓ Checkout form 45.0s
Test summary: 1 passed (1) in 45.0s

This is the unmodified repository flow. It exercised Maestro YAML discovery/parsing, launchApp, assertions, file runFlow, platform conditions, scrolling, selectors, text entry, swipe, and repeat control flow through the moved daemon adapter. Preflight snapshot showed the expected Agent Device Tester surface.

Android — Pixel 9 Pro XL API 37 emulator (emulator-5554)

The repository release test app was installed so launchApp(clearState: true) exercised the app directly rather than Expo Dev Launcher state. The focused evidence flow exercised launchApp with clear state, text/id assertions, scrollUntilVisible, tapOn, and swipe:

appId: com.callstack.agentdevicelab
name: Android Maestro adapter evidence
onFlowStart:
  - launchApp:
      clearState: true
  - assertVisible: Agent Device Tester
onFlowComplete:
  - assertVisible: Delivery choices
---
- scrollUntilVisible:
    element:
      id: home-open-form
- tapOn:
    id: home-open-form
- assertVisible: Checkout form
- swipe:
    start: 50%, 75%
    end: 50%, 35%
    duration: 300
- scrollUntilVisible:
    element:
      text: Delivery choices
- assertVisible: Delivery choices

node bin/agent-device.mjs \
  --session pr1506-maestro-android-evidence \
  --state-dir /private/tmp/agent-device-pr1506-android-release \
  test /private/tmp/pr1506-maestro-android-evidence.yaml \
  --maestro \
  --platform android \
  --artifacts-dir /private/tmp/pr1506-maestro-android-evidence-final-artifacts \
  --timeout 180000

Result:

✓ Android Maestro adapter evidence 5.77s
Test summary: 1 passed (1) in 5.77s

Preflight snapshot showed Agent Device Tester and confirmed androidSnapshot.backend=android-helper, helperVersion=0.20.3, API version 2, persistent-session transport.

All verification sessions auto-finalized or were explicitly closed; session lists were empty afterward. Metro was stopped and the emulator booted for this check was shut down.

CI

All exact-head checks are now green. The initial iOS fixture E2E failure reported structured wait_capture_stalled while waiting for Agent Device Tester; it was unrelated to the Maestro extraction and passed on the exact-head rerun. Passing iOS smoke rerun.

No source changes or push were needed. The PR remains draft at the same head.