vscode: show issue # + title in Open Builder Terminal & Send Message pickers

codev/plans/925-vscode-open-builder-terminal-a.md

@@ -0,0 +1,85 @@
+---
+approved: 2026-05-31
+validated: [gemini, codex, claude]
+---
+
+# PIR Plan: Show issue # + title in the two outlier builder Quick Picks
+
+## Understanding
+
+Issue #925 asks to bring two command-palette Quick Pick pickers — **Codev: Open Builder Terminal** (`packages/vscode/src/extension.ts:539-542`) and **Codev: Send Message to Builder** (`packages/vscode/src/commands/send.ts:22-25`) — in line with the seven other builder pickers, which render each row as `#<issueId> <issueTitle>` instead of the internal builder name (`pir-1333`).
+
+**The issue's literal fix does not compile, because its central premise is factually wrong.** The issue states the two outliers source their builders "from the same `client.getWorkspaceState(workspacePath)` endpoint the seven correct pickers use, so the fields should already be populated." That is not the case:
+
+- The **seven correct pickers all call `client.getOverview(workspacePath)`**, which returns `OverviewData.builders: OverviewBuilder[]`. `OverviewBuilder` *has* `issueId`, `issueTitle`, and `phase` (`packages/types/src/api.ts:129-170`). Verified each: `approve.ts:69`, `cleanup.ts:32`, `view-diff.ts:312`, `run-worktree-setup.ts:34`, `run-worktree-dev.ts:29`, `open-worktree-window.ts:32`, `open-worktree-folder.ts:25` — all `getOverview`.
+- The **two outliers call `client.getWorkspaceState(workspacePath)`**, which returns `DashboardState.builders: Builder[]` (`packages/types/src/api.ts:71-94`). The `Builder` interface (`api.ts:25-46`) has `phase` but **no `issueId` and no `issueTitle`**.
+
+So `b.issueId` / `b.issueTitle` on a `getWorkspaceState` builder is a TypeScript error — the issue's drop-in snippet would fail `tsc`. This is exactly the design ambiguity that makes #925 a PIR rather than a BUGFIX: the fix is not a mechanical field swap; the two outliers read the *wrong endpoint* for the data they now need.
+
+Why the outliers can't simply switch wholesale to `getOverview`: both depend on fields that live **only on the `getWorkspaceState` `Builder`**, not on `OverviewBuilder`:
+- `extension.ts` filters on `b.terminalId` and passes `picked.terminalId` + `picked.id` into `terminalManager.openBuilder(terminalId, builderId, label)` (`terminal-manager.ts:160`). `OverviewBuilder` has no `terminalId`.
+- `send.ts` filters on `b.terminalId` and passes `picked.id` into `client.sendMessage(id, ...)`.
+
+`OverviewBuilder.id` and `Builder.id` are also **different in shape** and can't be compared with strict `===` — this is documented at `run-worktree-dev.ts:51-54`, which is the exact precedent: a picker that needs both the overview display fields *and* a `getWorkspaceState`-only runtime field. It fetches `getOverview` (primary, for the label) **and** `getWorkspaceState`, joining the two via `resolveAgentName(overviewBuilder.id, workspaceState.builders)` (`packages/core/src/agent-names.ts:40`), which does case-insensitive exact + tail-match (`'109'` matches `'builder-spir-109'`).
+
+## Proposed Change
+
+Mirror the `run-worktree-dev.ts` precedent exactly: **`getOverview` as the display source, joined to `getWorkspaceState` via `resolveAgentName` for the `Builder`-only fields** (`terminalId`, canonical `Builder.id`, `Builder.name`).
+
+To avoid duplicating the join in two call sites — and following the existing `prune-builder-terminals.ts` precedent of extracting a pure, unit-tested helper used by command code — I'll add one small pure function:
+
+```ts
+// packages/vscode/src/builder-pick-rows.ts (new)
+export interface BuilderPickRow {
+  label: string;          // `#<issueId|id> <issueTitle>`
+  description: string;    // phase
+  id: string;             // workspace Builder.id (load-bearing for the action)
+  name: string;           // workspace Builder.name (for the terminal tab title)
+  terminalId?: string;    // workspace Builder.terminalId (Open Terminal only)
+}
+
+// Pairs each overview builder with its workspace Builder via resolveAgentName,
+// keeps only those with a live terminal, and formats the row.
+export function buildBuilderPickRows(
+  overviewBuilders: Array<{ id: string; issueId: string | null; issueTitle: string | null; phase: string }>,
+  workspaceBuilders: Array<{ id: string; name: string; terminalId?: string }>,
+): BuilderPickRow[]
+```
+
+Both call sites become: fetch `getOverview` + `getWorkspaceState`, call `buildBuilderPickRows`, show the rows. The label is `#${issueId ?? id} ${issueTitle ?? ''}` — byte-identical to the seven correct pickers, with the same `issueId ?? id` degenerate-state fallback. `description` is the phase, matching Cleanup Builder.
+
+This is in scope: the issue's "out of scope" excludes a *full* shared `pickBuilder` helper across **all nine** call sites (which would restructure the seven good ones). A tiny pure helper shared between **only the two being fixed** restructures nothing in the seven and is the same shape as the existing `prune-builder-terminals.ts` helper — it is the cleanest way to not write the `resolveAgentName` join twice.
+
+**Behavior preserved deliberately:**
+- The "only builders with a live terminal" filter is kept (now expressed as "overview builder whose joined workspace `Builder` has a `terminalId`"), preserving the current "no builder terminals / no active builders" empty-state messages.
+- The downstream action is fed from the **workspace `Builder`** (`id`, `terminalId`) exactly as today — the join resolves back to the same object, so `openBuilder(...)` and `sendMessage(...)` receive identical values. Zero change to the action path.
+- The resulting **terminal tab title** stays `Codev: <builder name>` by passing `row.name` (not the new friendly label) into `openBuilder`'s `label` arg — matching `terminal-manager.ts:204`. Only the *picker row* changes; the tab name is untouched. (#925 scope is the picker row only.)
+
+## Files to Change
+
+- `packages/vscode/src/builder-pick-rows.ts` — **new**. Pure `buildBuilderPickRows()` helper + `BuilderPickRow` type. Imports `resolveAgentName` from `@cluesmith/codev-core/agent-names`.
+- `packages/vscode/src/extension.ts:532-545` — in `codev.openBuilderTerminal`: also fetch `getOverview`, build rows via the helper, keep the empty-state guard, and call `openBuilder(picked.terminalId, picked.id, \`Codev: ${picked.name}\`, true)`.
+- `packages/vscode/src/commands/send.ts:15-34` — also fetch `getOverview`, build rows via the helper, keep the empty-state guard, send via `picked.id`; input-box / success messages reference `picked.label`.
+- `packages/vscode/src/__tests__/builder-pick-rows.test.ts` — **new**. Unit tests for the helper.
+
+## Risks & Alternatives Considered
+
+- **Risk: `resolveAgentName` returns `null` or ambiguous for a builder** (id shapes don't match, or multiple tail matches). Mitigation: such builders are dropped from the picker (no terminal join → not actionable anyway), mirroring how `run-worktree-dev.ts` already tolerates the join. Unit-tested explicitly.
+- **Risk: `issueId` genuinely missing** (degenerate state). Mitigation: `#${issueId ?? id}` fallback — identical to the seven correct pickers and called out in the acceptance criteria. Unit-tested.
+- **Alternative A — keep `getWorkspaceState` as primary, reverse-join to `getOverview` for `issueId`/`issueTitle`.** Smaller diff, but it enriches the "wrong" source and inverts the established `getOverview`-primary pattern the seven pickers and `run-worktree-dev` all follow — worse for consistency. Rejected.
+- **Alternative B — add `issueId`/`issueTitle` to the `Builder` wire type and populate them in Tower's `/api/state` handler.** Changes a wire contract + server for a VSCode-only display concern, and `getOverview` already carries these fields. Out of proportion. Rejected.
+- **Alternative C — literal issue snippet (`b.issueId` on `getWorkspaceState` builders).** Does not compile. Rejected.
+
+## Test Plan
+
+- **Build / typecheck**: `pnpm --filter @cluesmith/codev-vscode check-types` (or the package's `tsc --noEmit`) — must pass. This is the gate that proves the issue's literal snippet was wrong and the join approach is sound.
+- **Unit** (`pnpm --filter ... test:unit`, vitest), in `builder-pick-rows.test.ts`:
+  - Happy path: overview builder with `issueId`/`issueTitle`/`phase` joined to a workspace builder with `terminalId` → row label `#<id> <title>`, description = phase, correct `id`/`name`/`terminalId`.
+  - Fallback: `issueId: null` → label `#<id> ...`.
+  - Filter: overview builder whose join resolves to a workspace builder with **no** `terminalId` → excluded.
+  - Filter: overview builder with **no** matching workspace builder (resolveAgentName null) → excluded.
+  - Id-shape mismatch: overview id `pir-925` vs workspace id `builder-pir-925` → still joins via tail-match.
+- **Manual** (at the `dev-approval` gate, via `afx dev pir-925` and the VSCode extension built from this branch):
+  - Run **Codev: Open Builder Terminal** → rows show `#<id> <title>` with `<phase>` in the description; picking one opens the correct terminal (tab title unchanged: `Codev: <builder name>`).
+  - Run **Codev: Send Message to Builder** → rows show the same format; picking + sending delivers to the correct builder.
+  - Sanity-check one of the seven (e.g. **Cleanup Builder**) still renders correctly — no regression.

codev/projects/925-vscode-open-builder-terminal-a/status.yaml

@@ -0,0 +1,30 @@
+id: '925'
+title: vscode-open-builder-terminal-a
+protocol: pir
+phase: verified
+plan_phases: []
+current_plan_phase: null
+gates:
+  plan-approval:
+    status: approved
+    requested_at: '2026-05-29T23:12:24.174Z'
+    approved_at: '2026-05-30T00:06:13.895Z'
+  dev-approval:
+    status: approved
+    requested_at: '2026-05-30T00:11:54.663Z'
+    approved_at: '2026-05-31T11:38:16.821Z'
+  pr:
+    status: approved
+    requested_at: '2026-05-31T11:45:30.417Z'
+    approved_at: '2026-05-31T11:45:41.710Z'
+iteration: 1
+build_complete: true
+history: []
+started_at: '2026-05-29T23:08:23.164Z'
+updated_at: '2026-05-31T11:45:51.988Z'
+pr_history:
+  - phase: review
+    pr_number: 951
+    branch: builder/pir-925
+    created_at: '2026-05-31T11:41:36.902Z'
+pr_ready_for_human: false

codev/reviews/925-vscode-open-builder-terminal-a.md

@@ -0,0 +1,65 @@
+# PIR Review: Show issue # + title in the two outlier builder Quick Picks
+
+Fixes #925
+
+## Summary
+
+Two VSCode command-palette pickers — **Codev: Open Builder Terminal** and **Codev: Send Message to Builder** — rendered each builder as its bare internal id (`pir-1333`), while the other seven builder pickers show `#<issueId> <issueTitle>`. The fix brings the two outliers in line. The non-obvious part: the two outliers read `client.getWorkspaceState` (→ `Builder`, which has **no** `issueId`/`issueTitle`), whereas the seven correct pickers read `client.getOverview` (→ `OverviewBuilder`, which has them) — so the issue's proposed one-line field swap would not have compiled. The implementation mirrors the existing `run-worktree-dev.ts` pattern: source display fields from `getOverview`, join back to the workspace `Builder` (for `terminalId` and the canonical id the action needs) via `resolveAgentName`, behind one shared pure helper.
+
+## Files Changed
+
+`git diff --stat` vs merge-base (code only):
+
+- `packages/vscode/src/builder-pick-rows.ts` (+75 / -0) — new pure helper `buildBuilderPickRows` + types
+- `packages/vscode/src/__tests__/builder-pick-rows.test.ts` (+95 / -0) — new, 6 unit tests
+- `packages/vscode/src/extension.ts` (+10 / -7) — `openBuilderTerminal` picker rewired
+- `packages/vscode/src/commands/send.ts` (+11 / -6) — `sendMessage` picker rewired
+
+Plus protocol artifacts: `codev/plans/925-…md`, `codev/state/pir-925_thread.md`, `codev/projects/925-…/status.yaml`.
+
+## Commits
+
+`git log main..HEAD --oneline` (excluding porch chore commits):
+
+- `e521c3cd` [PIR #925] Plan draft
+- `991d311e` [PIR #925] Render issue # + title in Open Terminal & Send Message pickers
+
+## Test Results
+
+- Build (`tsc --noEmit` + `eslint` + `esbuild --production`): ✓ pass
+- Unit (`pnpm test:unit`): ✓ 119 passed (6 new in `builder-pick-rows.test.ts`)
+- Porch checks (`porch done`, implement phase): build ✓, tests ✓
+- Manual verification: human approved at the `dev-approval` gate.
+
+## Architecture Updates
+
+No `arch.md` changes needed. This reuses an existing pattern rather than introducing one: a picker that needs both `getOverview` display fields and a `getWorkspaceState`-only runtime field already exists (`run-worktree-dev.ts`), using `getOverview` as the display source joined to the workspace `Builder` via `resolveAgentName` (tail-match, because `OverviewBuilder.id` and `Builder.id` differ in shape). The only new surface is a small pure helper that factors that join out of the two call sites — no module boundary or data-flow change.
+
+## Lessons Learned Updates
+
+No new entry added to `codev/resources/lessons-learned.md` — the lesson here is a concrete instance of an existing one rather than a new principle:
+
+> [From 0059] Verify what data is actually available in state before designing features that depend on it.
+
+This PR is a textbook case: the issue body asserted both outliers sourced builders "from the same `getWorkspaceState` endpoint the seven correct pickers use, so the fields should already be populated." In fact the seven use `getOverview`, and `getWorkspaceState`'s `Builder` type carries no `issueId`/`issueTitle` — so the proposed drop-in `b.issueId` was a compile error, not a mechanical swap. The audit table in a well-researched issue can still mis-state the data source; confirm the actual endpoint/type before trusting a "~10 LOC mechanical" framing. Recording the specifics here keeps the curated lessons file from accumulating a near-duplicate of 0059.
+
+(Also relevant: memory `reference_overview_builder_dual_type` — the overview-builder shape is defined twice. The helper sidesteps this by accepting structural minimal interfaces rather than importing a concrete type, so it couples to neither definition.)
+
+## Things to Look At During PR Review
+
+- **Consultation REQUEST_CHANGES (codex) — addressed.** The single-pass 3-way consult returned gemini=APPROVE, claude=APPROVE, codex=REQUEST_CHANGES. Codex's only finding was that `codev/plans/925-…md` lacked the repo's approved-plan YAML frontmatter (`approved:` / `validated:`); it explicitly confirmed the implementation itself is sound (passed its local `pnpm check-types` + `pnpm lint`). Fixed by adding the frontmatter to the plan, matching the #927 precedent on main. Not a code defect, so no regression test applies. PIR is single-pass — this fix was **not** independently re-reviewed by the models; flagged to the architect for verification at the `pr` gate.
+- **The `resolveAgentName` join** (`builder-pick-rows.ts`) is the crux. It matches an overview id (`pir-925`) to a workspace id (`builder-pir-925`) by tail-match, not `===`. A builder that fails to join, or whose joined `Builder` has no `terminalId`, is dropped from the picker — this intentionally preserves the outliers' original `filter(b => b.terminalId)` "only builders with a live terminal" semantics. Verified by the "excludes no-terminal" and "excludes no-match" unit tests.
+- **Terminal tab title preserved**: `openBuilder` is still passed `Codev: ${name}` (the builder name), not the new friendly label — so only the *picker row* changes, the resulting tab name is unchanged. #925 scopes the picker row only.
+- **Two fetches per invocation**: both call sites now issue `getOverview` + `getWorkspaceState` concurrently via `Promise.all`. `getOverview` is served from a 30s-TTL cache (`overview.ts`), so the common case is a cache hit; the picker is a cold, user-initiated path regardless. This matches what `run-worktree-dev.ts` already does (it issues both, sequentially).
+
+## How to Test Locally
+
+For reviewers pulling the branch:
+
+- **View diff**: VSCode sidebar → right-click builder `pir-925` → **View Diff**
+- **Run the extension** (this is a VSCode-extension change, so `afx dev` does not apply — use the Extension Development Host): open the worktree in VSCode, press **F5** ("Run Codev Extension"), and in the dev-host window open your **main** Codev checkout (the workspace Tower tracks, so the picker is populated).
+- **What to verify** (mapped to the plan's Test Plan):
+  - `Codev: Open Builder Terminal…` → rows read `#<id> <title>` with `<phase>` in the description; picking opens the right terminal (tab title `Codev: <name>`).
+  - `Codev: Send Message` (or Cmd+K D) → same row format; sending reaches the right builder.
+  - Sanity: `Codev: Cleanup Builder` still renders the same format (no regression to the seven).
+  - Degenerate: a builder with a null `issueId` falls back to `#<id>`.

codev/state/pir-925_thread.md

@@ -0,0 +1,26 @@
+# pir-925 thread — vscode builder Quick Pick: show issue # + title
+
+## Plan phase
+
+Investigated the two outlier pickers. **Key finding: the issue's literal fix does not compile.**
+
+- The issue claims both outliers read `getWorkspaceState`, "the same endpoint the seven correct pickers use." Wrong on two counts:
+  - The **seven correct pickers all use `client.getOverview`** → `OverviewBuilder` (has `issueId`/`issueTitle`/`phase`).
+  - The **two outliers use `client.getWorkspaceState`** → `Builder` (`api.ts:25-46`), which has **no `issueId`/`issueTitle`**. `b.issueId` there is a TS error.
+- The outliers also depend on `Builder.terminalId` + `Builder.id` (not on `OverviewBuilder`), so they can't just swap wholesale to `getOverview`.
+- Precedent for "picker needs both": `run-worktree-dev.ts:29,53` — `getOverview` primary + `resolveAgentName(overviewId, workspaceBuilders)` join (ids differ in shape; tail-match, not `===`).
+
+**Approach**: extract a pure `buildBuilderPickRows(overviewBuilders, workspaceBuilders)` helper (mirrors the `prune-builder-terminals.ts` pure-helper precedent), used by both call sites. `getOverview` for the label, joined to `getWorkspaceState` for `terminalId`/canonical `id`/`name`. Action path fed from the workspace `Builder` exactly as today (zero downstream change). Terminal tab title preserved as `Codev: <name>`.
+
+Plan written to `codev/plans/925-vscode-open-builder-terminal-a.md`. Awaiting `plan-approval`.
+
+## Implement phase
+
+plan-approval approved. Implemented:
+- `builder-pick-rows.ts` (new) — pure `buildBuilderPickRows(overviewBuilders, workspaceBuilders)` joining overview→workspace via `resolveAgentName`, filtering to live-terminal builders, formatting `#<id> <title>` + phase.
+- `extension.ts` openBuilderTerminal + `send.ts` — both now `Promise.all([getOverview, getWorkspaceState])`, build rows via the helper. Action path unchanged (workspace `Builder.id`/`terminalId`); terminal tab title preserved as `Codev: <name>`.
+- `builder-pick-rows.test.ts` (new) — 6 unit tests (happy path, tail-match join, issueId fallback, no-terminal exclude, no-match exclude, mixed list).
+
+Env note: worktree shipped without `node_modules` and without `codev-core`/`codev-types` `dist/`. Ran `pnpm install`, built core + types — needed for vitest subpath resolution and esbuild. (Not a code change; flag for Lessons.)
+
+Checks: tsc ✓, eslint ✓, esbuild ✓, vitest 119/119 ✓. Awaiting `dev-approval`.