vscode: gate builder-row Run/Stop Dev Server menu on worktree.devCommand (#975)#978
Merged
Conversation
…ommand Add a codev.hasDevCommand context key, refreshed from BuildersProvider's render path, and gate the builder-row Run/Stop Dev Server context-menu entries (plus the dev keybindings and workspace-dev palette entries) on it. Builder-row dev palette entries are pinned when:false (they need a row arg). Extract a shared hasRunnableDevCommand() helper used by both the context key and the Workspace view's Start-row gate, fixing a latent empty-string bug (devCommand !== null would show a Start row for "" that errors on click).
Move connectionManager ahead of the transient workspaceState param so all parameter-properties (cache, diffCache, connectionManager) are grouped. workspaceState is consumed only in the constructor body, so it stays a plain param rather than becoming a dead readonly field.
The PR-stage consult (2 REQUEST_CHANGES, HIGH) correctly found that refreshing the context key from the Builders-tree render path left the global keybindings and workspace-dev palette entries stale when the tree wasn't rendered — cmd+alt+r could silently no-op with a devCommand configured. Move the refresh back to global signals (onStateChange + worktree-config-updated SSE) in extension.ts, mirroring WorkspaceProvider, so all three gated surfaces stay live and consistent. Reverts the render-path wiring in BuildersProvider.
…tation Recheck consult (Codex/Claude) flagged the review Summary still described the reverted render-path mechanism and Files Changed had duplicate entries. Update the Summary to the global onStateChange + SSE listener, dedupe Files Changed, and mark the superseded render-path narrative in the thread.
amrmelsayed
added a commit
that referenced
this pull request
Jun 3, 2026
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.
PIR Review: Gate builder-row Run/Stop Dev Server menu on
worktree.devCommandFixes #975
Summary
The builder-row Run/Stop Dev Server context-menu entries used to show on every builder row regardless of whether
worktree.devCommandwas configured, so picking one ran against a missing command (error toast or no-op). This PR gates those entries — plus the dev keybindings and the workspace-dev command-palette entries — on a newcodev.hasDevCommandcontext key. The key is refreshed by global signals inextension.ts(onStateChange+ theworktree-config-updatedSSE), mirroring the Workspace view's own dev-row gate, so all three gated surfaces stay live — including the keybindings/palette, which are invokable independent of the Builders tree's visibility. A sharedhasRunnableDevCommand()helper backs both the key and the Workspace view's Start-row gate, also fixing a latent empty-string bug.Files Changed
packages/vscode/src/extension.ts(+32 / -0) —syncHasDevCommandContext()refreshing the context key ononStateChange+ theworktree-config-updatedSSE envelope, plus an initial seedpackages/vscode/src/load-worktree-config.ts(+18 / -0) —hasRunnableDevCommand()helper (single source of truth)packages/vscode/src/views/workspace.ts(+~12 / -~11) — Start-row gate switched fromdevCommand !== nulltohasRunnableDevCommand(worktreeConfig)packages/vscode/package.json(+22 / -4) —&& codev.hasDevCommandon the two builder-row devwhenclauses;whengating on the two dev keybindings; commandPalette entries (workspace-dev →codev.hasDevCommand, builder-row dev →false)packages/vscode/src/__tests__/has-runnable-dev-command.test.ts(+47 / -0, new) — helper truth tablepackages/vscode/src/__tests__/menu-when-clauses.test.ts(+67 / -0) —when-shape assertions for the gating across menu / palette / keybinding surfacesCommits
4c856a92[PIR vscode: Run/Stop Dev Server context-menu entries on builder rows ignoreworktree.devCommandpresence #975] Gate builder-row Run/Stop Dev Server menu on worktree.devCommand50a66686[PIR vscode: Run/Stop Dev Server context-menu entries on builder rows ignoreworktree.devCommandpresence #975] Group BuildersProvider parameter-properties togetherworktree.devCommandpresence #975] Use global onStateChange + SSE refresh for hasDevCommand (consult REQUEST_CHANGES)Test Results
pnpm build: ✓ pass (porch check, 6.3s)pnpm test: ✓ pass (porch check, 20.8s)pnpm check-types: ✓ cleanpnpm test:unit: ✓ 276 tests pass (21 files), incl. 2 new test fileseslint(changed files): ✓ cleandev-approvalgate against the running worktree — builder-row entries hidden with nodevCommand, present with one, live after a config edit; Workspace view unaffected. Note: dev-approval was on an earlier render-path implementation; the refresh mechanism was changed post-gate (see "Things to Look At") — the user-facing menu behavior is unchanged and now extends correctly to the keybindings/palette.Architecture Updates
No
arch.mdchanges needed. This PR fixes a UI-gating bug within the existing VSCode TreeView +when-clause / setContext pattern; it introduces no new module boundary or architectural pattern. Thecodev.hasDevCommandrefresh mirrors the established global context-key pattern in this extension —WorkspaceProvider's own dev-row gate (onStateChange+worktree-config-updatedSSE) and the builders-tree setting keys (codev.buildersAutoCollapse,codev.buildersGroupBy), which are driven by globalvscode.workspace.onDidChangeConfigurationlisteners. The key is refreshed by global signals — not the tree render path — because the keybindings and palette entries it also gates are invokable independent of the Builders tree's visibility.Lessons Learned Updates
No addition to
codev/resources/lessons-learned.md. The transferable observation — a context key shared by surfaces with different lifecycles (an ephemeral tree context menu vs. global keybindings/palette) must be refreshed on the cadence of the most demanding surface; a render-path refresh that fits the menu silently breaks the global surfaces — is the heart of this PR's review back-and-forth and is captured in the code comments + the "Things to Look At" note below, not broad enough to warrant a standing lessons entry.Things to Look At During PR Review
codev.hasDevCommandfrom the Builders-tree render path, which left the global keybindings (cmd+alt+r/s) and workspace-dev palette entries stale when the Builders tree wasn't rendered (e.g. collapsed) —cmd+alt+rcould silently no-op with adevCommandconfigured. Disposition: fixed, not rebutted — the refresh was moved to global signals (onStateChange+worktree-config-updatedSSE) inextension.ts, mirroringWorkspaceProvider. All three surfaces are now live and consistent. Gemini also correctly flagged a factual error in an earlier draft of this review (it claimed the render-path was "consistent with"buildersAutoCollapse/buildersGroupBy, which are actuallyonDidChangeConfiguration-driven) — corrected above. PIR is single-pass, so this fix was not independently re-reviewed by the models; please verify it at theprgate.hasRunnableDevCommand— is fully covered (truth table incl. empty/whitespace). The menu/keybinding/palettewhen-clause contract is pinned inmenu-when-clauses.test.ts. The event-wiring itself (which signals trigger the refresh) is activation-time VSCode glue: it mirrorsWorkspaceProvider's already-trusted, identicalonStateChange+worktree-config-updatedfilter, and like all such glue in this extension is exercised by the vscode-test integration harness / manual run rather than vitest. No brittle vscode-mock unit test was added for it.hasRunnableDevCommandtreats"devCommand": ""/ whitespace as absent, matchingdev-shared.ts'sif (!devCommand)runnability gate. This also changes the Workspace view's Start-row gate (previouslydevCommand !== null, which would have shown a Start row for""that errors on click) — a latent bug fix. Confirm no Workspace-view regression.when: false. They need a tree-row argument; argless palette invocation falls through (same rationale asviewSpecFile/viewPlanFile/viewReviewFile). This is a behavior change — before this PR they had no palette entry and so defaulted to visible.How to Test Locally
pir-975→ View Diffafx dev pir-975worktree.devCommandin.codev/config.json: right-click a builder row → no Run/Stop Dev Server entries."devCommand": "pnpm dev"and save: entries appear live (no window reload), via theworktree-config-updatedSSE."": entries hidden live.cmd+alt+ris silent with no dev command (even with the Builders tree collapsed); starts dev with one — verify both tree-collapsed and tree-expanded states.