Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/tutorial/03-vscode-planning.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ Click it to run `lol impl <issue-number>`.
The implementation output appears in a separate **Implementation Log** panel, and the
button is disabled while the implementation run is active.

Both logs render canonical GitHub issue and PR URLs as clickable links.

## Next Steps

- [Tutorial 02: Issue to Implementation](./02-issue-to-impl.md) for the CLI flow
Expand Down
4 changes: 4 additions & 0 deletions docs/workflows/vscode-extension.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ The extension scans stdout/stderr lines in real time and stores the first matchi
number on the session. Capturing the issue number during execution ensures the UI can
surface the Implement and View PR actions immediately after the plan completes.

Plan and implementation logs also linkify canonical GitHub issue and pull request URLs.
External opening is restricted to canonical `github.com/<owner>/<repo>/issues/<id>` and
`github.com/<owner>/<repo>/pull/<id>` routes.

When a plan emits a local markdown path (for example, `.tmp/issue-928.md`), the UI
surfaces a View Plan button that opens the file inside the workspace.

Expand Down
85 changes: 59 additions & 26 deletions tests/vscode/test-link-detection.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ test_info "Testing link detection patterns"
TEST_SCRIPT=$(make_temp_dir "link-detection-test")/test.js

cat > "$TEST_SCRIPT" << 'NODEJS'
// Regex patterns extracted from vscode/webview/plan/index.ts
// Regex patterns extracted from vscode/webview/plan/utils.ts

