fix(dev-server): install dependencies in the worktree before bringing up#268
Merged
Conversation
Dogfood gap caught on PR #1571: cube prv syncs a PR-review worktree but skips the writer phase that normally runs ``verify.install``. First dev_server bring-up in a fresh PR worktree finds ``next`` / ``vite`` / etc not on PATH and exits with code 1 — judge_4 degrades to [BROWSER UNAVAILABLE]. Fix: ``_maybe_start_dev_server`` now lazily runs ``verify.install`` when (a) install is configured AND (b) ``node_modules`` is missing in the chosen worktree. ``cube auto`` already does this in Phase 1 (parallel with the prompter); this matches that behaviour for ``cube prv``. Failure to install is non-fatal — dev-server bring-up will still try, fail, and judge falls back to static review (same unavailable path as any other dev-server failure). 716/716 tests pass.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis pull request adds three task specifications for the next steering subsystem phases (C, D, E) and implements a practical improvement to the dev-server startup path. Phases C, D, and E respectively define halt behaviour with auto-resume, spec amendment with rate limiting, and telemetry aggregation with a new reporting command. The code change modifies the judge panel to lazily install node_modules in the selected worktree before starting the dev-server, using a bounded timeout and graceful error handling that logs but does not fail the panel on installation errors. Possibly related PRs
Comment |
jacsamell
added a commit
that referenced
this pull request
Jun 3, 2026
…271) The browser-judge pipeline looked wired but never drove a browser: judge_4 captured zero screenshots and made zero browser calls, even on a visual PR with the dev server up. Root causes, all fixed: 1. MCP startup is non-blocking in the Agent SDK, so the judge's turn-1 prompt was built before `npx @playwright/mcp` finished connecting — the tools were never in the prompt and the server sat 'pending' forever. The bridge now shapes forwarded stdio servers with `type: stdio` + `alwaysLoad: true`, which forces the tools into the turn-1 prompt and blocks startup until the server connects. Verified: status flips 'pending' -> 'connected', all mcp__playwright__browser_* tools exposed. 2. @playwright/mcp launches *headed* by default — nowhere to draw in a judge subprocess (or the cube-runner container), so it hung. Inject --headless, plus --isolated (ephemeral profile) and --output-dir <screenshots_dir> so browser_take_screenshot writes where the summary block looks. 3. Chromium was never installed. Added _ensure_playwright_browsers (mirrors the #268 node_modules fallback): lazy `npx playwright install chromium` when a browser-judge needs it and the cache is missing. Non-fatal. 4. The addendum named tools that don't exist (playwright_navigate vs the real mcp__playwright__browser_navigate), so even a connected judge was told the wrong API. Corrected to the real browser_* names, fixed the screenshot instructions to use the server's filename+output-dir, and made the browser pass mandatory for frontend PRs (skip only with explicit justification). Tests: _augment_playwright_args (inject/idempotent/operator-override/ignore non-playwright/empty). Full automation+cli suites green (409 passed). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Dogfood gap: cube prv worktrees lack node_modules → dev_server immediately fails with 'next: command not found'. Now runs verify.install lazily when node_modules is missing. 716/716 tests pass.
Summary
Fixes a dogfood gap where PR-review worktrees lack
node_modules, preventing the dev-server from starting. The dev-server now lazily installs dependencies before bringing up ifverify.installis configured andnode_modulesis missing in the selected worktree. Installation failures are treated as non-fatal—dev-server bring-up is still attempted, and if it fails, the judge falls back to static review.Changes
Code changes:
_maybe_start_dev_serverinpython/cube/automation/judge_panel.pyto check for missingnode_modulesbefore starting the dev-server_ensure_node_modules_for_dev_server(worktree, config)that conditionally runs the install command, with proper error handling and non-fatal fallback behaviour (+52 lines)Documentation additions:
Testing
All 716 tests pass.