fix(preview): wait for a ready reactor + fix workdir path matching#60
Open
acaldas wants to merge 2 commits into
Open
fix(preview): wait for a ready reactor + fix workdir path matching#60acaldas wants to merge 2 commits into
acaldas wants to merge 2 commits into
Conversation
Two related, architecture-agnostic preview fixes: - sameProjectPath: match a running reactor-project by realpath, not raw string. On macOS the /var -> /private/var canonicalization made resolvePreviewEndpoint report "not running" while the reactor WAS running, breaking spec-preview. - ensureReactorProjectReady: spec-preview-create/update/show/create-drive now ensure the reactor is up and block until ready (start if absent, poll a starting instance) instead of erroring on a still-starting one. Claude-Session: https://claude.ai/code/session_01A3bn1tgAytBtHoX7fxRGS6
There was a problem hiding this comment.
Pull request overview
This PR improves robustness of the spec-preview-* commands by (1) matching running reactor-project instances using canonicalized filesystem paths and (2) ensuring the reactor reaches a “ready / Switchboard captured” state before preview commands attempt to call it.
Changes:
- Add canonical-path comparison (
sameProjectPath) to avoid/varvs/private/varmismatches when locating a running reactor instance. - Introduce
ensureReactorProjectReadyand call it fromspec-preview-create,-update,-show, and-create-driveto reduce races against a still-starting reactor. - Thread proxy public URL through start params when the proxy is available.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vetra-cli/src/helpers/reactor-project-preview.ts | Adds canonical path matching and a readiness-wait helper for reactor-project preview operations. |
| vetra-cli/src/commands/spec-preview/update.ts | Ensures the reactor-project is ready before resolving endpoints and mutating preview docs. |
| vetra-cli/src/commands/spec-preview/show.ts | Ensures the reactor-project is ready before resolving endpoints and building preview URLs. |
| vetra-cli/src/commands/spec-preview/create.ts | Ensures the reactor-project is ready before creating a document in the preview drive. |
| vetra-cli/src/commands/spec-preview/create-drive.ts | Ensures the reactor-project is ready before creating/reusing an app-bound preview drive. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+49
to
+53
| // Compare project paths by canonical (realpath) form — a running instance | ||
| // registers a realpath'd workdir (macOS /var → /private/var) a raw compare misses. | ||
| function sameProjectPath(a: string | undefined, b: string): boolean { | ||
| return a !== undefined && canonicalProjectPath(a) === canonicalProjectPath(b); | ||
| } |
Comment on lines
+133
to
+152
| if (!starting) { | ||
| try { | ||
| await services.start("reactor-project", { | ||
| workdir: projectPath, | ||
| cwd: projectPath, | ||
| params: opts?.startParams, | ||
| }); | ||
| } catch (err) { | ||
| // Surface, don't swallow — a start failure here (e.g. "max instances") | ||
| // otherwise looks like a mysterious "not running" downstream. | ||
| console.error(`[ensure-ready] services.start failed: ${err instanceof Error ? err.message : String(err)}`); | ||
| } | ||
| } | ||
|
|
||
| const deadline = Date.now() + (opts?.timeoutMs ?? 45_000); | ||
| while (Date.now() < deadline) { | ||
| if (ready()) return; | ||
| await new Promise((r) => setTimeout(r, 500)); | ||
| } | ||
| } |
- status-filter the ready-instance find so a stale stopped/failed entry for the same path can't shadow the live ready instance (was a spurious full-timeout hang) - fail fast with an actionable error when a *different* project holds the single reactor slot, instead of polling to timeout then reporting a misleading "not running" - align the poll cap to the service's own cold-start readiness budget (VETRA_REACTOR_READINESS_TIMEOUT_MS default 90s + margin) so a slow but legitimate cold start isn't abandoned early - throw a clear "timed out"/start error instead of console.error in a shared helper Validated via no-LLM replay: preview succeeds, 0 "not running". Claude-Session: https://claude.ai/code/session_01A3bn1tgAytBtHoX7fxRGS6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two small, architecture-agnostic fixes to
spec-preview-*, extracted from the multi-agent exploration:1.
sameProjectPath— match the running reactor by realpathresolvePreviewEndpointmatched a runningreactor-projectinstance with a rawi.workdir === projectPathstring compare. On macOS the temp-dir/var→/private/varcanonicalization means the instance registers a realpath'd workdir that never===the passed path, so preview reported "is not running" while the reactor was running (the running-projects hint even listed it). Now both sides are compared viarealpath.2.
ensureReactorProjectReady— block until the reactor is readyspec-preview-create/-update/-show/-create-drivenow ensure the reactor is up before talking to it: return if already ready, poll astartinginstance until ready, or start one if absent (with a timeout). Previously these errored on a still-starting reactor, forcing the caller to notice, callreactor-project-start, and retry.Both are correctness/robustness fixes independent of agent architecture. Typecheck clean; validated end-to-end via a no-LLM replay-fixture run (preview steps succeed, zero "not running").
https://claude.ai/code/session_01A3bn1tgAytBtHoX7fxRGS6