Skip to content

refactor(replay-port): own the replay backend contract in contracts - #3622

Open
thymikee wants to merge 4 commits into
mainfrom
refactor/maestro-replay-backend-plugin-shape
Open

thymikee wants to merge 4 commits into
mainfrom
refactor/maestro-replay-backend-plugin-shape

Conversation

@thymikee

@thymikee thymikee commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

First half of the #3377 replay-backend prep, split from the original PR to stay under the diff budget. Behavior, wire shapes and help text are unchanged; the adapter stays in replay-port. Layer 2 (stacked on this) moves it into the Maestro package as a create(host) plugin.

  • New @agent-device/contracts/replay-backend: the replay session and command types (moved from replay-port command-types), the backend capability type, the failure descriptor, and the key-locked ReplayBackendDispatchOptions (a Pick of ReplayDispatchOptions, including killApp from feat(maestro): support killApp via close mode #2749).
  • ReplayCommand is derived from ReplayBackendCommand, so the shared fields have one definition; importers were repointed.
  • The divergence-failure response moved out of the Maestro failure module into replay-backend-failure.ts in replay-port; the Maestro failure module builds the descriptor and calls it. Existing daemon failure tests cover it unchanged.
  • The engine's MaestroDaemonDispatchOptions aliases the contracts type, so the dependency points from the engine to core.

The contracts module is the only place the plugin contract can live: a type import of replay-port from maestro would be a circular tsc -b project reference.

30 files touched. Refs #3377.

Validation

Tested SHA: e59514f.

  • pnpm check:affected --run: all runnable checks passed.
  • pnpm check:quick and focused vitest (replay-port, maestro, daemon sessions, contracts, CLI, closure budgets) passed.
  • Diff: +444/−348 against origin/main.
  • Coverage and provider integration are CI-authoritative.

🤖 Generated with Claude Code

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 55 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/replay-port/src/daemon-port/replay-backend-failure.ts
Comment thread packages/contracts/src/replay-backend.ts
Comment thread scripts/__tests__/eager-closure-budgets.ts Outdated
Comment thread packages/maestro/src/daemon-port/daemon-runtime-public-operation.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-3622/

Built to branch gh-pages at 2026-10-14 17:53 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.24 MB 5.24 MB +360 B
Package (unpacked) 5.24 MB 5.24 MB +360 B
Package (download) 1.58 MB 1.58 MB +60 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.3 ms 28.0 ms +0.8 ms
CLI --help 80.2 ms 76.8 ms -3.4 ms

@thymikee
thymikee force-pushed the refactor/maestro-replay-backend-plugin-shape branch from 2b70947 to 04f53b7 Compare October 14, 2026 17:56
@thymikee thymikee changed the title refactor(maestro): shape the replay backend as a create(host) plugin refactor(replay-port): own the replay backend contract in contracts Oct 14, 2026
@thymikee

Copy link
Copy Markdown
Member Author

The code at 04f53b7 keeps behavior unchanged, and the contract move looks right. One small fix is needed before merge: the isControl doc at https://github.com/callstack/agent-device/blob/04f53b7/packages/contracts/src/replay-backend.ts#L188 says control-flow steps fail without a post-failure screen read, but replay-backend-failure.ts still captures the screen and only skips suggestions, as the old code did. Please reword the doc and keep the code. This is the same point as the open thread at #3622 (comment).

CI is still running, and no check has failed. Smoke Tests may exercise replay, which overlaps the moved failure-response route, so it should finish green before merge.

Not blocking, take or leave: ReplayBackendStep at https://github.com/callstack/agent-device/blob/04f53b7/packages/contracts/src/replay-backend.ts#L96 copies ReplayTestAttemptStep in replay-test, so one could alias the other. Also, ReplayBackendActionFailure at https://github.com/callstack/agent-device/blob/04f53b7/packages/contracts/src/replay-backend.ts#L170 carries host handles (req, sessionStore, observationStore, logPath) that the backend only hands back. Could the host-provided failure builder close over them in Layer 2, so the descriptor keeps only backend-known fields?

On the other open threads: the closeAppOnly/killApp boolean widening (#3622 (comment)) still applies, but it is type widening only and no producer sends false. These threads do not apply, so you can resolve them: the maestro failure test file does not exist at this head (#3622 (comment)); the SessionAction[] inside Readonly<> was moved verbatim, so it is not a regression (#3622 (comment)); and this head does not change eager-closure-budgets.ts (#3622 (comment)).

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/maestro/src/daemon-port/daemon-runtime-public-operation.ts Outdated
@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready to merge at e59514f. The earlier findings at 04f53b7 are fixed: the isControl doc now says the screen is still captured, and the presence-only ?: true types are restored for closeAppOnly, killApp and observationOnly. All 36 checks are green. The change since the last review is a doc comment plus a type-only narrowing, so no runtime route changes. I did not run typecheck locally, so the compile check rests on CI.

Not blocking: ReplayBackendStep in replay-backend.ts is a field-for-field copy of ReplayTestAttemptStep in replay-test, and replay-test already depends on contracts. You could alias ReplayTestAttemptStep = ReplayBackendStep so contracts owns the one definition. Take it or leave it.

Of the earlier inline threads, these no longer apply, so please resolve them. The isControl doc is fixed at this head. The missing test file is outdated, since that file does not exist here. The actions: SessionAction[] thread covers a type moved verbatim from replay-port, so this move did not introduce it. The eager-closure budgets thread is moot because this head does not change that file. The narrowing thread and the per-key doc thread are both fixed in daemon-runtime-public-operation.ts. No open threads still apply.

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

This branch has not been deployed

No deployments
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