Repository navigation
perf(daemon): lazy-load Maestro in request binding; budget the daemon entry - #3284
Conversation
…a flow Request binding sits in the daemon's startup closure, so its static import of @agent-device/maestro evaluated the engine and the YAML parser (37 maestro + 72 yaml modules in the built output) on every daemon start. The flow is now inspected behind a function-scoped import reached only once the replay entry resolves to Maestro; native .ad replays and every other command never load it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
src/daemon.ts is what every daemon start evaluates, yet no budget row watched it, so a static edge back into Maestro would have regrown startup silently. As a hub it now falls under the merge-base no-growth rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
This PR is ready: the change looks right at f9c9b82, and the lazy Maestro load in request binding does what the title says. Not blocking: the new Maestro test in One open thread from another reviewer still applies: malformed bundle entry is low priority. It only matters for a malformed wire payload, where a non-string The Linux Smoke Tests job failed in the apt-get dependency install step on a timeout, before any repo code ran, and this diff does not touch that step, so it looks unrelated. Please re-run that job. Coverage and two other Smoke Tests jobs were still running when I looked, so they need to finish green before merge. I did not recount the daemon closure, re-run the planted regression, or re-measure startup latency. |
thymikee
added
the
ready-for-human
Making the Maestro engine lazy hoisted the bundle-entry read and format resolution out of the try that tolerates invalid replay plans (#1802). The HTTP boundary validates flags only as an object, so a wire request can carry a bundle whose entry is missing or non-string, and path.extname would then reject the request with a TypeError during advisory lock selection instead of falling back to normal device resolution. Wrap the whole probe in the advisory guard again; the engine import stays inside it.
…elper Keeping the whole probe inside the advisory guard grew its closure past the fallow complexity threshold. Split the maestro and .ad plan reads into named helpers so the guarded closure only resolves the bundle entry and routes by format; the #1802 tolerance stays in readAdvisoryResolution alone.
|
This PR is ready. The code at 651c22c is clean, and the earlier review of f9c9b82 is addressed. The new try block now covers the All 19 checks pass at 651c22c. I did not run the tests locally. I judged the regression coverage by reading the f9c9b82 code path. Not blocking: in https://github.com/callstack/agent-device/blob/651c22c/src/daemon/replay-device-selection.ts#L52 the The cubic-dev-ai P3 thread on replay-device-selection.ts lines 37-44 is fixed at this commit, so please resolve it: #3284 (comment) Nothing else is needed before a human merge. |
|
1 participant
Summary
Closes #3282 (part of #3276). 4 files.
src/daemon/replay-device-selection.ts) imports@agent-device/maestrothrough a function-scopedawait import, reached only when the replay entry resolves to a Maestro flow..adreplays and other commands never load it. In the built output, daemon boot drops exactly 109 modules (37 maestro + 72 yaml): 209→201 files, 2.33→2.17 MB; a sourcemap diff shows no other module moves.src/daemon.tsjoinsHUB_ENTRY_FILES, under the merge-base no-growth rule (621→571 here).startDaemonRuntimeawaitscreateDaemonProviderRuntimeComposition→createDefaultRuntimes(env)before it listens (daemon-runtime.ts:361vs:602). A dynamic import loads the same chunk in the same phase and only lowers the static count; the chunk evaluates in ~1.4 ms, too little to justify making runtime construction lazy.Validation
Commit
651c22ccb:AGENT_DEVICE_VITEST_MAX_WORKERS=2 pnpm check:affected --run— all runnable checks passed. CI at this head: all jobs green, incl. Compatibility & Provenance and live web smoke.src/daemon.tshub-row no-growth check (621 vs 571, route printed); without the row, the plant passes.lease_allocate(n=20): cold 297→295 ms, warm 86→87 ms.DEVICE_NOT_FOUND);lease_allocatereaches the credential check.readAdvisoryResolution, test mutation-proven at the pre-fix shape. Fallow complexity fixed by decomposition, no baseline change.b27e310: not reproducible, not this change. The lane passes locally on the built output (n=2) and on CI atf9c9b82and651c22. Its artifact showsweb setupstalled mid managed-install download (nomanifest.json, no daemon ever started).web setupis CLI-local and never starts a daemon, so the lazy branches are off-path. Runner-network flake.🤖 Generated with Claude Code