feat(cli): make local-assistant lifecycle ops programmatically reusable#32619
Conversation
Add an injectable LifecycleReporter so hatchLocal/retireLocal can report progress and log output through a sink instead of writing to the console directly, and return structured results instead of void. The CLI keeps its exact terminal output (and the HATCH_PROGRESS stdout protocol under VELLUM_DESKTOP_APP) via a default console reporter, while in-process callers can inject their own reporter and read results without scraping stdout. Curate package exports for hatch-local, retire-local, guardian-token, and lifecycle-reporter so a second host (e.g. the Electron main process) can import these functions directly. LUM-2051 Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8e38369a0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| runtimeUrl = await startGateway(watch, resources, { | ||
| signingKey, | ||
| bootstrapSecret, |
There was a problem hiding this comment.
Route nested hatch output through the reporter
When an in-process caller supplies options.reporter, this still invokes the nested lifecycle helpers without any reporting hook; startGateway (and the preceding startLocalDaemon) write status and warning messages directly with console.log/console.warn in cli/src/lib/local.ts. In a normal successful hatch this means the new reusable API still pollutes stdout/stderr and the caller cannot observe all lifecycle output via LifecycleReporter, defeating the purpose of avoiding subprocess stdout scraping. Please thread the reporter through these helpers or otherwise suppress their console output when a custom reporter is provided.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, and correct — hatchLocal's own progress/log lines now route through the reporter, but the nested startLocalDaemon/startGateway helpers in lib/local.ts still console.log/console.warn directly, so an in-process caller with a custom reporter won't yet see every line through it.
Deliberately scoping that out of this PR rather than fixing it here. Threading the reporter through local.ts touches shared infrastructure with several callers, and it's better landed alongside the first real in-process consumer (the Electron adapter) so it can be validated against an actual LifecycleReporter rather than plumbed speculatively. Captured this as the explicit next layer in the PR description's "Scope boundary / known follow-up" section.
For the CLI today it's a non-issue: the default consoleLifecycleReporter already routes everything to the console, so output is unchanged. Resolving this thread with that decision noted.
…le mock teleport.test.ts registers a process-global mock.module for ../lib/retire-local.js and never restores it, so a separate retire-local test file resolves the mocked retireLocal in the full suite. The structured return is covered by tsc and the command-level retire/teleport tests; reporter routing is covered by lifecycle-reporter.test.ts. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
|
@noanflaherty flagging you here since this is the foundation your local-mode unbundling + the Electron IPC tickets (LUM-1997–2000) sit on — wanted to align before more gets built on top. What this PR does: makes Why it helps your work: the Electron main process can now What it deliberately does NOT do: it doesn't add any Electron IPC handler, and it doesn't touch your tickets or the ATL findings — purely additive Two things I'd like to align on for what happens next:
No coordination blocker either way — this can merge independently and your tickets consume it whenever they're ready. Just want to make sure we're not about to build the same thing twice. |
…b-programmatic-reuse
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
✦ APPROVE — reviewed at 12b5a9f3
Value: Eliminates subprocess spawning + stdout regex-scraping as the only path to hatchLocal/retireLocal results — gives the Electron main process a clean in-process API to hatch, retire, and receive the guardian access token directly, without parsing Hatching local assistant:\s+(.+) off stdout.
Full analysis
lifecycle-reporter.ts
Clean, minimal interface. progress / log / warn / error — nothing over-engineered. consoleLifecycleReporter is a faithful shim that preserves exact existing terminal output AND the HATCH_PROGRESS: stdout contract under VELLUM_DESKTOP_APP, so no existing subscriber breaks.
hatch-local.ts
All console.log/warn/error and emitProgress calls replaced with reporter.*. The reporter field in HatchLocalOptions defaults to consoleLifecycleReporter, so the CLI commands get zero behavioral change. HatchLocalResult surfaces exactly what an in-process caller needs: assistantId, runtimeUrl, localUrl, species, and guardianAccessToken? — the token is the linchpin (currently only recoverable by reading guardian-token.json off disk or scraping stdout).
logHatchNextSteps call updated to (message) => reporter.log(message) — correct.
The keepAlive path correctly uses reporter.log throughout. No missed console.* calls in the happy path or error paths.
retire-local.ts
Same pattern — reporter param with default. RetireLocalResult adds archived, archivePath?, sharedDataDir?. All three early-return paths now return structured data. Clean.
lifecycle-reporter.test.ts
Covers the two meaningful behaviors: (1) HATCH_PROGRESS: emission gated on VELLUM_DESKTOP_APP, (2) suppression without it. log/warn/error routing is verified. afterEach properly restores VELLUM_DESKTOP_APP. Good.
Dropped retire-local.test.ts
Legitimate removal — teleport.test.ts registers a mock.module for ../lib/retire-local.js that leaks globally and poisons a separate retire-local.test.ts. Coverage is preserved: RetireLocalResult shape is enforced by tsc, reporter routing is covered by lifecycle-reporter.test.ts, and command-level retire/teleport tests cover the command surface.
Codex P2: startLocalDaemon / startGateway still use console.*
Valid observation, correctly scoped out. Threading the reporter through lib/local.ts touches shared infrastructure with multiple callers — doing it speculatively before the first real in-process consumer (Electron adapter) exists would mean plumbing with no real validation target. Right call to defer to the LUM-1997–2000 work.
package.json exports
hatch-local, retire-local, guardian-token, lifecycle-reporter — exactly the surface Electron needs, nothing over-exported.
Vellum Constitution — Yours: the assistant's lifecycle operations now belong to the host that runs them, not to a terminal process that has to be scraped.
Prompt / plan
Foundation for the Electron-shell work tracked in LUM-2051 (macOS Electron project). The browser SPA needs privileged local-assistant operations (hatch, retire, guardian token); a host with OS/process access must perform them. Today the only non-terminal consumer — the dev-only
apps/web/vite-plugin-local-mode.ts— gets them by spawning the CLI as a subprocess and scraping stdout (it regex-matchesHatching local assistant:\s+(.+)to recover the new assistant id, and reads the guardian access token offstdout.trim()). A second host (the Electron main process, a Node process that canimportthese functions) hits a wall:hatchLocal/retireLocalreturnvoidand write everything throughconsole.log, andcli/src/lib/*isn't exported from the package, so there's no way to get a result back without scraping logs.This PR reshapes those ops to be consumable in-process, without changing any CLI behavior. It does not add the Electron IPC handlers themselves — that's downstream work (Noa's LUM-1997–2000) that consumes this surface.
What changed
cli/src/lib/lifecycle-reporter.ts): aLifecycleReporterinterface (progress,log,warn,error) plus a defaultconsoleLifecycleReporter. The console reporter routeslog/warn/errorto the console and mirrorsprogressto the existingHATCH_PROGRESS:stdout channel viaemitProgress.hatchLocalnow returnsHatchLocalResult(assistantId,runtimeUrl,localUrl,species, optionalguardianAccessToken) instead ofvoid, and reports throughoptions.reporter(defaulting to the console reporter) instead of callingconsole.*/emitProgressdirectly.retireLocalnow returnsRetireLocalResult(assistantId,archived, optionalarchivePath, optionalsharedDataDir) and takes an optionalreporterarg (defaulting to the console reporter).hatch-local,retire-local,guardian-token, andlifecycle-reporterso a second host can import them. Theguardian-tokenhelpers already returned structured data; they only needed exporting.Why it's safe (behavior preservation)
hatch,retire,recover,teleport) are unchanged — they don't pass a reporter, so they get the default console reporter and produce byte-for-byte identical terminal output.HATCH_PROGRESS:{step,total,label}stdout protocol is consumed by the already-shipped Swift macOS app (clients/macos/.../HatchingStepView.swift), which can run against a newer CLI. The default reporter still emits those exact lines underVELLUM_DESKTOP_APP, so the cross-component wire contract is preserved (asserted by a unit test). Per the macOS↔platform skew policy, an older client running a newer CLI keeps working.void→ result object) is additive: existing callersawaitand ignore the return.cli/AGENTS.md, user-facing CLI output still goes throughconsole.log/console.error(via the default reporter).Alternatives considered and rejected
cli/is a separate package)../src/lib/*export — exposes the entirelib/surface; a curated list keeps the public seam intentional.Scope boundary / known follow-up
This PR routes the top-level lifecycle output of
hatchLocal/retireLocal(their own progress + log lines) through the reporter. The nested startup helpers inlib/local.ts(startLocalDaemon,startGateway) stillconsole.log/console.warndirectly, so an in-process caller that supplies a custom reporter will not yet observe every line through it (flagged by Codex review, P2). Threading the reporter deeper intolocal.tsis intentionally not in this PR: those helpers are shared infrastructure with many callers, and the deeper plumbing belongs with the first in-process caller (the Electron adapter) so it can be validated against a real consumer rather than landed speculatively here. Tracked as the natural next layer on top of this foundation. For the CLI today this is a non-issue — the default reporter already routes to the console, so output is unchanged.Test plan
cd cli && bunx tsc --noEmit— clean.cd cli && bun run lint— clean (one pre-existing unrelated warning inroadmap.ts).cli/src/lib/__tests__/lifecycle-reporter.test.ts: console routing,HATCH_PROGRESSemission underVELLUM_DESKTOP_APP, and suppression when unset.cli/src/__tests__/{retire,teleport}.test.tsstay green, including thetoHaveBeenCalledWithassertions that pin the CLI→retireLocal/hatchLocalcall shapes.retire-localunit test was intentionally not added:teleport.test.tsregisters a process-globalmock.module("../lib/retire-local.js")(and one forhatch-local.js) that is never restored, so any sibling test file resolves the mocked function in the full suite. The structured returns are covered bytsc; the command-level tests exercise the call sites; reporter routing is covered by the lifecycle-reporter test.Relates to LUM-2051.
Link to Devin session: https://app.devin.ai/sessions/15bca57bd4c64a3085cfb80e1f26355a
Requested by: @ashleeradka