From 71b4bad95a7796a85a75384c9b93e152979e8c84 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 09:35:58 +1000 Subject: [PATCH 01/21] chore(porch): 1412 init pir --- .../status.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml new file mode 100644 index 000000000..375743d01 --- /dev/null +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -0,0 +1,18 @@ +id: '1412' +title: vscode-terminallinkprovider-ma +protocol: pir +phase: plan +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: pending + dev-approval: + status: pending + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-08-11T23:35:58.126Z' +updated_at: '2026-08-11T23:35:58.128Z' From f93d351e884663cd5544271d4f6e9d06021aea2a Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 09:42:07 +1000 Subject: [PATCH 02/21] [PIR #1412] Plan draft Plan for clickable #N / PR #N terminal references in the VS Code extension. --- .../1412-vscode-terminallinkprovider-ma.md | 220 ++++++++++++++++++ codev/state/pir-1412_thread.md | 24 ++ 2 files changed, 244 insertions(+) create mode 100644 codev/plans/1412-vscode-terminallinkprovider-ma.md create mode 100644 codev/state/pir-1412_thread.md diff --git a/codev/plans/1412-vscode-terminallinkprovider-ma.md b/codev/plans/1412-vscode-terminallinkprovider-ma.md new file mode 100644 index 000000000..30381034a --- /dev/null +++ b/codev/plans/1412-vscode-terminallinkprovider-ma.md @@ -0,0 +1,220 @@ +# PIR Plan: Clickable `#N` / `PR #N` terminal references (VS Code) + +## Understanding + +Terminal output in the VS Code extension constantly cites issues and PRs by number +(`#915`, `PR #1402`). Cmd+click on those spans falls through to VS Code's workspace word +search, which matches nothing. The issue (decided design, 2026-08-12) asks for a +`registerTerminalLinkProvider` that claims `#\d+` and `PR #\d+` spans and, on click: + +- **`PR #N`** → open the PR's forge page in the browser (there is no in-editor PR preview — #1179). +- **bare `#N`** → issue-first: open the in-editor issue viewer (`viewBacklogIssue`, the #1096 + preview) unless the number is actually a PR, in which case fall through to the PR browser-open. + One click, no disambiguation prompt. +- **Setting** `codev.terminalLinks.issueTarget: editor | browser` (default `editor`) lets users + send genuine issues to the browser too. (PRs ignore it — browser is their only destination.) +- Unresolvable number → warning toast matching `openIssueById`'s failure grammar. + +The design is settled; this plan is the **how**. Two empirical findings drove the design below. + +### Finding 1 — the issue-vs-PR discriminator (architect concern #3, verified) + +The issue text assumed the issue fetch would *fail* on a PR number. **It does not.** GitHub's +`gh issue view` (what the `issue-view` forge concept runs — +`packages/codev/scripts/forge/github/issue-view.sh:5`) resolves a **PR** number successfully: + +``` +$ gh issue view 1405 --json state,url # 1405 is a merged PR +{"state":"MERGED","url":"https://github.com/cluesmith/codev/pull/1405"} # exit 0 +$ gh issue view 1412 --json state,url # 1412 is a genuine issue +{"state":"OPEN","url":"https://github.com/cluesmith/codev/issues/1412"} # exit 0 +``` + +So `getIssue(N)` returns a populated object for **both** issues and PRs. The clean, deterministic +discriminator is the **`url` path segment**: a PR's url is `.../pull/N`, an issue's is +`.../issues/N`. `IssueView.url` exists (optional string — `packages/types/src/api.ts:403`) and is +populated for GitHub. This *is* the faithful reading of the decided design's "if the number +resolves as a PR instead" — "resolves as a PR" == the resolved url is a `/pull/` url. + +### Finding 2 — the VS Code API shape (architect concern #1, verified against the pinned engine) + +Against the bundled `@types/vscode` (`~1.105.0`, matching `engines.vscode ^1.105.0`): + +- `TerminalLinkProvider` has `provideTerminalLinks(context, token)` and `handleTerminalLink(link)`. +- `TerminalLinkContext.line` is documented as **"the text from the unwrapped line"** + (`index.d.ts:8099`). So VS Code hands the provider one **logical, unwrapped** line — a reference + split across a visual wrap is *not* a problem (architect concern #4: no real wrap limitation to + document; refs are only ever missed if a *logical* line break splits them, which terminal output + does not do to a `#1234` token). Multiple refs per line are handled by scanning the line. +- The d.ts warns: *"do not share global objects (eg. `RegExp`) that could have problems when + asynchronous usage may overlap."* We construct the regex **inside** `provideTerminalLinks` (or use + `String.prototype.matchAll`) rather than the module-level shared-`lastIndex` pattern the sibling + `BuilderTerminalLinkProvider` uses — safer and idiomatic for the documented reentrancy. + +This is an established local pattern: `terminal-link-provider.ts` already hosts two providers +(`BuilderTerminalLinkProvider`, `ReconnectTerminalLinkProvider`) registered in `extension.ts`. + +## Proposed Change + +### Reuse discipline (the core review axis) + +No new fetch code. The only forge paths touched are the three sanctioned reuse targets: + +- `openPRInBrowser(cm, N)` — `commands/open-pr-by-id.ts` (getPR → openExternal) +- `openIssueInBrowser(cm, N)` — `commands/open-issue-by-id.ts` (getIssue → openExternal, preview-fallback) +- `viewBacklogIssue(cm, N)` — `commands/view-issue.ts` (getIssue → in-editor preview) + +The discriminator itself calls `client.getIssue(N)` — the **same SDK method** those helpers use, +not new fetch code — purely to read the resolved `url` and branch. Every actual *open* funnels +through one of the three helpers, so there is exactly one code path per destination. + +### Resolution logic — new file `apps/vscode/src/commands/open-terminal-ref.ts` + +Mirrors `open-issue-by-id.ts` / `open-pr-by-id.ts` (the named template). Exports one function: + +```ts +export async function openTerminalRef( + cm: ConnectionManager, + ref: { number: string; isPR: boolean }, +): Promise +``` + +- **`ref.isPR`** (explicit `PR #N`): `await openPRInBrowser(cm, ref.number)`. No discriminator + fetch needed — the prefix already told us it's a PR. +- **bare `#N`**: connection guard (client + workspacePath + `getState() === 'connected'`, same guard + as the helpers; error toast if not connected), then `const issue = await client.getIssue(number)`: + - `!issue` → `showWarningMessage("Codev: Could not open #N (not found, or forge unavailable).")` + (matches `openIssueInBrowser`'s grammar). + - `issue.url` matches `/\/pull\/\d/` → it is actually a PR → `await openPRInBrowser(cm, number)` + (funnel through the single PR-open path — consistent with the explicit `PR #N` branch; the extra + `getPR` round-trip on a single deliberate click is negligible and keeps one owner of PR-opening). + - otherwise (genuine issue, incl. url absent on a non-GitHub forge → treated as issue) → read the + setting `getConfiguration('codev').get('terminalLinks.issueTarget', 'editor')` **at click + time** (picks up live changes): + - `editor` → `await viewBacklogIssue(cm, number)` + - `browser` → `await openIssueInBrowser(cm, number)` + +### Provider — extend existing `apps/vscode/src/terminal-link-provider.ts` + +Add a third provider co-located with the other two (stronger house convention than a near-duplicate +new file; the file is literally named for this): + +```ts +interface IssueRefLink extends vscode.TerminalLink { number: string; isPR: boolean; } + +export class IssueRefTerminalLinkProvider implements vscode.TerminalLinkProvider { + constructor(private connectionManager: ConnectionManager) {} + + provideTerminalLinks(context: vscode.TerminalLinkContext): IssueRefLink[] { + const re = /(?\bPR\s+)?#(?\d+)/gi; // fresh per call — no shared lastIndex + const links: IssueRefLink[] = []; + for (const m of context.line.matchAll(re)) { + const isPR = Boolean(m.groups?.pr); + links.push({ + startIndex: m.index, + length: m[0].length, // claims the whole "PR #N" span → kills fallback search + tooltip: isPR ? `Open PR #${m.groups!.num} in browser` : `Open issue #${m.groups!.num}`, + number: m.groups!.num, + isPR, + }); + } + return links; + } + + handleTerminalLink(link: IssueRefLink): Promise { + return Promise.resolve(openTerminalRef(this.connectionManager, { number: link.number, isPR: link.isPR })); + } +} +``` + +Regex notes: the optional greedy `(?\bPR\s+)?` prefers the `PR #N` form when present, so in +`PR #1402` the whole span is claimed once and the inner `#1402` is **not** separately matched +(matchAll advances past the consumed span). `\b` before `PR` avoids `SUPR #12`. `#\d+` never matches +`#fff` (hex color) or `# 1` (heading, space before digit). Case-insensitive so `Pr`/`pr` also match. + +### Registration — `apps/vscode/src/extension.ts` (~line 1440, beside the existing two) + +```ts +context.subscriptions.push( + vscode.window.registerTerminalLinkProvider( + new IssueRefTerminalLinkProvider(connectionManager), + ), +); +``` + +`connectionManager` is already in scope at that point (used a few lines above). + +### Setting — `apps/vscode/package.json` `contributes.configuration.properties` + +```json +"codev.terminalLinks.issueTarget": { + "type": "string", + "enum": ["editor", "browser"], + "enumDescriptions": [ + "Open bare #N issue references in the in-editor issue viewer.", + "Open bare #N issue references in your browser." + ], + "default": "editor", + "markdownDescription": "Where Cmd+clicking a bare `#N` issue reference in a terminal opens it. `PR #N` references always open in the browser (there is no in-editor PR preview)." +} +``` + +## Files to Change + +- `apps/vscode/src/commands/open-terminal-ref.ts` — **new**. `openTerminalRef(cm, ref)`: discriminator + + delegation to the three reuse helpers + setting read. +- `apps/vscode/src/terminal-link-provider.ts` — add `IssueRefTerminalLinkProvider` (+ `IssueRefLink`), + import `openTerminalRef` and `ConnectionManager` type. +- `apps/vscode/src/extension.ts:~1440` — register the provider (one `push`, beside the existing two). +- `apps/vscode/package.json` — add `codev.terminalLinks.issueTarget` under + `contributes.configuration.properties`. +- `apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts` — **new**. Detection + resolution tests. + +Out of scope (issue's follow-ups): Tower web-dashboard xterm terminals (#1217-adjacent) and an +in-editor PR preview surface. + +## Risks & Alternatives Considered + +- **Double fetch on a bare `#N` click** (discriminator `getIssue`, then the helper re-fetches): + accepted. It is one extra round-trip on a single deliberate click; funneling every open through the + one sanctioned helper (single owner per destination) is worth more than saving it. + - *Alternative (rejected):* thread the preloaded `IssueView` into `viewBacklogIssue`/`openPRInBrowser` + to skip the re-fetch — expands their signatures for a non-user-visible micro-opt; against "lean." + - *Alternative (rejected):* `openExternal(issue.url)` directly for the discriminated-PR case (saves the + `getPR`) — creates a second browser-open path; consistency with the explicit `PR #N` branch wins. +- **Over-claiming spans** (e.g. `#2` in "step #2"): accepted. Cmd+click is opt-in; a stray click yields + a warning toast at worst. Hex colors and spaced `# 1` headings already don't match. +- **Non-GitHub forge with no `url`**: discriminator can't tell issue from PR; degrades to the issue + path (`viewBacklogIssue`). v1 targets GitHub, where `url` is always present. +- **Shared-`RegExp` reentrancy**: avoided by constructing the regex per call (d.ts explicitly warns + against the module-level shared pattern the sibling provider uses). + +## Test Plan + +**Unit (`terminal-ref-link-provider.test.ts`, vitest, `vi.mock('vscode')` + the three helper modules — +established pattern from `open-issue-by-id.test.ts` / `reconnect-link-provider.test.ts`):** + +- Detection: + - ordinary line → no links. + - `#915` → one link, `{number:'915', isPR:false}`, span == `#915`. + - `PR #1402` → one link, `{number:'1402', isPR:true}`, span == `PR #1402`. + - `see #12 and PR #34` → two links, correct flags/spans, inner `#34` claimed once (not double). + - `#fff` and `# 1` → no links. +- Resolution routing (fake `connectionManager` with controllable `client.getIssue`; assert which helper is called): + - explicit PR ref → `openPRInBrowser`; `getIssue` **not** called. + - bare `#N`, `getIssue` url `/issues/`, setting `editor` → `viewBacklogIssue`. + - bare `#N`, `getIssue` url `/issues/`, setting `browser` → `openIssueInBrowser`. + - bare `#N`, `getIssue` url `/pull/` → `openPRInBrowser` (PR fallthrough). + - `getIssue` → null → warning toast, no helper called. + - not connected → error toast. + +**Manual (dev-approval gate — live check in a real architect terminal on this workspace):** + +- In an architect terminal, Cmd+click `#1412` → in-editor issue preview opens (default `editor`). +- Cmd+click `PR #1405` → browser opens `.../pull/1405`. +- Cmd+click a bare number that is actually a PR (e.g. `#1405`) → browser opens the PR page (fallthrough). +- Set `codev.terminalLinks.issueTarget: browser`, Cmd+click `#1412` → browser opens the issue. +- Cmd+click a nonexistent number → warning toast; no silent fallthrough to workspace search. +- Line with two refs → both are individually clickable. + +**Build/verify:** `pnpm --filter codev-vscode build` and the vitest suite from the worktree. diff --git a/codev/state/pir-1412_thread.md b/codev/state/pir-1412_thread.md new file mode 100644 index 000000000..839b81da1 --- /dev/null +++ b/codev/state/pir-1412_thread.md @@ -0,0 +1,24 @@ +# pir-1412 — Clickable `#N` / `PR #N` terminal refs (VS Code) + +## Plan phase (2026-08-12) + +Wrote `codev/plans/1412-vscode-terminallinkprovider-ma.md`. Two empirical verifications drove the design: + +1. **Discriminator (architect concern #3):** `gh issue view ` does NOT fail on a PR number — it + resolves (exit 0) with a `.../pull/N` url. So issue-vs-PR must be decided by the **url path segment** + (`/pull/` vs `/issues/`), read off `IssueView.url`, not by fetch-failure. This is the faithful reading + of the decided "if the number resolves as a PR instead" behavior. +2. **API (concern #1):** against pinned `@types/vscode ~1.105.0`, `TerminalLinkContext.line` is the + **unwrapped logical line** — so no wrap limitation (concern #4). d.ts warns against shared-`RegExp` + reentrancy → build the regex per call (unlike the sibling `BuilderTerminalLinkProvider`). + +Design: extend existing `terminal-link-provider.ts` with `IssueRefTerminalLinkProvider`; new +`commands/open-terminal-ref.ts` holds resolution and delegates to the three sanctioned reuse helpers +(`openPRInBrowser` / `openIssueInBrowser` / `viewBacklogIssue`) — no new fetch code. New setting +`codev.terminalLinks.issueTarget` (editor|browser, default editor). Bare-#N click costs 2 SDK +round-trips (discriminator + helper re-fetch); accepted for single-owner-per-destination — documented. + +Registered in extension.ts beside the two existing providers. `vscode/` app only — no skeleton mirror +(this is our extension, not framework template content). + +**Status:** plan committed, awaiting `plan-approval` gate. dev-approval will be a live terminal check. From 660fda792e29e0b8285769b056bb860184748ecb Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 09:42:11 +1000 Subject: [PATCH 03/21] chore(porch): 1412 plan-approval gate-requested --- codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 375743d01..c70c1bcab 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -7,6 +7,7 @@ current_plan_phase: null gates: plan-approval: status: pending + requested_at: '2026-08-11T23:42:11.856Z' dev-approval: status: pending pr: @@ -15,4 +16,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-11T23:35:58.128Z' +updated_at: '2026-08-11T23:42:11.857Z' From 1ea365ecc9132f0f526229fb2a508d11b76302d3 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 11:45:09 +1000 Subject: [PATCH 04/21] [PIR #1412] Plan: rebase on main, correct extension.ts line refs --- codev/plans/1412-vscode-terminallinkprovider-ma.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/plans/1412-vscode-terminallinkprovider-ma.md b/codev/plans/1412-vscode-terminallinkprovider-ma.md index 30381034a..32d59fa7e 100644 --- a/codev/plans/1412-vscode-terminallinkprovider-ma.md +++ b/codev/plans/1412-vscode-terminallinkprovider-ma.md @@ -132,7 +132,7 @@ Regex notes: the optional greedy `(?\bPR\s+)?` prefers the `PR #N` form when (matchAll advances past the consumed span). `\b` before `PR` avoids `SUPR #12`. `#\d+` never matches `#fff` (hex color) or `# 1` (heading, space before digit). Case-insensitive so `Pr`/`pr` also match. -### Registration — `apps/vscode/src/extension.ts` (~line 1440, beside the existing two) +### Registration — `apps/vscode/src/extension.ts` (~line 1450, beside the existing two providers registered at 1440/1447) ```ts context.subscriptions.push( @@ -165,7 +165,7 @@ context.subscriptions.push( + delegation to the three reuse helpers + setting read. - `apps/vscode/src/terminal-link-provider.ts` — add `IssueRefTerminalLinkProvider` (+ `IssueRefLink`), import `openTerminalRef` and `ConnectionManager` type. -- `apps/vscode/src/extension.ts:~1440` — register the provider (one `push`, beside the existing two). +- `apps/vscode/src/extension.ts:~1450` — register the provider (one `push`, beside the existing two). - `apps/vscode/package.json` — add `codev.terminalLinks.issueTarget` under `contributes.configuration.properties`. - `apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts` — **new**. Detection + resolution tests. From a1c38283a649b2b048475e263c134598c6eaba55 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 11:55:46 +1000 Subject: [PATCH 05/21] chore(porch): 1412 plan-approval gate-approved --- .../projects/1412-vscode-terminallinkprovider-ma/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index c70c1bcab..995e2244c 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -6,8 +6,9 @@ plan_phases: [] current_plan_phase: null gates: plan-approval: - status: pending + status: approved requested_at: '2026-08-11T23:42:11.856Z' + approved_at: '2026-08-12T01:55:46.552Z' dev-approval: status: pending pr: @@ -16,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-11T23:42:11.857Z' +updated_at: '2026-08-12T01:55:46.552Z' From 29a4443b9864d6f7afe05226d20cf59e45ab93c2 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 11:55:55 +1000 Subject: [PATCH 06/21] chore(porch): 1412 implement phase-transition --- .../projects/1412-vscode-terminallinkprovider-ma/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 995e2244c..72335c8d0 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -1,7 +1,7 @@ id: '1412' title: vscode-terminallinkprovider-ma protocol: pir -phase: plan +phase: implement plan_phases: [] current_plan_phase: null gates: @@ -17,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T01:55:46.552Z' +updated_at: '2026-08-12T01:55:55.013Z' From 9017f4d581a21adee20ee051ed4906c035ecd74c Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 12:00:21 +1000 Subject: [PATCH 07/21] [PIR #1412] Clickable #N / PR #N terminal references Add IssueRefTerminalLinkProvider claiming #N and PR #N spans; resolve via the existing openPRInBrowser / openIssueInBrowser / codev.viewBacklogIssue paths. Bare #N discriminates issue vs PR by the resolved url (/pull/ vs /issues/), since gh issue view resolves PR numbers too. New setting codev.terminalLinks.issueTarget (editor|browser, default editor). --- apps/vscode/package.json | 10 +++ apps/vscode/src/commands/open-terminal-ref.ts | 87 +++++++++++++++++++ apps/vscode/src/extension.ts | 9 +- apps/vscode/src/terminal-link-provider.ts | 44 ++++++++++ 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 apps/vscode/src/commands/open-terminal-ref.ts diff --git a/apps/vscode/package.json b/apps/vscode/package.json index 8ef08fb5c..e0ae86c02 100644 --- a/apps/vscode/package.json +++ b/apps/vscode/package.json @@ -1042,6 +1042,16 @@ "default": "editor", "description": "Where to open Codev terminals (editor area or bottom panel)" }, + "codev.terminalLinks.issueTarget": { + "type": "string", + "enum": ["editor", "browser"], + "enumDescriptions": [ + "Open bare #N issue references in the in-editor issue viewer.", + "Open bare #N issue references in your browser." + ], + "default": "editor", + "markdownDescription": "Where Cmd+clicking a bare `#N` issue reference in a terminal opens it. `PR #N` references always open in the browser (there is no in-editor PR preview)." + }, "codev.diffCodelensMode": { "type": "string", "enum": ["comment", "forward"], diff --git a/apps/vscode/src/commands/open-terminal-ref.ts b/apps/vscode/src/commands/open-terminal-ref.ts new file mode 100644 index 000000000..637aa0d89 --- /dev/null +++ b/apps/vscode/src/commands/open-terminal-ref.ts @@ -0,0 +1,87 @@ +/** + * Codev: resolve a `#N` / `PR #N` reference clicked in a terminal (#1412) and + * open it in the right surface. This is the resolution half of the terminal + * link feature; the detection half is `IssueRefTerminalLinkProvider` in + * `terminal-link-provider.ts`. + * + * Reuse discipline (the core review axis): every *open* funnels through one of + * the three existing sanctioned paths — `openPRInBrowser` (open-pr-by-id.ts), + * `openIssueInBrowser` (open-issue-by-id.ts), and the `codev.viewBacklogIssue` + * command (view-issue.ts). The command is invoked via `executeCommand` (not a + * direct import) so this module carries no load-time dependency on view-issue's + * singleton — the same indirection open-issue-by-id.ts uses for its fallback. + * No new fetch code: the one `getIssue` call below is the same forge-agnostic + * SDK method those helpers use, invoked solely as the issue-vs-PR discriminator. + * + * The discriminator is the resolved `url`, not fetch-failure. GitHub's + * `gh issue view` (the `issue-view` forge concept) resolves a PR *number* + * successfully — issues and PRs share one number space — returning a + * `.../pull/N` url. A genuine issue returns `.../issues/N`. So a bare `#N` + * whose `getIssue` url is a `/pull/` url is actually a PR and falls through to + * the PR browser-open, exactly as the decided design intends. + */ + +import * as vscode from 'vscode'; +import type { ConnectionManager } from '../connection-manager.js'; +import { openIssueInBrowser } from './open-issue-by-id.js'; +import { openPRInBrowser } from './open-pr-by-id.js'; + +/** A `#N` or `PR #N` reference detected in a terminal line. */ +export interface TerminalRef { + /** Bare numeric id, e.g. `"1402"`. */ + number: string; + /** True when the reference carried an explicit `PR ` prefix. */ + isPR: boolean; +} + +/** + * Open the surface for a clicked terminal reference. + * + * - `PR #N` → the PR's forge page in the browser (no in-editor PR preview exists). + * - bare `#N` → the in-editor issue viewer by default, or the browser when + * `codev.terminalLinks.issueTarget` is `browser`; but if the number resolves + * to a PR, the browser PR page regardless of the setting. + * - unresolvable number → warning toast (matches `openIssueInBrowser`'s grammar). + */ +export async function openTerminalRef( + connectionManager: ConnectionManager, + ref: TerminalRef, +): Promise { + if (ref.isPR) { + await openPRInBrowser(connectionManager, ref.number); + return; + } + + // Bare `#N`: fetch once to discriminate issue vs PR by the resolved url. This + // guard mirrors the reuse helpers so an unconnected click fails the same way. + const client = connectionManager.getClient(); + const workspacePath = connectionManager.getWorkspacePath(); + if (!client || !workspacePath || connectionManager.getState() !== 'connected') { + vscode.window.showErrorMessage('Codev: Not connected to Tower'); + return; + } + + const issue = await client.getIssue(ref.number, workspacePath); + if (!issue) { + vscode.window.showWarningMessage( + `Codev: Could not open #${ref.number} (not found, or forge unavailable).`, + ); + return; + } + + if (issue.url && /\/pull\/\d/.test(issue.url)) { + // The number is actually a PR — funnel through the single PR-open path. + await openPRInBrowser(connectionManager, ref.number); + return; + } + + // Genuine issue (or a forge that supplies no url — treat as an issue). + const target = vscode.workspace + .getConfiguration('codev') + .get('terminalLinks.issueTarget', 'editor'); + if (target === 'browser') { + await openIssueInBrowser(connectionManager, ref.number); + return; + } + await vscode.commands.executeCommand('codev.viewBacklogIssue', ref.number); +} diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index 4f1de7b37..cf99a772d 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -43,7 +43,7 @@ import { submitReview, discardReviewComments } from './review-queue/submit.js'; import { activateSubmitReviewStatusBar } from './review-queue/status-bar.js'; import { MarkdownPreviewProvider } from './markdown-preview/preview-provider.js'; import { BuilderSpawnHandler } from './builder-spawn-handler.js'; -import { BuilderTerminalLinkProvider, ReconnectTerminalLinkProvider } from './terminal-link-provider.js'; +import { BuilderTerminalLinkProvider, ReconnectTerminalLinkProvider, IssueRefTerminalLinkProvider } from './terminal-link-provider.js'; import { computeBuildersToClose, roleIdsFromBuilders } from './prune-builder-terminals.js'; import { buildBuilderPickRows } from './builder-pick-rows.js'; import { readBuildersFileViewAsTree } from './builders-config.js'; @@ -1449,6 +1449,13 @@ export async function activate(context: vscode.ExtensionContext) { ), ); + // Make #N / PR #N references clickable in any terminal output (#1412) + context.subscriptions.push( + vscode.window.registerTerminalLinkProvider( + new IssueRefTerminalLinkProvider(connectionManager), + ), + ); + // IDE empty-window surface (#1144 Part 3). Runtime code by hard // constraint: extension-contributed configurationDefaults register // asynchronously and race whatever renders at startup, so nothing diff --git a/apps/vscode/src/terminal-link-provider.ts b/apps/vscode/src/terminal-link-provider.ts index 4c5fe8805..d9376e2ec 100644 --- a/apps/vscode/src/terminal-link-provider.ts +++ b/apps/vscode/src/terminal-link-provider.ts @@ -1,6 +1,8 @@ import * as vscode from 'vscode'; import type { TerminalManager } from './terminal-manager.js'; +import type { ConnectionManager } from './connection-manager.js'; import { RECONNECT_LINK_TEXT } from './terminal-adapter.js'; +import { openTerminalRef } from './commands/open-terminal-ref.js'; // Matches Codev builder role names like `builder-spir-153`, `builder-bugfix-42`. const BUILDER_REGEX = /\bbuilder-[a-z]+-[a-z0-9]+\b/g; @@ -66,3 +68,45 @@ export class ReconnectTerminalLinkProvider implements vscode.TerminalLinkProvide this.terminalManager.reconnectByTerminal(link.terminal); } } + +interface IssueRefLink extends vscode.TerminalLink { + number: string; + isPR: boolean; +} + +/** + * Makes `#N` and `PR #N` references in terminal output clickable (#1412). + * `PR #N` opens the PR's forge page in the browser; a bare `#N` opens the + * in-editor issue viewer (or the browser, per `codev.terminalLinks.issueTarget`), + * falling through to the PR browser-open when the number turns out to be a PR. + * Claiming the span also stops VSCode's useless fallback word-search over it. + */ +export class IssueRefTerminalLinkProvider implements vscode.TerminalLinkProvider { + constructor(private connectionManager: ConnectionManager) {} + + provideTerminalLinks(context: vscode.TerminalLinkContext): IssueRefLink[] { + // Built per call, not shared at module scope: the VSCode d.ts warns that + // provideTerminalLinks may be re-entered before a prior call resolves, and + // a shared /g regex's lastIndex would race across those overlapping calls. + // The optional greedy `PR ` prefix claims the whole `PR #N` span in one + // match, so the inner `#N` is not separately matched. + const re = /(?\bPR\s+)?#(?\d+)/gi; + const links: IssueRefLink[] = []; + for (const m of context.line.matchAll(re)) { + const num = m.groups!.num; + const isPR = Boolean(m.groups?.pr); + links.push({ + startIndex: m.index, + length: m[0].length, + tooltip: isPR ? `Open PR #${num} in browser` : `Open issue #${num}`, + number: num, + isPR, + }); + } + return links; + } + + handleTerminalLink(link: IssueRefLink): Promise { + return openTerminalRef(this.connectionManager, { number: link.number, isPR: link.isPR }); + } +} From a561c00015e8a33edf1f105a4673f84e69f177f7 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 12:00:21 +1000 Subject: [PATCH 08/21] [PIR #1412] Tests: terminal ref detection and resolution routing --- .../terminal-ref-link-provider.test.ts | 156 ++++++++++++++++++ codev/state/pir-1412_thread.md | 20 +++ 2 files changed, 176 insertions(+) create mode 100644 apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts diff --git a/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts b/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts new file mode 100644 index 000000000..99d5cee18 --- /dev/null +++ b/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts @@ -0,0 +1,156 @@ +/** + * PIR #1412: clickable `#N` / `PR #N` terminal references. + * + * Two halves: `IssueRefTerminalLinkProvider.provideTerminalLinks` (detection — + * spans, flags, multiple refs per line) and `openTerminalRef` (resolution — + * issue-first with the url-based PR discriminator, and the `issueTarget` + * setting). Both import `vscode` at module load, so we stub it (the established + * pattern from open-issue-by-id.test.ts), and stub the three reuse helper + * modules so we can assert delegation without real fetches. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const showErrorMessage = vi.fn(); +const showWarningMessage = vi.fn(); +const getConfiguration = vi.fn(); +const executeCommand = vi.fn(); + +vi.mock('vscode', () => ({ + window: { showErrorMessage, showWarningMessage }, + workspace: { getConfiguration }, + commands: { executeCommand }, +})); + +const openPRInBrowser = vi.fn(); +const openIssueInBrowser = vi.fn(); + +vi.mock('../commands/open-pr-by-id.js', () => ({ openPRInBrowser })); +vi.mock('../commands/open-issue-by-id.js', () => ({ openIssueInBrowser })); + +// terminal-link-provider.ts pulls in terminal-adapter.js (via RECONNECT_LINK_TEXT) +// and terminal-manager types; stub the adapter so the module loads in isolation. +vi.mock('../terminal-adapter.js', () => ({ RECONNECT_LINK_TEXT: '[reconnect]' })); + +const { IssueRefTerminalLinkProvider } = await import('../terminal-link-provider.js'); +const { openTerminalRef } = await import('../commands/open-terminal-ref.js'); + +function makeProvider() { + return new (IssueRefTerminalLinkProvider as unknown as new (cm: unknown) => { + provideTerminalLinks(ctx: { line: string; terminal: unknown }): Array<{ + startIndex: number; length: number; number: string; isPR: boolean; tooltip: string; + }>; + })({}); +} + +describe('PIR #1412 — IssueRefTerminalLinkProvider detection', () => { + it('produces no link on an ordinary line', () => { + expect(makeProvider().provideTerminalLinks({ line: 'nothing to see here', terminal: {} })) + .toHaveLength(0); + }); + + it('claims a bare #N and spans exactly the token', () => { + const line = 'see #915 for context'; + const links = makeProvider().provideTerminalLinks({ line, terminal: {} }); + expect(links).toHaveLength(1); + expect(line.substr(links[0].startIndex, links[0].length)).toBe('#915'); + expect(links[0]).toMatchObject({ number: '915', isPR: false }); + }); + + it('claims the whole `PR #N` span (not the inner #N)', () => { + const line = 'merged PR #1402 today'; + const links = makeProvider().provideTerminalLinks({ line, terminal: {} }); + expect(links).toHaveLength(1); + expect(line.substr(links[0].startIndex, links[0].length)).toBe('PR #1402'); + expect(links[0]).toMatchObject({ number: '1402', isPR: true }); + }); + + it('detects multiple refs per line with correct flags and spans', () => { + const line = 'see #12 and PR #34 before #56'; + const links = makeProvider().provideTerminalLinks({ line, terminal: {} }); + expect(links).toHaveLength(3); + expect(links.map((l) => ({ number: l.number, isPR: l.isPR }))).toEqual([ + { number: '12', isPR: false }, + { number: '34', isPR: true }, + { number: '56', isPR: false }, + ]); + for (const l of links) { + expect(line.substr(l.startIndex, l.length)).toMatch(/^(PR )?#\d+$/); + } + }); + + it('ignores non-numeric (#fff) and spaced (# 1) forms', () => { + expect(makeProvider().provideTerminalLinks({ line: 'color #fff and # 1 heading', terminal: {} })) + .toHaveLength(0); + }); + + it('is case-insensitive on the PR prefix', () => { + const links = makeProvider().provideTerminalLinks({ line: 'pr #7', terminal: {} }); + expect(links).toHaveLength(1); + expect(links[0]).toMatchObject({ number: '7', isPR: true }); + }); +}); + +describe('PIR #1412 — openTerminalRef resolution', () => { + const getIssue = vi.fn(); + const connected = { + getClient: () => ({ getIssue }), + getWorkspacePath: () => '/work', + getState: () => 'connected', + }; + + function withSetting(target: string) { + getConfiguration.mockReturnValue({ get: () => target }); + } + + beforeEach(() => { + vi.clearAllMocks(); + withSetting('editor'); + }); + + it('routes an explicit PR ref straight to openPRInBrowser without a discriminator fetch', async () => { + await openTerminalRef(connected as never, { number: '1402', isPR: true }); + expect(openPRInBrowser).toHaveBeenCalledWith(connected, '1402'); + expect(getIssue).not.toHaveBeenCalled(); + }); + + it('opens a genuine issue in the editor viewer by default', async () => { + getIssue.mockResolvedValue({ url: 'https://github.com/o/r/issues/915' }); + await openTerminalRef(connected as never, { number: '915', isPR: false }); + expect(executeCommand).toHaveBeenCalledWith('codev.viewBacklogIssue', '915'); + expect(openIssueInBrowser).not.toHaveBeenCalled(); + expect(openPRInBrowser).not.toHaveBeenCalled(); + }); + + it('opens a genuine issue in the browser when issueTarget = browser', async () => { + withSetting('browser'); + getIssue.mockResolvedValue({ url: 'https://github.com/o/r/issues/915' }); + await openTerminalRef(connected as never, { number: '915', isPR: false }); + expect(openIssueInBrowser).toHaveBeenCalledWith(connected, '915'); + expect(executeCommand).not.toHaveBeenCalled(); + }); + + it('falls through to the PR browser-open when a bare #N resolves to a PR url', async () => { + getIssue.mockResolvedValue({ url: 'https://github.com/o/r/pull/1405' }); + await openTerminalRef(connected as never, { number: '1405', isPR: false }); + expect(openPRInBrowser).toHaveBeenCalledWith(connected, '1405'); + expect(executeCommand).not.toHaveBeenCalled(); + expect(openIssueInBrowser).not.toHaveBeenCalled(); + }); + + it('warns and opens nothing when the number is unresolvable', async () => { + getIssue.mockResolvedValue(null); + await openTerminalRef(connected as never, { number: '999999', isPR: false }); + expect(showWarningMessage).toHaveBeenCalledWith(expect.stringContaining('#999999')); + expect(executeCommand).not.toHaveBeenCalled(); + expect(openIssueInBrowser).not.toHaveBeenCalled(); + expect(openPRInBrowser).not.toHaveBeenCalled(); + }); + + it('errors on a bare #N click when not connected', async () => { + const disconnected = { getClient: () => null, getWorkspacePath: () => null, getState: () => 'disconnected' }; + await openTerminalRef(disconnected as never, { number: '915', isPR: false }); + expect(showErrorMessage).toHaveBeenCalledWith('Codev: Not connected to Tower'); + expect(getIssue).not.toHaveBeenCalled(); + }); +}); diff --git a/codev/state/pir-1412_thread.md b/codev/state/pir-1412_thread.md index 839b81da1..d02650c59 100644 --- a/codev/state/pir-1412_thread.md +++ b/codev/state/pir-1412_thread.md @@ -22,3 +22,23 @@ Registered in extension.ts beside the two existing providers. `vscode/` app only (this is our extension, not framework template content). **Status:** plan committed, awaiting `plan-approval` gate. dev-approval will be a live terminal check. + +## Implement phase (2026-08-12) + +plan-approval approved. Rebased on main first (was 95 behind). Implemented: +- `commands/open-terminal-ref.ts` — resolution: PR#N→openPRInBrowser; bare #N→getIssue discriminator + (url `/pull/`→PR fallthrough, else editor via `executeCommand('codev.viewBacklogIssue')` or browser + via openIssueInBrowser per `codev.terminalLinks.issueTarget`). +- `terminal-link-provider.ts` — added `IssueRefTerminalLinkProvider` (regex `/(?\bPR\s+)?#(?\d+)/gi`, + built per call). +- `extension.ts` — registered beside the two existing providers. +- `package.json` — `codev.terminalLinks.issueTarget` setting (editor|browser, default editor). +- `__tests__/terminal-ref-link-provider.test.ts` — 12 tests (detection + resolution routing). + +**Gotcha caught by tests:** first version imported `viewBacklogIssue` directly from view-issue.js, whose +module-level `new vscode.EventEmitter()` singleton broke `reconnect-link-provider.test.ts` (loads the +provider with a bare vscode mock). Switched to `executeCommand('codev.viewBacklogIssue', N)` — the same +indirection open-issue-by-id.ts already uses, no load-time coupling. Full suite green after. + +Verified from worktree: `pnpm compile` (check-types + lint) clean (1 pre-existing tunnel.ts lint warning, +not mine); `pnpm test:unit` 806 passed (68 files). Awaiting `dev-approval` — live terminal demo. From d1250a34b1a8bc29f25325aaa0547fff29d4835f Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 12:01:06 +1000 Subject: [PATCH 09/21] chore(porch): 1412 dev-approval gate-requested --- codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 72335c8d0..956798b8a 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -11,10 +11,11 @@ gates: approved_at: '2026-08-12T01:55:46.552Z' dev-approval: status: pending + requested_at: '2026-08-12T02:01:06.883Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T01:55:55.013Z' +updated_at: '2026-08-12T02:01:06.885Z' From cb12cab64fe17fdf09de97ae20bf4610a0232428 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:06:11 +1000 Subject: [PATCH 10/21] [PIR #1412] Add click feedback + drop redundant fetch on terminal ref open Wrap resolution in a status-bar withProgress so a click gives instant feedback, and open the discriminator's already-resolved url directly (openExternal) on the PR-fallthrough and browser-issue paths instead of re-fetching via a helper. Bare #N is now a single forge round-trip except the in-editor preview, whose fetch renders the content. --- .../terminal-ref-link-provider.test.ts | 32 ++++++++---- apps/vscode/src/commands/open-terminal-ref.ts | 52 +++++++++++-------- apps/vscode/src/terminal-link-provider.ts | 2 +- codev/state/pir-1412_thread.md | 13 +++++ 4 files changed, 67 insertions(+), 32 deletions(-) diff --git a/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts b/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts index 99d5cee18..94aa474f4 100644 --- a/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts +++ b/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts @@ -15,18 +15,22 @@ const showErrorMessage = vi.fn(); const showWarningMessage = vi.fn(); const getConfiguration = vi.fn(); const executeCommand = vi.fn(); +const openExternal = vi.fn(); +// withProgress just runs the task; the status-bar spinner is UI-only. +const withProgress = vi.fn((_opts: unknown, task: () => unknown) => task()); vi.mock('vscode', () => ({ - window: { showErrorMessage, showWarningMessage }, + window: { showErrorMessage, showWarningMessage, withProgress }, workspace: { getConfiguration }, commands: { executeCommand }, + env: { openExternal }, + Uri: { parse: (s: string) => ({ __parsed: s }) }, + ProgressLocation: { Window: 10 }, })); const openPRInBrowser = vi.fn(); -const openIssueInBrowser = vi.fn(); vi.mock('../commands/open-pr-by-id.js', () => ({ openPRInBrowser })); -vi.mock('../commands/open-issue-by-id.js', () => ({ openIssueInBrowser })); // terminal-link-provider.ts pulls in terminal-adapter.js (via RECONNECT_LINK_TEXT) // and terminal-manager types; stub the adapter so the module loads in isolation. @@ -118,24 +122,32 @@ describe('PIR #1412 — openTerminalRef resolution', () => { getIssue.mockResolvedValue({ url: 'https://github.com/o/r/issues/915' }); await openTerminalRef(connected as never, { number: '915', isPR: false }); expect(executeCommand).toHaveBeenCalledWith('codev.viewBacklogIssue', '915'); - expect(openIssueInBrowser).not.toHaveBeenCalled(); + expect(openExternal).not.toHaveBeenCalled(); expect(openPRInBrowser).not.toHaveBeenCalled(); }); - it('opens a genuine issue in the browser when issueTarget = browser', async () => { + it('opens a genuine issue in the browser via the already-resolved url when issueTarget = browser', async () => { withSetting('browser'); getIssue.mockResolvedValue({ url: 'https://github.com/o/r/issues/915' }); await openTerminalRef(connected as never, { number: '915', isPR: false }); - expect(openIssueInBrowser).toHaveBeenCalledWith(connected, '915'); + expect(openExternal).toHaveBeenCalledWith({ __parsed: 'https://github.com/o/r/issues/915' }); expect(executeCommand).not.toHaveBeenCalled(); }); - it('falls through to the PR browser-open when a bare #N resolves to a PR url', async () => { + it('opens a browser-target issue in the editor preview when the forge supplies no url', async () => { + withSetting('browser'); + getIssue.mockResolvedValue({ url: undefined }); + await openTerminalRef(connected as never, { number: '915', isPR: false }); + expect(executeCommand).toHaveBeenCalledWith('codev.viewBacklogIssue', '915'); + expect(openExternal).not.toHaveBeenCalled(); + }); + + it('falls through to the PR page via the resolved /pull/ url when a bare #N is actually a PR', async () => { getIssue.mockResolvedValue({ url: 'https://github.com/o/r/pull/1405' }); await openTerminalRef(connected as never, { number: '1405', isPR: false }); - expect(openPRInBrowser).toHaveBeenCalledWith(connected, '1405'); + expect(openExternal).toHaveBeenCalledWith({ __parsed: 'https://github.com/o/r/pull/1405' }); expect(executeCommand).not.toHaveBeenCalled(); - expect(openIssueInBrowser).not.toHaveBeenCalled(); + expect(openPRInBrowser).not.toHaveBeenCalled(); }); it('warns and opens nothing when the number is unresolvable', async () => { @@ -143,7 +155,7 @@ describe('PIR #1412 — openTerminalRef resolution', () => { await openTerminalRef(connected as never, { number: '999999', isPR: false }); expect(showWarningMessage).toHaveBeenCalledWith(expect.stringContaining('#999999')); expect(executeCommand).not.toHaveBeenCalled(); - expect(openIssueInBrowser).not.toHaveBeenCalled(); + expect(openExternal).not.toHaveBeenCalled(); expect(openPRInBrowser).not.toHaveBeenCalled(); }); diff --git a/apps/vscode/src/commands/open-terminal-ref.ts b/apps/vscode/src/commands/open-terminal-ref.ts index 637aa0d89..9eee9dbe1 100644 --- a/apps/vscode/src/commands/open-terminal-ref.ts +++ b/apps/vscode/src/commands/open-terminal-ref.ts @@ -4,26 +4,25 @@ * link feature; the detection half is `IssueRefTerminalLinkProvider` in * `terminal-link-provider.ts`. * - * Reuse discipline (the core review axis): every *open* funnels through one of - * the three existing sanctioned paths — `openPRInBrowser` (open-pr-by-id.ts), - * `openIssueInBrowser` (open-issue-by-id.ts), and the `codev.viewBacklogIssue` - * command (view-issue.ts). The command is invoked via `executeCommand` (not a - * direct import) so this module carries no load-time dependency on view-issue's - * singleton — the same indirection open-issue-by-id.ts uses for its fallback. - * No new fetch code: the one `getIssue` call below is the same forge-agnostic - * SDK method those helpers use, invoked solely as the issue-vs-PR discriminator. + * Latency + feedback (#1412 dev-approval): a terminal-link click gets no + * built-in VSCode feedback, so the whole resolution runs inside a status-bar + * `withProgress` — the user sees "Opening #N…" the instant they click, even + * while the forge fetch is in flight. And a bare `#N` fetches the forge exactly + * once: that single `getIssue` both discriminates issue-vs-PR AND yields the + * canonical `url`, which we then open directly rather than making a reuse + * helper re-fetch it. Only the in-editor issue preview fetches a second time, + * because that fetch renders the content the user is about to read. * * The discriminator is the resolved `url`, not fetch-failure. GitHub's * `gh issue view` (the `issue-view` forge concept) resolves a PR *number* * successfully — issues and PRs share one number space — returning a * `.../pull/N` url. A genuine issue returns `.../issues/N`. So a bare `#N` - * whose `getIssue` url is a `/pull/` url is actually a PR and falls through to - * the PR browser-open, exactly as the decided design intends. + * whose `getIssue` url is a `/pull/` url is actually a PR and opens the PR page, + * exactly as the decided design intends. */ import * as vscode from 'vscode'; import type { ConnectionManager } from '../connection-manager.js'; -import { openIssueInBrowser } from './open-issue-by-id.js'; import { openPRInBrowser } from './open-pr-by-id.js'; /** A `#N` or `PR #N` reference detected in a terminal line. */ @@ -35,7 +34,8 @@ export interface TerminalRef { } /** - * Open the surface for a clicked terminal reference. + * Open the surface for a clicked terminal reference, with a status-bar spinner + * for the duration. * * - `PR #N` → the PR's forge page in the browser (no in-editor PR preview exists). * - bare `#N` → the in-editor issue viewer by default, or the browser when @@ -43,17 +43,25 @@ export interface TerminalRef { * to a PR, the browser PR page regardless of the setting. * - unresolvable number → warning toast (matches `openIssueInBrowser`'s grammar). */ -export async function openTerminalRef( +export function openTerminalRef( connectionManager: ConnectionManager, ref: TerminalRef, -): Promise { +): Thenable { + return vscode.window.withProgress( + { location: vscode.ProgressLocation.Window, title: `Codev: Opening #${ref.number}…` }, + () => resolveRef(connectionManager, ref), + ); +} + +async function resolveRef(connectionManager: ConnectionManager, ref: TerminalRef): Promise { if (ref.isPR) { await openPRInBrowser(connectionManager, ref.number); return; } - // Bare `#N`: fetch once to discriminate issue vs PR by the resolved url. This - // guard mirrors the reuse helpers so an unconnected click fails the same way. + // Bare `#N`: one fetch does double duty — discriminate issue vs PR by the + // resolved url, and hand us that url to open. This guard mirrors the reuse + // helpers so an unconnected click fails the same way. const client = connectionManager.getClient(); const workspacePath = connectionManager.getWorkspacePath(); if (!client || !workspacePath || connectionManager.getState() !== 'connected') { @@ -69,18 +77,20 @@ export async function openTerminalRef( return; } + // The number is actually a PR — open the `/pull/` url we already hold. if (issue.url && /\/pull\/\d/.test(issue.url)) { - // The number is actually a PR — funnel through the single PR-open path. - await openPRInBrowser(connectionManager, ref.number); + await vscode.env.openExternal(vscode.Uri.parse(issue.url)); return; } - // Genuine issue (or a forge that supplies no url — treat as an issue). + // Genuine issue. Browser target opens the resolved url directly (no re-fetch); + // editor target — and any forge that supplied no url — renders the in-editor + // preview, which fetches once to build its content. const target = vscode.workspace .getConfiguration('codev') .get('terminalLinks.issueTarget', 'editor'); - if (target === 'browser') { - await openIssueInBrowser(connectionManager, ref.number); + if (target === 'browser' && issue.url) { + await vscode.env.openExternal(vscode.Uri.parse(issue.url)); return; } await vscode.commands.executeCommand('codev.viewBacklogIssue', ref.number); diff --git a/apps/vscode/src/terminal-link-provider.ts b/apps/vscode/src/terminal-link-provider.ts index d9376e2ec..8d8de3cf7 100644 --- a/apps/vscode/src/terminal-link-provider.ts +++ b/apps/vscode/src/terminal-link-provider.ts @@ -106,7 +106,7 @@ export class IssueRefTerminalLinkProvider implements vscode.TerminalLinkProvider return links; } - handleTerminalLink(link: IssueRefLink): Promise { + handleTerminalLink(link: IssueRefLink): Thenable { return openTerminalRef(this.connectionManager, { number: link.number, isPR: link.isPR }); } } diff --git a/codev/state/pir-1412_thread.md b/codev/state/pir-1412_thread.md index d02650c59..05ad8fce2 100644 --- a/codev/state/pir-1412_thread.md +++ b/codev/state/pir-1412_thread.md @@ -42,3 +42,16 @@ indirection open-issue-by-id.ts already uses, no load-time coupling. Full suite Verified from worktree: `pnpm compile` (check-types + lint) clean (1 pre-existing tunnel.ts lint warning, not mine); `pnpm test:unit` 806 passed (68 files). Awaiting `dev-approval` — live terminal demo. + +### dev-approval feedback #1 — perceived slowness (2026-08-12) + +Reviewer: working but ~2s with no feedback → feels like nothing happens. Root cause: (a) VSCode gives +no click feedback on terminal links; (b) bare #N did TWO gh round-trips (discriminator getIssue + the +reuse helper re-fetching). Fix: +- Wrapped resolution in `withProgress` (status-bar "Opening #N…") for instant feedback. +- Bare #N now fetches once: the discriminator getIssue's `url` is opened directly via `openExternal` + for the PR-fallthrough and browser-issue paths (no helper re-fetch). Editor preview still fetches once + to render (that fetch IS the content). Explicit PR #N unchanged (openPRInBrowser, no discriminator). +- Tradeoff: PR can now be browser-opened by two paths (explicit vs fallthrough); still no new fetch code + (openExternal opens a url already fetched via the sanctioned getIssue). Flagged to reviewer. +Green: check-types + lint clean, 807 unit tests. From ee6dea3f140e4b97a7f3ea985bea1f29a580315e Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:14:30 +1000 Subject: [PATCH 11/21] [PIR #1412] Reword issueTarget setting: drop internal 'bare #N' jargon --- apps/vscode/package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/vscode/package.json b/apps/vscode/package.json index e0ae86c02..dfc724d13 100644 --- a/apps/vscode/package.json +++ b/apps/vscode/package.json @@ -1046,11 +1046,11 @@ "type": "string", "enum": ["editor", "browser"], "enumDescriptions": [ - "Open bare #N issue references in the in-editor issue viewer.", - "Open bare #N issue references in your browser." + "Open issue references (#N) in the in-editor issue viewer.", + "Open issue references (#N) in your browser." ], "default": "editor", - "markdownDescription": "Where Cmd+clicking a bare `#N` issue reference in a terminal opens it. `PR #N` references always open in the browser (there is no in-editor PR preview)." + "markdownDescription": "Where Cmd+clicking an issue reference (`#N`) in a terminal opens it. Pull-request references (`PR #N`) always open in the browser (there is no in-editor PR preview)." }, "codev.diffCodelensMode": { "type": "string", From 17c198a8a064d300227546aa018b92001daede73 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:16:45 +1000 Subject: [PATCH 12/21] chore(porch): 1412 dev-approval gate-approved --- .../projects/1412-vscode-terminallinkprovider-ma/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 956798b8a..1612f60ca 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -10,12 +10,13 @@ gates: requested_at: '2026-08-11T23:42:11.856Z' approved_at: '2026-08-12T01:55:46.552Z' dev-approval: - status: pending + status: approved requested_at: '2026-08-12T02:01:06.883Z' + approved_at: '2026-08-12T04:16:45.041Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T02:01:06.885Z' +updated_at: '2026-08-12T04:16:45.042Z' From ad1462281b60add8b4d88f1f80033181729749d7 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:16:51 +1000 Subject: [PATCH 13/21] chore(porch): 1412 review phase-transition --- .../projects/1412-vscode-terminallinkprovider-ma/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 1612f60ca..85c391d99 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -1,7 +1,7 @@ id: '1412' title: vscode-terminallinkprovider-ma protocol: pir -phase: implement +phase: review plan_phases: [] current_plan_phase: null gates: @@ -19,4 +19,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T04:16:45.042Z' +updated_at: '2026-08-12T04:16:51.831Z' From 326ed83605a0f431b557514855b5a8b1a678846a Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:18:39 +1000 Subject: [PATCH 14/21] [PIR #1412] Review + retrospective --- codev/resources/lessons-learned.md | 1 + .../1412-vscode-terminallinkprovider-ma.md | 58 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 codev/reviews/1412-vscode-terminallinkprovider-ma.md diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index 49ac02506..2ffef8a99 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -535,6 +535,7 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated - [From 1052] A PTY-side SIGWINCH redraws the running app's *current frame* but cannot re-wrap xterm.js's existing *scrollback* — so it fixes a blank/stale *live* frame (#1047) but not wrong-width *history* (#1052). #1052's corruption was the inverse failure: VSCode reports a new terminal's size in two steps (e.g. 112→114 cols, ~120ms apart), and the adapter painted the bracketed replay immediately at the first, not-yet-final width, so the restored history wrapped wrong and a stale frame stranded in scrollback (a ghost status bar, visible on scroll). The #1047 post-connect SIGWINCH nudge couldn't clear it (redraws the app, not the scrollback), and an `onDidOverrideDimensions` shrink-then-restore reflow made it *worse* — a down-then-up re-wrap churns scrollback wrap flags → scroll distortion. The fix again mirrored the web client (`Terminal.tsx` `flushInitialBuffer`): hold the replay and paint it **once after the size settles**, debounced on `setDimensions`, so it lands at the final width. Captured per-pathway diagnostic logging was decisive — four prior approaches (defer-until-sized, post-replay SIGWINCH, override reflow) were each tried and reverted before the log showed a mid-hold 112→114 resize resetting the debounce and the flush landing after it at the final width. Don't render a full-screen replay until the terminal geometry has stopped moving. - [From #1227] `parseInt(env.X || 'default', 10) || fallbackDefault` silently discards a legitimate `0` override — `0` is falsy in JS, so `0 || fallbackDefault` evaluates to `fallbackDefault`, not `0`. Check `Number.isNaN(parsed) ? fallbackDefault : parsed` instead. Caught only because an E2E test deliberately set an env var to `'0'` (for an immediate-eligibility grace period) and the periodic timer silently kept using the 1-hour default instead — a live, running-process assertion surfaced it where a unit test mocking the parse wouldn't have. +- [From #1412] GitHub's `issue-view` concept (`gh issue view N`) resolves a **PR** number too — issues and PRs share one number space, so `gh issue view ` returns the PR (exit 0) with a `.../pull/N` url; it does **not** fail. To tell an issue from a PR by number alone, discriminate on the resolved **url path** (`/pull/` vs `/issues/`), never on fetch-failure. Verify this empirically before designing around it: issue #1412's own decided design assumed the issue fetch would *fail* on a PR number, and it doesn't. Corollary: that one `getIssue` call then does double duty — the discriminator *and* the canonical url to open — so open the url you already hold rather than re-fetching through a helper just to reach the browser (the second forge round-trip is what made the first cut feel unresponsive). - [From 787] When adding a field to a multi-forge concept contract (`pr-list`, `issue-list`, …), per-CLI data availability diverges — do not assume parity across `gh`/`glab`/`tea`. Verify each empirically (live output or, failing that, the CLI's own `--help` and official docs): `gh pr list --json` exposes `isDraft` + `reviewRequests` (reviewer objects → flatten `.login`, dropping teams); `glab mr list --output json` exposes `draft` + `reviewers[].username` (same command, just add jq); but `tea pulls list` exposes *neither* (its JSON is limited to the selectable `--fields`, which omit draft/reviewers — the data exists only via raw `tea api`). Populate where the existing command can; default safely (`[]`/`false`) where it can't, and document the verified reason in the script. Defaulting a field the CLI *does* expose silently drops working data; rewriting a script onto a different command (e.g. `tea api`) to reach an unavailable field is a separate, larger change. Make the new required fields safe end-to-end by defaulting at the server mapping (`?? []` / `?? false`) so a non-conforming forge degrades rather than emitting `undefined`. - [From 863] React's `dangerouslySetInnerHTML` re-commits the element's innerHTML on *every* re-render, silently wiping any DOM children you injected imperatively into that same element. The artifact-canvas renders parsed markdown into the body via `dangerouslySetInnerHTML`, then injected inline comment cards as DOM children of the parsed blocks; the cards flashed on first paint and vanished on the next render (a refreshKey bump), while the React-owned minimap survived untouched — that asymmetry located the bug in the React-owned subtree, not the injection logic (a faithful jsdom repro of the injection round-trip kept the card every time, proving the defect was browser-render-only). Fix: stop letting React own that subtree — set `ref.innerHTML = html` imperatively inside a `[html]`-keyed `useEffect`, then inject the non-React DOM after, so React never re-commits those children (the standard escape hatch for mixing hand-built DOM into a React tree). Rule of thumb: never imperatively mutate the children of a node React controls via `dangerouslySetInnerHTML` — render everything through React, or own the whole subtree imperatively, never both. diff --git a/codev/reviews/1412-vscode-terminallinkprovider-ma.md b/codev/reviews/1412-vscode-terminallinkprovider-ma.md new file mode 100644 index 000000000..68ee3542c --- /dev/null +++ b/codev/reviews/1412-vscode-terminallinkprovider-ma.md @@ -0,0 +1,58 @@ +# PIR Review: Clickable `#N` / `PR #N` terminal references (VS Code) + +Fixes #1412 + +## Summary + +Architect and builder terminal output constantly cites issues and PRs by number, but Cmd+click on those spans fell through to VS Code's workspace word-search (which matches nothing). This PR registers a `TerminalLinkProvider` that claims `#N` and `PR #N` spans and opens them: `PR #N` → the PR's forge page in the browser; a bare `#N` → the in-editor issue viewer by default (or the browser, per a new `codev.terminalLinks.issueTarget` setting), falling through to the PR page when the number turns out to be a PR. Claiming the span also suppresses VS Code's useless fallback search. + +## Files Changed + +- `apps/vscode/src/commands/open-terminal-ref.ts` (+97 / -0) — new; click resolution (issue-vs-PR discriminator, setting, progress spinner) +- `apps/vscode/src/terminal-link-provider.ts` (+44 / -0) — new `IssueRefTerminalLinkProvider` (span detection) +- `apps/vscode/src/extension.ts` (+9 / -1) — register the provider beside the two existing ones +- `apps/vscode/package.json` (+10 / -0) — `codev.terminalLinks.issueTarget` setting +- `apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts` (+168 / -0) — new; detection + resolution routing tests +- `codev/resources/lessons-learned.md` (+1) — the `gh issue view` PR-discriminator lesson + +## Commits + +- `9017f4d58` [PIR #1412] Clickable #N / PR #N terminal references +- `a561c0001` [PIR #1412] Tests: terminal ref detection and resolution routing +- `cb12cab64` [PIR #1412] Add click feedback + drop redundant fetch on terminal ref open +- `ee6dea3f1` [PIR #1412] Reword issueTarget setting: drop internal 'bare #N' jargon + +## Test Results + +- `pnpm compile` (check-types + lint + esbuild): ✓ pass (0 type errors; 1 pre-existing `tunnel.ts` lint warning, not from this change) +- `pnpm test:unit`: ✓ pass (807 tests, 13 new in this file) +- Porch verify block (`build`, `tests`): ✓ pass +- Manual verification (human, at the `dev-approval` gate): Cmd+click `#N` opens the in-editor issue viewer; `PR #N` opens the browser PR page; a bare number that is a PR falls through to the browser; the `issueTarget: browser` setting flips bare `#N` to the browser. A first pass felt unresponsive (~2s, no feedback); fixed with a progress spinner + single forge round-trip, then re-verified. Wording of the setting description was tightened at the human's request. + +## Architecture Updates + +No arch changes. This adds a self-contained terminal link provider plus a resolution module inside the existing `apps/vscode/` structure — no module boundaries, invariants, ports, or state paths are affected, and it reuses the existing forge-fetch/open paths (`getIssue`, `openPRInBrowser`, `codev.viewBacklogIssue`) rather than introducing a new one. Not a HOT arch-critical fact; nothing durable to route to COLD `arch.md`. + +## Lessons Learned Updates + +Routed one COLD lesson to `codev/resources/lessons-learned.md` (near the `[From 787]` multi-forge contract lesson): `gh issue view ` **resolves** a PR number (exit 0, `.../pull/N` url) rather than failing — so issue-vs-PR must be discriminated on the resolved url path (`/pull/` vs `/issues/`), not on fetch-failure, and the single `getIssue` call should do double duty (discriminator + url-to-open) rather than triggering a second round-trip. Not HOT: it's a GitHub-forge-narrow recipe, not a behavior-changing cross-cutting rule (it reinforces the existing HOT "verify API behavior empirically" lesson rather than replacing it). + +## Things to Look At During PR Review + +- **The issue-vs-PR discriminator** (`open-terminal-ref.ts`, `/\/pull\/\d/` test on `issue.url`). This is the crux: `gh issue view` resolves PR numbers too, so fetch-failure is *not* the discriminator — the resolved url path is. Verified empirically (`gh issue view 1405` on a merged PR returns exit 0 with a `/pull/` url). If the forge supplies no `url` (non-GitHub), the code degrades to the issue path; v1 targets GitHub. +- **Reuse vs latency tradeoff** (`resolveRef`). The first cut funneled every open through `openPRInBrowser` / `openIssueInBrowser` for a single owner per destination, which meant bare `#N` did two forge round-trips (discriminator + helper re-fetch) and felt unresponsive. The current code opens the url the discriminator already resolved (`openExternal`) for the PR-fallthrough and browser-issue paths — no new fetch code, but a PR can now be browser-opened by two code paths (explicit `PR #N` via `openPRInBrowser`, and the bare-`#N`-is-a-PR fallthrough via `openExternal`). The in-editor issue preview still fetches once to render its content. +- **Regex reentrancy** (`terminal-link-provider.ts`). The `/(?\bPR\s+)?#(?\d+)/gi` is built *inside* `provideTerminalLinks`, not shared at module scope — the VS Code d.ts warns the method may be re-entered before a prior call resolves, and a shared `/g` regex's `lastIndex` would race. (The sibling `BuilderTerminalLinkProvider` uses the module-scope pattern; this one deliberately does not.) +- **Module-load coupling avoided**: the editor path uses `executeCommand('codev.viewBacklogIssue', N)` rather than importing `viewBacklogIssue` directly, because `view-issue.ts` instantiates a `vscode.EventEmitter` singleton at module load, which broke a sibling test that loads the provider with a bare `vscode` mock. Same indirection `open-issue-by-id.ts` already uses. + +## How to Test Locally + +- **View diff**: VS Code sidebar → right-click builder `pir-1412` → **Review Diff** +- **Run dev**: VS Code sidebar → **Run Dev**, or `afx dev pir-1412` +- **What to verify** (in an architect/builder terminal on this workspace): + - Cmd+click `#1412` → in-editor issue preview (default `editor`) + - Cmd+click `PR #1405` → browser opens `.../pull/1405` + - Cmd+click a bare number that is actually a PR (e.g. `#1405`) → browser opens the PR page (fallthrough) + - Set `codev.terminalLinks.issueTarget: browser`, Cmd+click `#1412` → browser opens the issue + - Cmd+click a nonexistent number → warning toast (no silent fallthrough to workspace search) + - A line with two refs (e.g. `see #12 and PR #34`) → both are individually clickable + - Each click shows a "Codev: Opening #N…" status-bar spinner immediately From 9a6f90c0f46f1aedbe012a5e021235585861ea59 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:18:53 +1000 Subject: [PATCH 15/21] chore(porch): 1412 record PR #1418 --- .../1412-vscode-terminallinkprovider-ma/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 85c391d99..eedff54c4 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -19,4 +19,9 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T04:16:51.831Z' +updated_at: '2026-08-12T04:18:53.021Z' +pr_history: + - phase: review + pr_number: 1418 + branch: builder/pir-1412 + created_at: '2026-08-12T04:18:53.020Z' From 7f236c0b92c1c648eca9a8f8984817734649c31c Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:19:04 +1000 Subject: [PATCH 16/21] chore(porch): 1412 review build-complete --- .../projects/1412-vscode-terminallinkprovider-ma/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index eedff54c4..67a69daef 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -16,10 +16,10 @@ gates: pr: status: pending iteration: 1 -build_complete: false +build_complete: true history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T04:18:53.021Z' +updated_at: '2026-08-12T04:19:04.746Z' pr_history: - phase: review pr_number: 1418 From 3b511bb4d9c9a0defc55cb22d1a61809067a1406 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:22:30 +1000 Subject: [PATCH 17/21] [PIR #1412] Docs: amend plan for adopted latency approach; fix stale test header Consultation (all APPROVE) flagged two stale-text nits: the plan still labeled the shipped openExternal approach 'rejected', and the test header claimed three helper modules are stubbed. Text-only. --- .../src/__tests__/terminal-ref-link-provider.test.ts | 6 ++++-- codev/plans/1412-vscode-terminallinkprovider-ma.md | 7 +++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts b/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts index 94aa474f4..00e8b58d6 100644 --- a/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts +++ b/apps/vscode/src/__tests__/terminal-ref-link-provider.test.ts @@ -5,8 +5,10 @@ * spans, flags, multiple refs per line) and `openTerminalRef` (resolution — * issue-first with the url-based PR discriminator, and the `issueTarget` * setting). Both import `vscode` at module load, so we stub it (the established - * pattern from open-issue-by-id.test.ts), and stub the three reuse helper - * modules so we can assert delegation without real fetches. + * pattern from open-issue-by-id.test.ts). We also stub `open-pr-by-id.js` — the + * one reuse helper still called directly (for `PR #N` and the PR fallthrough); + * the browser-issue and editor paths are asserted via the stubbed + * `vscode.env.openExternal` / `vscode.commands.executeCommand`. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; diff --git a/codev/plans/1412-vscode-terminallinkprovider-ma.md b/codev/plans/1412-vscode-terminallinkprovider-ma.md index 32d59fa7e..a1fa6b98d 100644 --- a/codev/plans/1412-vscode-terminallinkprovider-ma.md +++ b/codev/plans/1412-vscode-terminallinkprovider-ma.md @@ -175,6 +175,12 @@ in-editor PR preview surface. ## Risks & Alternatives Considered +> **Amendment (dev-approval, 2026-08-12):** the "double fetch, accepted" call below was **reversed** +> after the human tested it — a bare `#N` felt unresponsive (~2s, no feedback). The shipped code takes +> the second alternative ("rejected" here): it opens `issue.url` directly via `openExternal` for the +> PR-fallthrough and browser-issue paths (single round-trip), and adds a `withProgress` spinner. See the +> review file's "Things to Look At". The text below is preserved as the original plan-time reasoning. + - **Double fetch on a bare `#N` click** (discriminator `getIssue`, then the helper re-fetches): accepted. It is one extra round-trip on a single deliberate click; funneling every open through the one sanctioned helper (single owner per destination) is worth more than saving it. @@ -182,6 +188,7 @@ in-editor PR preview surface. to skip the re-fetch — expands their signatures for a non-user-visible micro-opt; against "lean." - *Alternative (rejected):* `openExternal(issue.url)` directly for the discriminated-PR case (saves the `getPR`) — creates a second browser-open path; consistency with the explicit `PR #N` branch wins. + **(Adopted after all — see amendment above.)** - **Over-claiming spans** (e.g. `#2` in "step #2"): accepted. Cmd+click is opt-in; a stray click yields a warning toast at worst. Hex colors and spaced `# 1` headings already don't match. - **Non-GitHub forge with no `url`**: discriminator can't tell issue from PR; degrades to the issue From 4c920670f11fda6d949b16560544775d02cbf8ad Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:22:36 +1000 Subject: [PATCH 18/21] chore(porch): 1412 pr gate-requested --- .../1412-vscode-terminallinkprovider-ma/status.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 67a69daef..85051e25e 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -15,13 +15,15 @@ gates: approved_at: '2026-08-12T04:16:45.041Z' pr: status: pending + requested_at: '2026-08-12T04:22:36.347Z' iteration: 1 -build_complete: true +build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T04:19:04.746Z' +updated_at: '2026-08-12T04:22:36.348Z' pr_history: - phase: review pr_number: 1418 branch: builder/pir-1412 created_at: '2026-08-12T04:18:53.020Z' +pr_ready_for_human: true From 572b8e72311f3c24721ca00c215a656e4b9b6925 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:22:58 +1000 Subject: [PATCH 19/21] [PIR #1412] Update builder thread: consultation all-clear, pr gate pending --- codev/state/pir-1412_thread.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/codev/state/pir-1412_thread.md b/codev/state/pir-1412_thread.md index 05ad8fce2..c3609a8cb 100644 --- a/codev/state/pir-1412_thread.md +++ b/codev/state/pir-1412_thread.md @@ -55,3 +55,26 @@ reuse helper re-fetching). Fix: - Tradeoff: PR can now be browser-opened by two paths (explicit vs fallthrough); still no new fetch code (openExternal opens a url already fetched via the sanctioned getIssue). Flagged to reviewer. Green: check-types + lint clean, 807 unit tests. + +### dev-approval feedback #2 — setting wording (2026-08-12) + +Reviewer flagged "bare #N" jargon in the `issueTarget` setting description (odd to end users). Reworded to +"issue references (#N)" / "Pull-request references (`PR #N`)"; dropped "bare" entirely. Committed ee6dea3f1. + +## Review phase (2026-08-12) + +dev-approval approved. Wrote `codev/reviews/1412-vscode-terminallinkprovider-ma.md`; added one COLD lesson +to lessons-learned.md (gh issue view resolves PR numbers → url-path discriminator). No arch changes +(self-contained provider + resolution module, reuses existing forge paths). PR #1418 opened, recorded with +porch. `porch done 1412` running the single 3-way consultation pass (bg). Awaiting verdicts → pr gate. + +### 3-way consultation (2026-08-12): ALL APPROVE + +gemini=APPROVE, codex=APPROVE, claude=APPROVE (all HIGH). No blocking issues. Claude flagged 3 minor +non-blocking nits; fixed 2 stale-text ones (plan "rejected" note now amended to reflect adopted +openExternal approach; test header comment corrected) in 3b511bb4d. Left the 3rd (no try/catch on +getIssue) — matches existing openIssueInBrowser behavior, out of scope. + +**pr gate now pending.** Notified architect (all-clear). Waiting for porch "Gate pr approved" wake-up — +NOT merging on pane prose. On approval: verify gate_status via `porch next`, check if human already +merged #1418, then `gh pr merge 1418 --merge` (never squash) + `porch done 1412 --merged 1418`. From b55ead745a16e5b0370169d815e7faaf1efa1859 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:47:53 +1000 Subject: [PATCH 20/21] chore(porch): 1412 pr gate-approved --- .../1412-vscode-terminallinkprovider-ma/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 85051e25e..2f95c57e0 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -14,16 +14,17 @@ gates: requested_at: '2026-08-12T02:01:06.883Z' approved_at: '2026-08-12T04:16:45.041Z' pr: - status: pending + status: approved requested_at: '2026-08-12T04:22:36.347Z' + approved_at: '2026-08-12T04:47:53.113Z' iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T04:22:36.348Z' +updated_at: '2026-08-12T04:47:53.114Z' pr_history: - phase: review pr_number: 1418 branch: builder/pir-1412 created_at: '2026-08-12T04:18:53.020Z' -pr_ready_for_human: true +pr_ready_for_human: false From a33084874ec0944b527a1a8478085b05d4ab8b21 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 12 Aug 2026 14:48:04 +1000 Subject: [PATCH 21/21] chore(porch): 1412 protocol complete --- .../projects/1412-vscode-terminallinkprovider-ma/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml index 2f95c57e0..c00bf16a0 100644 --- a/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml +++ b/codev/projects/1412-vscode-terminallinkprovider-ma/status.yaml @@ -1,7 +1,7 @@ id: '1412' title: vscode-terminallinkprovider-ma protocol: pir -phase: review +phase: verified plan_phases: [] current_plan_phase: null gates: @@ -21,7 +21,7 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-11T23:35:58.126Z' -updated_at: '2026-08-12T04:47:53.114Z' +updated_at: '2026-08-12T04:48:04.004Z' pr_history: - phase: review pr_number: 1418