Skip to content

Commit 604c6cc

Browse files
author
Thibaud
committed
feat: enrich review comments with clickable file:line links
Replace plain `file.py:42` references in POST_COMMENT bodies with clickable GitLab blob links pointing to the exact line at the head SHA. Also includes: - Strip CLAUDECODE env var when spawning Claude child process - Normalize SSH git URLs to HTTPS in config loader
1 parent f6314ab commit 604c6cc

File tree

8 files changed

+183
-7
lines changed

8 files changed

+183
-7
lines changed

src/entities/reviewAction/reviewAction.gateway.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export interface ExecutionContext {
66
mrNumber: number
77
localPath: string
88
diffMetadata?: DiffMetadata
9+
baseUrl?: string
910
}
1011

1112
export interface ExecutionResult {

src/frameworks/claude/claudeInvoker.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,13 @@ export async function invokeClaudeReview(
305305
let stderr = '';
306306
let cancelled = false;
307307

308+
const childEnv = { ...process.env };
309+
// Remove CLAUDECODE to allow spawning Claude from within a Claude session
310+
delete childEnv.CLAUDECODE;
308311
const child = spawn(resolveClaudePath(), args, {
309312
cwd: job.localPath,
310313
env: {
311-
...process.env,
314+
...childEnv,
312315
// Ensure non-interactive mode
313316
TERM: 'dumb',
314317
CI: 'true',

src/frameworks/config/configLoader.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,23 @@ function loadProjectConfig(localPath: string): ProjectConfig | null {
7575
}
7676
}
7777

78+
function normalizeGitUrl(url: string): string {
79+
// Convert SSH URLs (git@host:org/repo.git) to HTTPS (https://host/org/repo)
80+
const sshMatch = url.match(/^git@([^:]+):(.+)$/);
81+
if (sshMatch) {
82+
return `https://${sshMatch[1]}/${sshMatch[2].replace(/\.git$/, '')}`;
83+
}
84+
return url.replace(/\.git$/, '');
85+
}
86+
7887
function getGitRemoteUrl(localPath: string): string | null {
7988
try {
8089
const result = execSync('git remote get-url origin', {
8190
cwd: localPath,
8291
encoding: 'utf-8',
8392
timeout: 5000,
8493
});
85-
return result.trim().replace(/\.git$/, '');
94+
return normalizeGitUrl(result.trim());
8695
} catch {
8796
return null;
8897
}

src/interface-adapters/controllers/webhook/gitlab.controller.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,24 @@ import type { ReviewContextGateway } from '@/entities/reviewContext/reviewContex
3030
import type { ThreadFetchGateway } from '@/entities/threadFetch/threadFetch.gateway.js';
3131
import type { DiffMetadataFetchGateway } from '@/entities/diffMetadata/diffMetadata.gateway.js';
3232

33+
function extractBaseUrl(remoteUrl: string): string | undefined {
34+
try {
35+
// Handle HTTPS URLs: https://gitlab.example.com/group/project.git
36+
if (remoteUrl.startsWith('http')) {
37+
const url = new URL(remoteUrl)
38+
return `${url.protocol}//${url.host}`
39+
}
40+
// Handle SSH URLs: git@gitlab.example.com:group/project.git
41+
const sshMatch = remoteUrl.match(/@([^:]+):/)
42+
if (sshMatch) {
43+
return `https://${sshMatch[1]}`
44+
}
45+
} catch {
46+
// Invalid URL — return undefined
47+
}
48+
return undefined
49+
}
50+
3351
export interface GitLabWebhookDependencies {
3452
reviewContextGateway: ReviewContextGateway;
3553
threadFetchGateway: ThreadFetchGateway;
@@ -299,11 +317,13 @@ export async function handleGitLabWebhook(
299317
const reviewContext = contextGateway.read(j.localPath, mergeRequestId);
300318
if (reviewContext && reviewContext.actions.length > 0) {
301319
threadResolveCount = reviewContext.actions.filter(a => a.type === 'THREAD_RESOLVE').length;
320+
const followupBaseUrl = extractBaseUrl(updateRepoConfig.remoteUrl);
302321
const contextActionResult = await executeActionsFromContext(
303322
reviewContext,
304323
j.localPath,
305324
logger,
306-
defaultCommandExecutor
325+
defaultCommandExecutor,
326+
followupBaseUrl,
307327
);
308328
logger.info(
309329
{ ...contextActionResult, threadResolveCount, mrNumber: j.mrNumber },
@@ -533,11 +553,13 @@ export async function handleGitLabWebhook(
533553
// PRIMARY: Execute actions from context file (agent writes actions here)
534554
const reviewContext = contextGateway.read(j.localPath, mergeRequestId);
535555
if (reviewContext && reviewContext.actions.length > 0) {
556+
const reviewBaseUrl = extractBaseUrl(repoConfig.remoteUrl);
536557
const contextActionResult = await executeActionsFromContext(
537558
reviewContext,
538559
j.localPath,
539560
logger,
540-
defaultCommandExecutor
561+
defaultCommandExecutor,
562+
reviewBaseUrl,
541563
);
542564
logger.info(
543565
{ ...contextActionResult, mrNumber: j.mrNumber },

src/interface-adapters/gateways/cli/reviewAction.gitlab.cli.gateway.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ReviewAction } from '../../../entities/reviewAction/reviewAction.js'
22
import type { ReviewActionGateway, ExecutionContext } from '../../../entities/reviewAction/reviewAction.gateway.js'
33
import { ExecutionGatewayBase, type CommandInfo } from '../../../shared/foundation/executionGateway.base.js'
4+
import { enrichCommentWithLinks } from '../../../services/commentLinkEnricher.js'
45

56
export class GitLabReviewActionCliGateway
67
extends ExecutionGatewayBase<ReviewAction, ExecutionContext>
@@ -17,11 +18,15 @@ export class GitLabReviewActionCliGateway
1718
args: ['api', '--method', 'PUT', `${baseUrl}/discussions/${action.threadId}`, '--field', 'resolved=true'],
1819
}
1920

20-
case 'POST_COMMENT':
21+
case 'POST_COMMENT': {
22+
const enrichedBody = context.baseUrl && context.diffMetadata
23+
? enrichCommentWithLinks(action.body, context.baseUrl, context.projectPath, context.diffMetadata.headSha)
24+
: action.body
2125
return {
2226
command: 'glab',
23-
args: ['api', '--method', 'POST', `${baseUrl}/notes`, '--field', `body=${action.body}`],
27+
args: ['api', '--method', 'POST', `${baseUrl}/notes`, '--field', `body=${enrichedBody}`],
2428
}
29+
}
2530

2631
case 'THREAD_REPLY':
2732
return {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Replace file:line references in a comment body with clickable GitLab/GitHub blob links.
3+
*
4+
* Pattern matches references like:
5+
* file.py:42
6+
* `file.py:42`
7+
* (file.py:42)
8+
* users/api.py:247
9+
*
10+
* Produces markdown links:
11+
* [`file.py:42`](baseUrl/project/-/blob/sha/file.py#L42)
12+
*
13+
* Does NOT match:
14+
* - URLs (http://localhost:8000, https://example.com:443)
15+
* - Bare filenames without line numbers (docker-compose.yml)
16+
*/
17+
18+
// File path must start with a word char, contain at least one dot (extension), then :lineNumber.
19+
// The path must NOT start with // (to exclude URLs).
20+
const FILE_LINE_PATTERN =
21+
/(?<prefix>[`(]?)(?<filePath>[\w][\w./-]*\.[\w]+):(?<line>\d+)(?<suffix>[`)]?)/g
22+
23+
export function enrichCommentWithLinks(
24+
body: string,
25+
baseUrl: string,
26+
projectPath: string,
27+
headSha: string,
28+
): string {
29+
return body.replace(FILE_LINE_PATTERN, (match, prefix, filePath, line, suffix, offset) => {
30+
// Skip if preceded by :// (URL pattern like https://example.com:443)
31+
const beforeMatch = body.slice(Math.max(0, offset - 10), offset)
32+
if (/:\/{1,2}$/.test(beforeMatch) || /:\/{1,2}[\w.-]*$/.test(beforeMatch)) {
33+
return match
34+
}
35+
36+
// Skip if the filePath portion doesn't look like a real file (must contain a dot for extension)
37+
// Already handled by regex, but double check for edge cases
38+
if (!filePath.includes('.')) {
39+
return match
40+
}
41+
42+
const blobUrl = `${baseUrl}/${projectPath}/-/blob/${headSha}/${filePath}#L${line}`
43+
// When wrapped in backticks, the link markdown already includes backticks — don't double them
44+
const outerPrefix = prefix === '`' ? '' : prefix
45+
const outerSuffix = suffix === '`' ? '' : suffix
46+
return `${outerPrefix}[\`${filePath}:${line}\`](${blobUrl})${outerSuffix}`
47+
})
48+
}

src/services/contextActionsExecutor.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@ export async function executeActionsFromContext(
2525
context: ReviewContext,
2626
localPath: string,
2727
_logger: Logger,
28-
executor: CommandExecutor
28+
executor: CommandExecutor,
29+
baseUrl?: string,
2930
): Promise<ExecutionResult> {
3031
const gatewayContext = {
3132
projectPath: context.projectPath,
3233
mrNumber: context.mergeRequestNumber,
3334
localPath,
3435
diffMetadata: context.diffMetadata,
36+
baseUrl,
3537
}
3638

3739
const gateway =
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { describe, it, expect } from 'vitest'
2+
import { enrichCommentWithLinks } from '../../../services/commentLinkEnricher.js'
3+
4+
const BASE_URL = 'https://gitlab.example.com'
5+
const PROJECT_PATH = 'my-org/my-project'
6+
const HEAD_SHA = 'abc123def456'
7+
8+
function enrich(body: string): string {
9+
return enrichCommentWithLinks(body, BASE_URL, PROJECT_PATH, HEAD_SHA)
10+
}
11+
12+
describe('enrichCommentWithLinks', () => {
13+
it('should replace a simple file:line reference with a link', () => {
14+
const result = enrich('See file.py:42 for details')
15+
expect(result).toBe(
16+
'See [`file.py:42`](https://gitlab.example.com/my-org/my-project/-/blob/abc123def456/file.py#L42) for details'
17+
)
18+
})
19+
20+
it('should replace a backtick-wrapped file:line reference', () => {
21+
const result = enrich('Check `file.py:42` for the issue')
22+
expect(result).toBe(
23+
'Check [`file.py:42`](https://gitlab.example.com/my-org/my-project/-/blob/abc123def456/file.py#L42) for the issue'
24+
)
25+
})
26+
27+
it('should replace a parenthesized file:line reference', () => {
28+
const result = enrich('See issue (file.py:42)')
29+
expect(result).toBe(
30+
'See issue ([`file.py:42`](https://gitlab.example.com/my-org/my-project/-/blob/abc123def456/file.py#L42))'
31+
)
32+
})
33+
34+
it('should handle nested path references', () => {
35+
const result = enrich('Found at users/api.py:247')
36+
expect(result).toBe(
37+
'Found at [`users/api.py:247`](https://gitlab.example.com/my-org/my-project/-/blob/abc123def456/users/api.py#L247)'
38+
)
39+
})
40+
41+
it('should handle deeply nested paths', () => {
42+
const result = enrich('See src/components/posts/post-card.js:15')
43+
expect(result).toBe(
44+
'See [`src/components/posts/post-card.js:15`](https://gitlab.example.com/my-org/my-project/-/blob/abc123def456/src/components/posts/post-card.js#L15)'
45+
)
46+
})
47+
48+
it('should NOT match http:// URLs', () => {
49+
const body = 'Visit http://localhost:8000/api'
50+
expect(enrich(body)).toBe(body)
51+
})
52+
53+
it('should NOT match https:// URLs', () => {
54+
const body = 'See https://example.com:443/path'
55+
expect(enrich(body)).toBe(body)
56+
})
57+
58+
it('should NOT match filenames without line numbers', () => {
59+
const body = 'Edit docker-compose.yml for config'
60+
expect(enrich(body)).toBe(body)
61+
})
62+
63+
it('should handle multiple references in the same body', () => {
64+
const result = enrich('Issues in file.py:10 and users/views.py:25 and lib/utils.py:100')
65+
expect(result).toContain('[`file.py:10`]')
66+
expect(result).toContain('[`users/views.py:25`]')
67+
expect(result).toContain('[`lib/utils.py:100`]')
68+
})
69+
70+
it('should return body unchanged when all parameters are provided but no matches exist', () => {
71+
const body = 'No file references here, just plain text.'
72+
expect(enrich(body)).toBe(body)
73+
})
74+
75+
it('should handle file references at start and end of body', () => {
76+
const result = enrich('file.py:1 is the start and end is file.py:99')
77+
expect(result).toContain('[`file.py:1`]')
78+
expect(result).toContain('[`file.py:99`]')
79+
})
80+
81+
it('should handle references in markdown context', () => {
82+
const result = enrich('- **Blocking**: Missing validation in `users/api.py:247`')
83+
expect(result).toContain('[`users/api.py:247`]')
84+
expect(result).toContain('https://gitlab.example.com/my-org/my-project/-/blob/abc123def456/users/api.py#L247')
85+
})
86+
})

0 commit comments

Comments
 (0)