// GitHub issue URLs: https://github.com/owner/repo/issues/N
const githubRegex = /https:\/\/github\.com\/([^\/\s]+)\/([^\/\s]+)\/issues\/(\d+)/g;
// Canonical GitHub issue/PR URLs inside log text.
const githubRegex = /https:\/\/github\.com\/[^/\s]+\/[^/\s]+\/(?:issues|pull)\/\d+(?=$|[^\w/?#])/g;

// Local markdown paths: .tmp/issue-N.md or /path/to/file.md
const mdPathRegex = /(?<=\s|^)(\.tmp\/[^\s\n]+\.md|[\w\-\/]+\.tmp\/[^\s\n]+\.md)(?=\s|$)/g;

// isValidGitHubUrl from vscode/src/view/unifiedViewProvider.ts
function isValidGitHubUrl(url) {
return /^https:\/\/github\.com\/[^/]+\/[^/]+\/issues\/\d+$/.test(url);
return /^https:\/\/github\.com\/[^/]+\/[^/]+\/(?:issues|pull)\/\d+$/.test(url);
}

// Test cases for GitHub URL detection
Expand All @@ -28,42 +28,69 @@ const githubTestCases = [
name: 'Standard GitHub issue URL',
input: 'Check https://github.com/owner/repo/issues/123 for details',
expectedMatch: ['https://github.com/owner/repo/issues/123'],
expectedValid: true,
},
{
name: 'GitHub URL with numbers in owner name',
input: 'See https://github.com/user123/repo-name/issues/456',
expectedMatch: ['https://github.com/user123/repo-name/issues/456'],
expectedValid: true,
},
{
name: 'Multiple GitHub URLs',
input: 'Issues: https://github.com/a/b/issues/1 and https://github.com/c/d/issues/2',
expectedMatch: ['https://github.com/a/b/issues/1', 'https://github.com/c/d/issues/2'],
expectedValid: true,
},
{
name: 'GitHub PR URL (should not match issue regex)',
name: 'GitHub PR URL',
input: 'PR at https://github.com/owner/repo/pull/123',
expectedMatch: [],
expectedValid: false,
expectedMatch: ['https://github.com/owner/repo/pull/123'],
},
{
name: 'GitHub non-issue URL',
input: 'Visit https://github.com/owner/repo',
expectedMatch: [],
expectedValid: false,
},
{
name: 'Non-GitHub URL',
input: 'Visit https://example.com/some/path',
expectedMatch: [],
expectedValid: false,
},
{
name: 'Invalid GitHub URL format',
input: 'Check https://github.com/owner/issues/123 (no repo)',
expectedMatch: [],
},
{
name: 'GitHub PR URL with extra route segment',
input: 'See https://github.com/owner/repo/pull/123/files',
expectedMatch: [],
},
];

// Test cases for host URL allowlist validation
const githubValidationCases = [
{
name: 'Canonical issue URL is valid',
url: 'https://github.com/owner/repo/issues/123',
expectedValid: true,
},
{
name: 'Canonical PR URL is valid',
url: 'https://github.com/owner/repo/pull/456',
expectedValid: true,
},
{
name: 'Unsupported GitHub route is invalid',
url: 'https://github.com/owner/repo/actions/runs/1',
expectedValid: false,
},
{
name: 'PR URL with extra segment is invalid',
url: 'https://github.com/owner/repo/pull/123/files',
expectedValid: false,
},
{
name: 'Non-GitHub URL is invalid',
url: 'https://example.com/owner/repo/issues/1',
expectedValid: false,
},
];
Expand Down Expand Up @@ -114,25 +141,31 @@ let failed = 0;
console.log('Testing GitHub URL detection:\n');
githubTestCases.forEach(tc => {
const matches = [...tc.input.matchAll(githubRegex)].map(m => m[0]);
const isValid = tc.expectedMatch.length > 0 ? isValidGitHubUrl(tc.expectedMatch[0]) : false;

let matchOk = JSON.stringify(matches) === JSON.stringify(tc.expectedMatch);
let validOk = isValid === tc.expectedValid;

if (matchOk && validOk) {

if (JSON.stringify(matches) === JSON.stringify(tc.expectedMatch)) {
console.log(`✓ PASS: ${tc.name}`);
passed++;
} else {
console.log(`✗ FAIL: ${tc.name}`);
console.log(` Input: ${tc.input}`);
if (!matchOk) {
console.log(` Expected matches: ${JSON.stringify(tc.expectedMatch)}`);
console.log(` Got matches: ${JSON.stringify(matches)}`);
}
if (!validOk) {
console.log(` Expected valid: ${tc.expectedValid}`);
console.log(` Got valid: ${isValid}`);
}
console.log(` Expected matches: ${JSON.stringify(tc.expectedMatch)}`);
console.log(` Got matches: ${JSON.stringify(matches)}`);
failed++;
}
});

// Test URL allowlist validation
console.log('\nTesting GitHub URL allowlist validation:\n');
githubValidationCases.forEach(tc => {
const actual = isValidGitHubUrl(tc.url);
if (actual === tc.expectedValid) {
console.log(`✓ PASS: ${tc.name}`);
passed++;
} else {
console.log(`✗ FAIL: ${tc.name}`);
console.log(` URL: ${tc.url}`);
console.log(` Expected valid: ${tc.expectedValid}`);
console.log(` Got valid: ${actual}`);
failed++;
}
});
Expand Down
7 changes: 5 additions & 2 deletions vscode/src/view/unifiedViewProvider.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Consumes UI messages:
- `plan/view-plan`
- `plan/view-issue`
- `plan/view-pr`
- `link/openExternal` (GitHub issue URLs)
- `link/openExternal` (canonical GitHub issue/PR URLs)
- `link/openFile` (local markdown paths)

Emits UI messages:
Expand Down Expand Up @@ -111,7 +111,10 @@ Resolves the working directory for Plan/Implementation runs by preferring the
`trees/main` layout and falling back to an Agentize worktree root when needed.

### Link Handling
- `isValidGitHubUrl(url: string)`: validates GitHub issue URLs.
- `isValidGitHubUrl(url: string)`: validates canonical GitHub issue/PR URLs
(`/issues/<id>` or `/pull/<id>`).
- `openExternalGitHubUrl(url: string)`: central allowlist gate for external URL open
paths (`plan/view-issue`, `plan/view-pr`, and `link/openExternal`).
- `openLocalFile(filePath: string, options?)`: resolves relative paths from workspace
roots, expands `~/...`, optionally creates missing files, and opens in the current
editor group as a non-preview tab.
Expand Down
22 changes: 13 additions & 9 deletions vscode/src/view/unifiedViewProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ export class UnifiedViewProvider implements vscode.WebviewViewProvider {
return;
}
const issueUrl = await this.resolveIssueUrl(issueNumber);
if (issueUrl && this.isValidGitHubUrl(issueUrl)) {
void vscode.env.openExternal(vscode.Uri.parse(issueUrl));
if (issueUrl) {
this.openExternalGitHubUrl(issueUrl);
}
return;
}
Expand All @@ -476,7 +476,7 @@ export class UnifiedViewProvider implements vscode.WebviewViewProvider {
if (!session || !session.prUrl) {
return;
}
void vscode.env.openExternal(vscode.Uri.parse(session.prUrl));
this.openExternalGitHubUrl(session.prUrl);
return;
}
case 'settings/load': {
Expand All @@ -488,10 +488,7 @@ export class UnifiedViewProvider implements vscode.WebviewViewProvider {
return;
}
case 'link/openExternal': {
const url = message.url ?? '';
if (this.isValidGitHubUrl(url)) {
void vscode.env.openExternal(vscode.Uri.parse(url));
}
this.openExternalGitHubUrl(message.url ?? '');
return;
}
case 'link/openFile': {
Expand All @@ -508,9 +505,16 @@ export class UnifiedViewProvider implements vscode.WebviewViewProvider {
}
}

private openExternalGitHubUrl(url: string): void {
if (!this.isValidGitHubUrl(url)) {
return;
}
void vscode.env.openExternal(vscode.Uri.parse(url));
}

private isValidGitHubUrl(url: string): boolean {
// Validate GitHub issue URLs: https://github.com/owner/repo/issues/N
return /^https:\/\/github\.com\/[^/]+\/[^/]+\/issues\/\d+$/.test(url);
// Allow only canonical GitHub issue/PR URLs.
return /^https:\/\/github\.com\/[^/]+\/[^/]+\/(?:issues|pull)\/\d+$/.test(url);
}

private async openLocalFile(
Expand Down
6 changes: 4 additions & 2 deletions vscode/webview/plan/utils.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ Builds and returns a DOM container with step indicators for the given session ke
Uses `className` when provided; defaults to `step-indicators`.

### renderLinks(text)
Converts GitHub issue URLs and local `.tmp/*.md` paths in a log line into clickable links
for the webview message handlers.
Converts canonical GitHub issue/PR URLs and local `.tmp/*.md` paths in a log line into
clickable links for the webview message handlers. Only canonical routes are linkified:
`https://github.com/<owner>/<repo>/issues/<id>` and
`https://github.com/<owner>/<repo>/pull/<id>`.

### extractIssueNumber(line)
Extracts an issue number from placeholder creation logs or GitHub issue URLs.
Expand Down
2 changes: 1 addition & 1 deletion vscode/webview/plan/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export const renderStepIndicatorsFrom = (
};

export const renderLinks = (text: string): string => {
const githubRegex = /https:\/\/github\.com\/([^\/\s]+)\/([^\/\s]+)\/issues\/(\d+)/g;
const githubRegex = /https:\/\/github\.com\/[^/\s]+\/[^/\s]+\/(?:issues|pull)\/\d+(?=$|[^\w/?#])/g;
const mdPathRegex = /(?<=\s|^)(\.tmp\/[^\s\n]+\.md|[\w\-\/]+\.tmp\/[^\s\n]+\.md)(?=\s|$)/g;

let result = escapeHtml(text);
Expand Down