Skip to content

Commit 7b7b3e9

Browse files
committed
feat(vscode): make Builders and Needs Attention rows clickable
Clicking a row in the Builders sidebar (or a blocked-builder row in Needs Attention) now opens that builder's terminal in the editor — previously these rows were decorative because no `command` was wired. Lifts the role-id-to-terminalId resolution out of BuilderTerminalLinkProvider into TerminalManager.openBuilderByRoleOrId so the sidebar tree views, terminal link provider, and a new `codev.openBuilderById` command all share one path. The link provider loses its now-unused ConnectionManager and OutputChannel deps.
1 parent e0281ca commit 7b7b3e9

6 files changed

Lines changed: 56 additions & 71 deletions

File tree

packages/vscode/src/extension.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ export async function activate(context: vscode.ExtensionContext) {
176176
vscode.window.showErrorMessage('Codev: Failed to create shell');
177177
}
178178
}),
179+
vscode.commands.registerCommand('codev.openBuilderById', async (roleOrId: string) => {
180+
if (!roleOrId) { return; }
181+
await terminalManager?.openBuilderByRoleOrId(roleOrId);
182+
}),
179183
vscode.commands.registerCommand('codev.spawnBuilder', () => spawnBuilder()),
180184
vscode.commands.registerCommand('codev.sendMessage', () => sendMessage(connectionManager!)),
181185
vscode.commands.registerCommand('codev.approveGate', () => approveGate(connectionManager!)),
@@ -200,7 +204,7 @@ export async function activate(context: vscode.ExtensionContext) {
200204
// Make builder names clickable in any terminal output
201205
context.subscriptions.push(
202206
vscode.window.registerTerminalLinkProvider(
203-
new BuilderTerminalLinkProvider(connectionManager, terminalManager, outputChannel),
207+
new BuilderTerminalLinkProvider(terminalManager),
204208
),
205209
);
206210

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import * as vscode from 'vscode';
2-
import type { ConnectionManager } from './connection-manager.js';
32
import type { TerminalManager } from './terminal-manager.js';
43

54
// Matches Codev builder role names like `builder-spir-153`, `builder-bugfix-42`.
@@ -14,11 +13,7 @@ interface BuilderLink extends vscode.TerminalLink {
1413
* Clicking opens (or focuses) that builder's terminal.
1514
*/
1615
export class BuilderTerminalLinkProvider implements vscode.TerminalLinkProvider<BuilderLink> {
17-
constructor(
18-
private connectionManager: ConnectionManager,
19-
private terminalManager: TerminalManager,
20-
private outputChannel: vscode.OutputChannel,
21-
) {}
16+
constructor(private terminalManager: TerminalManager) {}
2217

2318
provideTerminalLinks(context: vscode.TerminalLinkContext): BuilderLink[] {
2419
const links: BuilderLink[] = [];
@@ -36,29 +31,6 @@ export class BuilderTerminalLinkProvider implements vscode.TerminalLinkProvider<
3631
}
3732

3833
async handleTerminalLink(link: BuilderLink): Promise<void> {
39-
const client = this.connectionManager.getClient();
40-
const workspacePath = this.connectionManager.getWorkspacePath();
41-
if (!client || !workspacePath) {
42-
vscode.window.showErrorMessage('Codev: Not connected to Tower');
43-
return;
44-
}
45-
46-
try {
47-
const state = await client.getWorkspaceState(workspacePath);
48-
const builder = state?.builders?.find(b => b.name === link.roleId || b.id === link.roleId);
49-
if (!builder?.terminalId) {
50-
vscode.window.showWarningMessage(`Codev: No active terminal for ${link.roleId}`);
51-
return;
52-
}
53-
await this.terminalManager.openBuilder(builder.terminalId, builder.id, `Codev: ${builder.name}`);
54-
} catch (err) {
55-
this.log('ERROR', `Failed to open builder ${link.roleId}: ${(err as Error).message}`);
56-
vscode.window.showErrorMessage(`Codev: Failed to open ${link.roleId}`);
57-
}
58-
}
59-
60-
private log(level: string, message: string): void {
61-
const timestamp = new Date().toISOString();
62-
this.outputChannel.appendLine(`[${timestamp}] [BuilderLinks] [${level}] ${message}`);
34+
await this.terminalManager.openBuilderByRoleOrId(link.roleId);
6335
}
6436
}

packages/vscode/src/terminal-manager.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,32 @@ export class TerminalManager {
5959
await this.openTerminal(terminalId, 'builder', label, key);
6060
}
6161

62+
/**
63+
* Resolve a builder by `roleId` or `id` via Tower workspace state, then
64+
* open its terminal. Used by the sidebar tree views, terminal link
65+
* provider, and command palette so the lookup logic lives in one place.
66+
*/
67+
async openBuilderByRoleOrId(roleOrId: string): Promise<void> {
68+
const client = this.connectionManager.getClient();
69+
const workspacePath = this.connectionManager.getWorkspacePath();
70+
if (!client || !workspacePath) {
71+
vscode.window.showErrorMessage('Codev: Not connected to Tower');
72+
return;
73+
}
74+
try {
75+
const state = await client.getWorkspaceState(workspacePath);
76+
const builder = state?.builders?.find((b) => b.name === roleOrId || b.id === roleOrId);
77+
if (!builder?.terminalId) {
78+
vscode.window.showWarningMessage(`Codev: No active terminal for ${roleOrId}`);
79+
return;
80+
}
81+
await this.openBuilder(builder.terminalId, builder.id, `Codev: ${builder.name}`);
82+
} catch (err) {
83+
this.log('ERROR', `Failed to open builder ${roleOrId}: ${(err as Error).message}`);
84+
vscode.window.showErrorMessage(`Codev: Failed to open ${roleOrId}`);
85+
}
86+
}
87+
6288
/**
6389
* Open a shell terminal.
6490
*/

packages/vscode/src/test/terminal-link-provider.test.ts

Lines changed: 13 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,29 @@
11
import * as assert from 'assert';
22
import * as vscode from 'vscode';
33
import { BuilderTerminalLinkProvider } from '../terminal-link-provider.js';
4-
import type { ConnectionManager } from '../connection-manager.js';
54
import type { TerminalManager } from '../terminal-manager.js';
65

7-
const fakeOutputChannel = (): vscode.OutputChannel => ({
8-
name: 'test',
9-
append: () => {},
10-
appendLine: () => {},
11-
clear: () => {},
12-
show: () => {},
13-
hide: () => {},
14-
dispose: () => {},
15-
replace: () => {},
16-
});
17-
186
function fakeContext(line: string): vscode.TerminalLinkContext {
197
return { line, terminal: undefined as unknown as vscode.Terminal };
208
}
219

10+
// A TerminalManager that throws if any method is called from render.
11+
const guardedTerminalManager = {
12+
openBuilderByRoleOrId: () => { throw new Error('openBuilderByRoleOrId called from provideTerminalLinks'); },
13+
openBuilder: () => { throw new Error('openBuilder called from provideTerminalLinks'); },
14+
} as unknown as TerminalManager;
15+
2216
suite('BuilderTerminalLinkProvider.provideTerminalLinks', () => {
2317
test('matches a single builder role name', () => {
24-
const provider = new BuilderTerminalLinkProvider(
25-
{} as ConnectionManager,
26-
{} as TerminalManager,
27-
fakeOutputChannel(),
28-
);
18+
const provider = new BuilderTerminalLinkProvider(guardedTerminalManager);
2919
const links = provider.provideTerminalLinks(fakeContext('starting builder-spir-153 now'));
3020
assert.strictEqual(links.length, 1);
3121
assert.strictEqual(links[0].roleId, 'builder-spir-153');
3222
assert.strictEqual(links[0].length, 'builder-spir-153'.length);
3323
});
3424

3525
test('matches multiple builder role names on one line', () => {
36-
const provider = new BuilderTerminalLinkProvider(
37-
{} as ConnectionManager,
38-
{} as TerminalManager,
39-
fakeOutputChannel(),
40-
);
26+
const provider = new BuilderTerminalLinkProvider(guardedTerminalManager);
4127
const links = provider.provideTerminalLinks(
4228
fakeContext('builders: builder-spir-153, builder-bugfix-42, builder-air-7'),
4329
);
@@ -48,36 +34,23 @@ suite('BuilderTerminalLinkProvider.provideTerminalLinks', () => {
4834
});
4935

5036
test('does not match non-builder text', () => {
51-
const provider = new BuilderTerminalLinkProvider(
52-
{} as ConnectionManager,
53-
{} as TerminalManager,
54-
fakeOutputChannel(),
55-
);
37+
const provider = new BuilderTerminalLinkProvider(guardedTerminalManager);
5638
assert.strictEqual(provider.provideTerminalLinks(fakeContext('no role here')).length, 0);
5739
assert.strictEqual(provider.provideTerminalLinks(fakeContext('foo-bar-baz')).length, 0);
5840
});
5941

60-
test('does not call into ConnectionManager during render', () => {
42+
test('does not call into TerminalManager during render', () => {
6143
// Regression pin for PR #682 review #2: provideTerminalLinks fires per
6244
// terminal paint, so it must stay pure. Tower lookup belongs in
63-
// handleTerminalLink (click handler).
64-
const guarded = {
65-
getClient: () => { throw new Error('getClient called from provideTerminalLinks'); },
66-
getWorkspacePath: () => { throw new Error('getWorkspacePath called from provideTerminalLinks'); },
67-
} as unknown as ConnectionManager;
68-
const provider = new BuilderTerminalLinkProvider(guarded, {} as TerminalManager, fakeOutputChannel());
69-
45+
// handleTerminalLink (click handler) via openBuilderByRoleOrId.
46+
const provider = new BuilderTerminalLinkProvider(guardedTerminalManager);
7047
assert.doesNotThrow(() =>
7148
provider.provideTerminalLinks(fakeContext('builder-spir-1 builder-spir-2 builder-spir-3')),
7249
);
7350
});
7451

7552
test('regex re-resets between calls (statefulness check)', () => {
76-
const provider = new BuilderTerminalLinkProvider(
77-
{} as ConnectionManager,
78-
{} as TerminalManager,
79-
fakeOutputChannel(),
80-
);
53+
const provider = new BuilderTerminalLinkProvider(guardedTerminalManager);
8154
// Module-level regex with /g flag carries lastIndex; provider must reset.
8255
const a = provider.provideTerminalLinks(fakeContext('builder-spir-1'));
8356
const b = provider.provideTerminalLinks(fakeContext('builder-spir-2'));

packages/vscode/src/views/builders.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ export class BuildersProvider implements vscode.TreeDataProvider<vscode.TreeItem
2525
item.iconPath = b.blocked
2626
? new vscode.ThemeIcon('debug-pause', new vscode.ThemeColor('testing.iconFailed'))
2727
: new vscode.ThemeIcon('play', new vscode.ThemeColor('testing.iconPassed'));
28+
item.command = {
29+
command: 'codev.openBuilderById',
30+
title: 'Open Builder Terminal',
31+
arguments: [b.id],
32+
};
2833
return item;
2934
});
3035
}

packages/vscode/src/views/needs-attention.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ export class NeedsAttentionProvider implements vscode.TreeDataProvider<vscode.Tr
2727
const item = new vscode.TreeItem(`#${b.issueId ?? b.id} — blocked on ${b.blocked} ${waitTime}`);
2828
item.iconPath = new vscode.ThemeIcon('bell', new vscode.ThemeColor('notificationsWarningIcon.foreground'));
2929
item.contextValue = 'blocked-builder';
30+
item.command = {
31+
command: 'codev.openBuilderById',
32+
title: 'Open Builder Terminal',
33+
arguments: [b.id],
34+
};
3035
items.push(item);
3136
}
3237

0 commit comments

Comments
 (0)