diff --git a/source/constants.ts b/source/constants.ts index aebbec35b..5b96718cd 100644 --- a/source/constants.ts +++ b/source/constants.ts @@ -19,6 +19,9 @@ export const TIMEOUT_UPDATE_CHECK_MS = 10_000; export const TIMEOUT_SOCKET_DEFAULT_MS = 120_000; export const TIMEOUT_SOCKET_LOCAL_DEFAULT_MS = 600_000; // 10 minutes for local models (Ollama, etc.) export const TIMEOUT_LSP_DIAGNOSTICS_MS = 5000; +// gh run view --log/--log-failed can be slow and the log itself huge; bound +// the fetch so a hung request doesn't hang the tool call indefinitely. +export const TIMEOUT_GH_LOG_MS = 60_000; // Ceiling on the pricing lookup for the per-response usage footer: past // this the message renders with token counts only rather than holding the // streaming-to-static swap hostage to a cold models.dev fetch. diff --git a/source/tools/git/git-pr.spec.tsx b/source/tools/git/git-pr.spec.tsx index 301b011b9..1977582ae 100644 --- a/source/tools/git/git-pr.spec.tsx +++ b/source/tools/git/git-pr.spec.tsx @@ -47,6 +47,14 @@ test('git_pr tool has formatter function', t => { t.is(typeof gitPrTool.formatter, 'function'); }); +test('git_pr tool is marked read-only for parallel-execution batching', t => { + // Safe despite bundling mutating actions (create/comment/review): the + // batching this flag feeds only ever runs on calls resolveToolApproval + // already routed to auto-execute, via the `approval` fn above — see the + // comment on the export in git-pr.tsx. + t.true(gitPrTool.readOnly); +}); + // ============================================================================ // Formatter Tests // ============================================================================ @@ -211,3 +219,156 @@ test('git_pr formatter shows PR body', t => { t.regex(output!, /Body/i); t.regex(output!, /This is the PR description/); }); + +// ============================================================================ +// New Action Formatter Tests (diff/comment/review/checks/logs) +// ============================================================================ + +test('git_pr formatter shows diff action', t => { + const formatter = gitPrTool.formatter; + if (!formatter) { + t.fail('Formatter is not defined'); + return; + } + + const element = formatter({diff: 42}, ''); + const {lastFrame} = render({element}); + + const output = lastFrame(); + t.truthy(output); + t.regex(output!, /diff/i); + t.regex(output!, /#42/); +}); + +test('git_pr formatter shows comment action with body', t => { + const formatter = gitPrTool.formatter; + if (!formatter) { + t.fail('Formatter is not defined'); + return; + } + + const element = formatter( + {comment: {pr: 7, body: 'Looks good to me'}}, + '', + ); + const {lastFrame} = render({element}); + + const output = lastFrame(); + t.truthy(output); + t.regex(output!, /comment/i); + t.regex(output!, /#7/); + t.regex(output!, /Looks good to me/); +}); + +test('git_pr formatter shows review action with verdict', t => { + const formatter = gitPrTool.formatter; + if (!formatter) { + t.fail('Formatter is not defined'); + return; + } + + const element = formatter( + {review: {pr: 9, verdict: 'request-changes', body: 'Needs a test'}}, + '', + ); + const {lastFrame} = render({element}); + + const output = lastFrame(); + t.truthy(output); + t.regex(output!, /review/i); + t.regex(output!, /#9/); + t.regex(output!, /request-changes/); + t.regex(output!, /Needs a test/); +}); + +test('git_pr formatter shows checks action', t => { + const formatter = gitPrTool.formatter; + if (!formatter) { + t.fail('Formatter is not defined'); + return; + } + + const element = formatter({checks: {pr: 3}}, ''); + const {lastFrame} = render({element}); + + const output = lastFrame(); + t.truthy(output); + t.regex(output!, /checks/i); + t.regex(output!, /#3/); +}); + +test('git_pr formatter shows logs action resolved by PR', t => { + const formatter = gitPrTool.formatter; + if (!formatter) { + t.fail('Formatter is not defined'); + return; + } + + const element = formatter({logs: {pr: 5, search: 'Error'}}, ''); + const {lastFrame} = render({element}); + + const output = lastFrame(); + t.truthy(output); + t.regex(output!, /logs/i); + t.regex(output!, /#5/); + t.regex(output!, /Error/); +}); + +test('git_pr formatter shows logs action resolved by explicit run', t => { + const formatter = gitPrTool.formatter; + if (!formatter) { + t.fail('Formatter is not defined'); + return; + } + + const element = formatter({logs: {run: 123456}}, ''); + const {lastFrame} = render({element}); + + const output = lastFrame(); + t.truthy(output); + t.regex(output!, /123456/); +}); + +// ============================================================================ +// Approval Policy Tests +// ============================================================================ + +test('git_pr approval requires confirmation for create', t => { + const approval = gitPrTool.approval; + t.is(typeof approval, 'function'); + if (typeof approval !== 'function') return; + t.true(Boolean(approval({create: {title: 'x'}}, 'normal'))); +}); + +test('git_pr approval requires confirmation for comment', t => { + const approval = gitPrTool.approval; + if (typeof approval !== 'function') { + t.fail('approval is not a function'); + return; + } + t.true(Boolean(approval({comment: {pr: 1, body: 'x'}}, 'normal'))); +}); + +test('git_pr approval requires confirmation for review', t => { + const approval = gitPrTool.approval; + if (typeof approval !== 'function') { + t.fail('approval is not a function'); + return; + } + t.true( + Boolean(approval({review: {pr: 1, verdict: 'approve'}}, 'normal')), + ); +}); + +test('git_pr approval auto-runs read-only actions', t => { + const approval = gitPrTool.approval; + if (typeof approval !== 'function') { + t.fail('approval is not a function'); + return; + } + t.false(Boolean(approval({view: 1}, 'normal'))); + t.false(Boolean(approval({list: {}}, 'normal'))); + t.false(Boolean(approval({diff: 1}, 'normal'))); + t.false(Boolean(approval({checks: {pr: 1}}, 'normal'))); + t.false(Boolean(approval({logs: {pr: 1}}, 'normal'))); +}); diff --git a/source/tools/git/git-pr.tsx b/source/tools/git/git-pr.tsx index e8801e699..0138bec67 100644 --- a/source/tools/git/git-pr.tsx +++ b/source/tools/git/git-pr.tsx @@ -1,16 +1,19 @@ /** * Git PR Tool * - * Pull request management using gh CLI: create, view, list. + * Pull request management using gh CLI: create, view, list, diff, comment, + * review, checks, and CI run log reading. */ import {Box, Text} from 'ink'; import React from 'react'; +import {TIMEOUT_GH_LOG_MS} from '@/constants'; import {useTerminalWidth} from '@/hooks/useTerminalWidth'; import {useTheme} from '@/hooks/useTheme'; import type {NanocoderToolExport} from '@/types/core'; import {jsonSchema, tool} from '@/types/core'; import {formatError} from '@/utils/error-formatter'; +import {queryCiLog} from './log-utils'; import { type CommitInfo, execGh, @@ -18,6 +21,7 @@ import { getCurrentBranch, getDefaultBranch, getUpstreamBranch, + truncateDiff, } from './utils'; // ============================================================================ @@ -37,6 +41,28 @@ interface GitPrInput { author?: string; limit?: number; }; + diff?: number; + comment?: { + pr: number; + body: string; + }; + review?: { + pr: number; + verdict: 'approve' | 'request-changes' | 'comment'; + body?: string; + }; + checks?: { + pr: number; + }; + logs?: { + run?: number; + pr?: number; + branch?: string; + failedOnly?: boolean; + search?: string; + offset?: number; + limit?: number; + }; } // ============================================================================ @@ -55,6 +81,54 @@ async function getCreatePreview( // Execution // ============================================================================ +/** + * Resolve a GitHub Actions run ID for the `logs` action: an explicit `run` + * wins; otherwise resolve a branch (explicit `branch`, else the given PR's + * head branch, else the current branch) and take its most recent run. + */ +async function resolveRunId( + logs: NonNullable, +): Promise { + if (logs.run !== undefined) { + return logs.run.toString(); + } + + let branch = logs.branch; + if (!branch && logs.pr !== undefined) { + const output = await execGh( + ['pr', 'view', logs.pr.toString(), '--json', 'headRefName'], + TIMEOUT_GH_LOG_MS, + ); + branch = JSON.parse(output).headRefName; + } + if (!branch) { + branch = await getCurrentBranch(); + } + + // --status completed: an in-progress/queued run has no (or incomplete) + // failure logs, so only ever resolve to the latest finished run. + const output = await execGh( + [ + 'run', + 'list', + '--branch', + branch, + '--status', + 'completed', + '--limit', + '1', + '--json', + 'databaseId', + ], + TIMEOUT_GH_LOG_MS, + ); + const runs = JSON.parse(output); + if (!runs.length) { + throw new Error(`No completed CI runs found for branch "${branch}".`); + } + return runs[0].databaseId.toString(); +} + const executeGitPr = async (args: GitPrInput): Promise => { try { // CREATE @@ -147,8 +221,125 @@ const executeGitPr = async (args: GitPrInput): Promise => { return lines.join('\n'); } + // DIFF + if (args.diff !== undefined) { + const output = await execGh(['pr', 'diff', args.diff.toString()]); + const {content, truncated, totalLines} = truncateDiff(output); + + const lines: string[] = []; + lines.push(`Diff for PR #${args.diff}:`); + lines.push(''); + lines.push(content); + if (truncated) { + lines.push(''); + lines.push(`(${totalLines} total lines)`); + } + + return lines.join('\n'); + } + + // COMMENT + if (args.comment) { + await execGh([ + 'pr', + 'comment', + args.comment.pr.toString(), + '--body', + args.comment.body, + ]); + return `Comment posted on PR #${args.comment.pr}.`; + } + + // REVIEW + if (args.review) { + const {pr, verdict, body} = args.review; + const ghArgs: string[] = ['pr', 'review', pr.toString()]; + + if (verdict === 'approve') { + ghArgs.push('--approve'); + } else if (verdict === 'request-changes') { + ghArgs.push('--request-changes'); + } else { + ghArgs.push('--comment'); + } + + if (body) { + ghArgs.push('--body', body); + } + + await execGh(ghArgs); + return `Review (${verdict}) submitted on PR #${pr}.`; + } + + // CHECKS + if (args.checks) { + const output = await execGh([ + 'pr', + 'checks', + args.checks.pr.toString(), + '--json', + 'name,bucket,state,workflow', + ]); + const checks = JSON.parse(output); + + if (!Array.isArray(checks) || checks.length === 0) { + return `No checks found for PR #${args.checks.pr}.`; + } + + const lines: string[] = []; + lines.push(`Checks for PR #${args.checks.pr}:`); + lines.push(''); + for (const c of checks) { + const icon = + c.bucket === 'pass' ? '✓' : c.bucket === 'fail' ? '✗' : '…'; + lines.push(`${icon} ${c.name} (${c.state})`); + } + + return lines.join('\n'); + } + + // LOGS + if (args.logs) { + const runId = await resolveRunId(args.logs); + const failedOnly = args.logs.failedOnly !== false; + + const rawLog = await execGh( + ['run', 'view', runId, failedOnly ? '--log-failed' : '--log'], + TIMEOUT_GH_LOG_MS, + ); + + const {content, totalLines, truncated, matchCount} = queryCiLog(rawLog, { + search: args.logs.search, + offset: args.logs.offset, + limit: args.logs.limit, + }); + + const lines: string[] = []; + lines.push( + `Log for run #${runId}${failedOnly ? ' (failed steps only)' : ''}:`, + ); + lines.push( + `Total lines: ${totalLines}` + + (matchCount !== undefined ? `, matches: ${matchCount}` : '') + + (truncated ? ' (truncated)' : ''), + ); + lines.push(''); + lines.push(content || '(empty)'); + + return lines.join('\n'); + } + // LIST - if (args.list || (!args.create && args.view === undefined)) { + if ( + args.list || + (!args.create && + args.view === undefined && + args.diff === undefined && + !args.comment && + !args.review && + !args.checks && + !args.logs) + ) { const state = args.list?.state || 'open'; const limit = args.list?.limit || 10; @@ -188,7 +379,7 @@ const executeGitPr = async (args: GitPrInput): Promise => { return lines.join('\n'); } - return 'Error: No valid action specified. Use create, view, or list.'; + return 'Error: No valid action specified. Use create, view, list, diff, comment, review, checks, or logs.'; } catch (error) { const message = formatError(error); @@ -215,7 +406,7 @@ const executeGitPr = async (args: GitPrInput): Promise => { const gitPrCoreTool = tool({ description: - 'Manage GitHub pull requests. Create new PR, view PR details, or list PRs. Requires gh CLI to be installed and authenticated.', + 'Manage GitHub pull requests: create, view, list, diff, comment, review, check CI status, and read CI run logs. Requires gh CLI to be installed and authenticated.', inputSchema: jsonSchema({ type: 'object', properties: { @@ -265,6 +456,83 @@ const gitPrCoreTool = tool({ }, }, }, + diff: { + type: 'number', + description: 'Show the diff for a specific PR by number', + }, + comment: { + type: 'object', + description: 'Post a comment on a PR', + properties: { + pr: {type: 'number', description: 'PR number'}, + body: {type: 'string', description: 'Comment body'}, + }, + required: ['pr', 'body'], + }, + review: { + type: 'object', + description: 'Submit a review on a PR', + properties: { + pr: {type: 'number', description: 'PR number'}, + verdict: { + type: 'string', + enum: ['approve', 'request-changes', 'comment'], + description: 'Review verdict', + }, + body: { + type: 'string', + description: 'Review body (required by gh for request-changes)', + }, + }, + required: ['pr', 'verdict'], + }, + checks: { + type: 'object', + description: 'List CI check status for a PR', + properties: { + pr: {type: 'number', description: 'PR number'}, + }, + required: ['pr'], + }, + logs: { + type: 'object', + description: + 'Read a CI run log, with search/pagination to avoid returning huge logs. ' + + 'Resolves the run from `run`, else the head branch of `pr`, else `branch`, else the current branch.', + properties: { + run: { + type: 'number', + description: 'Explicit GitHub Actions run ID', + }, + pr: { + type: 'number', + description: "Resolve the latest run from this PR's head branch", + }, + branch: { + type: 'string', + description: 'Resolve the latest run from this branch', + }, + failedOnly: { + type: 'boolean', + description: + 'Only fetch failed-step logs via --log-failed (default: true). Set false for --log (full log).', + }, + search: { + type: 'string', + description: + 'Case-insensitive substring filter; returns matching lines with context instead of the whole log', + }, + offset: { + type: 'number', + description: + 'Line offset for pagination, counted back from the end of the log (ignored when `search` is set)', + }, + limit: { + type: 'number', + description: 'Max lines returned (default 300, hard cap 2000)', + }, + }, + }, }, required: [], }), @@ -297,7 +565,17 @@ function GitPrFormatter({ ? 'create' : args.view !== undefined ? 'view' - : 'list'; + : args.diff !== undefined + ? 'diff' + : args.comment + ? 'comment' + : args.review + ? 'review' + : args.checks + ? 'checks' + : args.logs + ? 'logs' + : 'list'; // Load preview for create before execution React.useEffect(() => { @@ -369,6 +647,91 @@ function GitPrFormatter({ )} + {action === 'diff' && ( + + PR: + #{args.diff} + + )} + + {action === 'comment' && args.comment && ( + <> + + PR: + #{args.comment.pr} + + + Body: + + {args.comment.body} + + + + )} + + {action === 'review' && args.review && ( + <> + + PR: + #{args.review.pr} + + + Verdict: + + {args.review.verdict} + + + {args.review.body && ( + + Body: + + {args.review.body} + + + )} + + )} + + {action === 'checks' && args.checks && ( + + PR: + #{args.checks.pr} + + )} + + {action === 'logs' && args.logs && ( + <> + + Run: + + {args.logs.run !== undefined + ? `#${args.logs.run}` + : args.logs.pr !== undefined + ? `PR #${args.logs.pr}` + : args.logs.branch || 'current branch'} + + + {args.logs.search && ( + + Search: + {args.logs.search} + + )} + {args.logs.offset !== undefined && ( + + Offset: + {args.logs.offset} + + )} + + )} + {action === 'list' && ( State: @@ -388,6 +751,13 @@ function GitPrFormatter({ )} + {(result?.includes('Comment posted') || + result?.includes('submitted on PR')) && ( + + ✓ {result} + + )} + {result?.includes('Error:') && ( ✗ {result} @@ -409,7 +779,15 @@ export const gitPrTool: NanocoderToolExport = { name: 'git_pr' as const, tool: gitPrCoreTool, formatter, - // Approval varies by action: create always prompts (user should see - // title/body); view/list auto-run. - approval: (args: GitPrInput) => Boolean(args.create), + // Safe for the parallel-execution batching in tool-executor.tsx: that + // grouping only ever runs on calls `resolveToolApproval` already routed + // to auto-execute (see the `approval` fn below), so mutating actions + // (create/comment/review) never reach it regardless of this flag — they + // go through the confirmation path one at a time either way. + readOnly: true, + // Approval varies by action: actions that write to GitHub (create, + // comment, review) always prompt; read-only inspection (view, list, + // diff, checks, logs) auto-runs. + approval: (args: GitPrInput) => + Boolean(args.create || args.comment || args.review), }; diff --git a/source/tools/git/log-utils.spec.ts b/source/tools/git/log-utils.spec.ts new file mode 100644 index 000000000..8be311c12 --- /dev/null +++ b/source/tools/git/log-utils.spec.ts @@ -0,0 +1,129 @@ +/** + * CI Log Query Utilities Tests + */ + +import test from 'ava'; +import {queryCiLog} from './log-utils'; + +console.log('\nlog-utils.spec.ts – CI Log Query Utilities'); + +function makeLog(lineCount: number): string { + return Array.from({length: lineCount}, (_, i) => `line ${i + 1}`).join('\n'); +} + +// ============================================================================ +// Pagination (default, no search) +// ============================================================================ + +test('queryCiLog returns the whole log unchanged when under the limit', t => { + const log = makeLog(10); + const result = queryCiLog(log, {limit: 300}); + t.is(result.content, log); + t.is(result.totalLines, 10); + t.false(result.truncated); +}); + +test('queryCiLog handles an empty log', t => { + const result = queryCiLog(''); + t.is(result.content, ''); + t.is(result.totalLines, 0); + t.false(result.truncated); +}); + +test('queryCiLog defaults to the tail of a large log', t => { + const log = makeLog(1000); + const result = queryCiLog(log, {limit: 300}); + t.true(result.truncated); + t.true(result.content.includes('line 1000')); + t.true(result.content.includes('line 701')); + t.false(result.content.includes('line 700')); +}); + +test('queryCiLog honors offset to page further back', t => { + const log = makeLog(1000); + const result = queryCiLog(log, {limit: 300, offset: 300}); + t.true(result.content.includes('line 700')); + t.true(result.content.includes('line 401')); + t.false(result.content.includes('line 701')); + t.false(result.content.includes('line 400')); +}); + +test('queryCiLog offset beyond the start returns the earliest lines only', t => { + const log = makeLog(100); + const result = queryCiLog(log, {limit: 300, offset: 1000}); + t.true(result.content.includes('line 1')); + t.true(result.content.includes('line 100')); +}); + +test('queryCiLog caps limit at the hard maximum', t => { + const log = makeLog(5000); + const result = queryCiLog(log, {limit: 100_000}); + const returnedLines = result.content + .split('\n') + .filter(l => l.startsWith('line ')); + t.true(returnedLines.length <= 2000); +}); + +// ============================================================================ +// Search +// ============================================================================ + +test('queryCiLog search finds a matching line with context', t => { + const lines = ['a', 'b', 'ERROR: boom', 'd', 'e']; + const result = queryCiLog(lines.join('\n'), { + search: 'error', + limit: 300, + contextLines: 1, + }); + t.is(result.matchCount, 1); + t.true(result.content.includes('ERROR: boom')); + t.true(result.content.includes('b')); + t.true(result.content.includes('d')); + t.false(result.content.includes('a')); + t.false(result.content.includes('e')); +}); + +test('queryCiLog search is case-insensitive', t => { + const result = queryCiLog('Something FAILED here', {search: 'failed'}); + t.is(result.matchCount, 1); +}); + +test('queryCiLog search reports zero matches clearly', t => { + const result = queryCiLog(makeLog(10), {search: 'nope'}); + t.is(result.matchCount, 0); + t.regex(result.content, /No matches/); +}); + +test('queryCiLog search truncates when too many context lines match', t => { + const lines = Array.from({length: 500}, (_, i) => `ERROR ${i}`); + const result = queryCiLog(lines.join('\n'), { + search: 'error', + limit: 50, + contextLines: 0, + }); + t.is(result.matchCount, 500); + t.true(result.truncated); + t.regex(result.content, /showing last 50/); +}); + +test('queryCiLog search truncation keeps matches near the end, not the start', t => { + const lines = Array.from({length: 500}, (_, i) => `line ${i}`); + for (let i = 0; i < 500; i += 10) lines[i] = `ERROR at ${i}`; + lines[499] = 'ERROR real cause here'; + const result = queryCiLog(lines.join('\n'), { + search: 'error', + limit: 5, + contextLines: 0, + }); + t.true(result.content.includes('ERROR real cause here')); + t.false(result.content.includes('ERROR at 0')); +}); + +test('queryCiLog search separates non-adjacent match blocks with a marker', t => { + const lines = ['ERROR one', 'x', 'x', 'x', 'x', 'x', 'ERROR two']; + const result = queryCiLog(lines.join('\n'), { + search: 'error', + contextLines: 1, + }); + t.true(result.content.includes('--')); +}); diff --git a/source/tools/git/log-utils.ts b/source/tools/git/log-utils.ts new file mode 100644 index 000000000..f35ba7317 --- /dev/null +++ b/source/tools/git/log-utils.ts @@ -0,0 +1,155 @@ +/** + * CI Log Query Utilities + * + * `gh run view --log` / `--log-failed` output can run to thousands of lines, + * far more than fits an LLM context window. `queryCiLog` gives callers a way + * to either page through the tail of a log (failures are usually near the + * end) or search it for a substring with surrounding context, so a tool can + * hand back a bounded, useful slice instead of the whole thing. + * + * Pure string-in/string-out — no gh/child_process dependency — so it's + * testable without a real CI run. + */ + +const DEFAULT_LIMIT = 300; +const MAX_LIMIT = 2000; +const DEFAULT_CONTEXT_LINES = 2; + +export interface LogQueryOptions { + /** Case-insensitive substring match. When set, pagination is ignored. */ + search?: string; + /** Line offset. Without `search`, counts back from the end of the log. */ + offset?: number; + /** Max lines returned. Default 300, hard-capped at 2000. */ + limit?: number; + /** Lines of context kept around each search match. Default 2. */ + contextLines?: number; +} + +export interface LogQueryResult { + content: string; + totalLines: number; + truncated: boolean; + /** Only set when `search` is used. */ + matchCount?: number; +} + +function resolveLimit(limit?: number): number { + if (!limit || limit <= 0) return DEFAULT_LIMIT; + return Math.min(limit, MAX_LIMIT); +} + +function paginateTail( + lines: string[], + offset: number, + limit: number, +): LogQueryResult { + const totalLines = lines.length; + // offset counts back from the end: offset=0 means "the last `limit` + // lines", offset=200 means "skip the most recent 200 lines, then take + // `limit` lines before that". An offset overshooting the log (nothing + // left to skip to) saturates at the earliest `limit` lines instead of + // returning nothing. + let end = Math.max(totalLines - offset, 0); + if (end === 0 && totalLines > 0) { + end = Math.min(limit, totalLines); + } + const start = Math.max(end - limit, 0); + const content = lines.slice(start, end).join('\n'); + const truncated = start > 0 || end < totalLines; + + if (!truncated) { + return {content, totalLines, truncated: false}; + } + + const marker = + `... [Log truncated: showing lines ${start + 1}-${end} of ${totalLines}; ` + + `pass logs.offset=${totalLines - start} to see earlier lines]`; + return { + content: content ? `${marker}\n\n${content}` : marker, + totalLines, + truncated: true, + }; +} + +function searchLines( + lines: string[], + search: string, + contextLines: number, + limit: number, +): LogQueryResult { + const totalLines = lines.length; + const needle = search.toLowerCase(); + const keep = new Set(); + let matchCount = 0; + + for (let i = 0; i < lines.length; i++) { + if (lines[i].toLowerCase().includes(needle)) { + matchCount++; + for ( + let j = Math.max(0, i - contextLines); + j <= Math.min(lines.length - 1, i + contextLines); + j++ + ) { + keep.add(j); + } + } + } + + if (matchCount === 0) { + return { + content: `No matches for "${search}" in ${totalLines} lines.`, + totalLines, + truncated: false, + matchCount: 0, + }; + } + + const keptIndices = Array.from(keep).sort((a, b) => a - b); + // Keep the *latest* matches, not the earliest: CI failures are typically + // near the end of the log, so truncating from the front would silently + // drop the root cause on a noisy log with many matches. + const limited = keptIndices.slice(-limit); + const truncated = limited.length < keptIndices.length; + + const contentLines: string[] = []; + let previous = -2; + for (const idx of limited) { + if (idx !== previous + 1 && contentLines.length > 0) { + contentLines.push('--'); + } + contentLines.push(lines[idx]); + previous = idx; + } + + let content = contentLines.join('\n'); + if (truncated) { + content += `\n\n... [${matchCount} matches for "${search}"; showing last ${limited.length} of ${keptIndices.length} context lines]`; + } + + return {content, totalLines, truncated, matchCount}; +} + +/** + * Query a CI log for a bounded, useful slice: either the tail (paginated + * via `offset`/`limit`) or lines matching `search` (with surrounding + * context), so a tool can avoid returning an entire multi-thousand-line log. + */ +export function queryCiLog( + log: string, + options?: LogQueryOptions, +): LogQueryResult { + const lines = log.length === 0 ? [] : log.split('\n'); + const limit = resolveLimit(options?.limit); + + if (options?.search) { + return searchLines( + lines, + options.search, + options.contextLines ?? DEFAULT_CONTEXT_LINES, + limit, + ); + } + + return paginateTail(lines, Math.max(options?.offset ?? 0, 0), limit); +} diff --git a/source/tools/git/utils.ts b/source/tools/git/utils.ts index 1aa6187aa..fb0dab025 100644 --- a/source/tools/git/utils.ts +++ b/source/tools/git/utils.ts @@ -262,16 +262,35 @@ export function formatGitStatusSummary(status: GitStatusSummary): { * Spawn a command, collect stdout, and resolve with the trimmed output. * Rejects with stderr (or an exit-code message) on non-zero exit. `label` is * the human-readable command name used in error messages (e.g. 'Git', 'gh'). + * When `timeoutMs` is given, the process is killed and the promise rejects + * if it hasn't closed in time (unset by default, matching prior behavior). */ function execProcess( command: string, args: string[], label: string, + timeoutMs?: number, ): Promise { return new Promise((resolve, reject) => { const proc = spawn(command, args); let stdout = ''; let stderr = ''; + let timedOut = false; + let closed = false; + + const timer = + timeoutMs && timeoutMs > 0 + ? setTimeout(() => { + timedOut = true; + proc.kill('SIGTERM'); + // Force-kill if the process ignores SIGTERM within a grace window. + const forceKillTimer = setTimeout(() => { + if (!closed) proc.kill('SIGKILL'); + }, 1_000); + forceKillTimer.unref(); + }, timeoutMs) + : undefined; + timer?.unref(); proc.stdout.on('data', (data: Buffer) => { stdout += data.toString(); @@ -282,7 +301,11 @@ function execProcess( }); proc.on('close', (code: number | null) => { - if (code === 0) { + closed = true; + if (timer) clearTimeout(timer); + if (timedOut) { + reject(new Error(`${label} command timed out after ${timeoutMs}ms`)); + } else if (code === 0) { resolve(stdout.trimEnd()); } else { const errorMessage = @@ -292,6 +315,8 @@ function execProcess( }); proc.on('error', error => { + closed = true; + if (timer) clearTimeout(timer); reject(new Error(`Failed to execute ${command}: ${error.message}`)); }); }); @@ -305,10 +330,14 @@ export async function execGit(args: string[]): Promise { } /** - * Execute a gh CLI command and return the output + * Execute a gh CLI command and return the output. Pass `timeoutMs` for + * calls that can be slow/hang (e.g. fetching CI run logs). */ -export async function execGh(args: string[]): Promise { - return execProcess('gh', args, 'gh'); +export async function execGh( + args: string[], + timeoutMs?: number, +): Promise { + return execProcess('gh', args, 'gh', timeoutMs); } // ============================================================================ diff --git a/source/tools/tool-manager.ts b/source/tools/tool-manager.ts index aeb3a5bbe..95331f2b1 100644 --- a/source/tools/tool-manager.ts +++ b/source/tools/tool-manager.ts @@ -51,7 +51,13 @@ const MODE_EXCLUDED_TOOLS: Record = { // No git mutation tools — keep read-only git tools 'git_add', 'git_commit', - 'git_pr', // can create PRs — excluded like other git mutators + // git_pr bundles both read-only actions (view/list/diff/checks/logs) + // and mutating ones (create/comment/review) behind one tool name, so + // it can't be split per-action here — excluded wholesale for now. + // source/verify/trust.ts is the action-granular allowlist meant to + // replace this blunt exclusion once the verify subcommand (issue + // #861) consumes it. + 'git_pr', ], headless: ['ask_user', 'agent'], }; diff --git a/source/verify/trust.spec.ts b/source/verify/trust.spec.ts new file mode 100644 index 000000000..bb168bd5b --- /dev/null +++ b/source/verify/trust.spec.ts @@ -0,0 +1,120 @@ +/** + * Trust Level Tests + */ + +import test from 'ava'; +import {getAllowedToolNames, isActionAllowed} from './trust'; + +console.log('\ntrust.spec.ts – Trust Levels'); + +// ============================================================================ +// comment-only: the write-access boundary the issue calls out explicitly +// ============================================================================ + +test('comment-only denies write_file', t => { + t.false(isActionAllowed('comment-only', 'write_file')); +}); + +test('comment-only denies string_replace', t => { + t.false(isActionAllowed('comment-only', 'string_replace')); +}); + +test('comment-only denies execute_bash', t => { + t.false(isActionAllowed('comment-only', 'execute_bash')); +}); + +test('comment-only denies git_add and git_commit', t => { + t.false(isActionAllowed('comment-only', 'git_add')); + t.false(isActionAllowed('comment-only', 'git_commit')); +}); + +test('comment-only allows read/investigate tools', t => { + t.true(isActionAllowed('comment-only', 'read_file')); + t.true(isActionAllowed('comment-only', 'find_files')); + t.true(isActionAllowed('comment-only', 'search_file_contents')); + t.true(isActionAllowed('comment-only', 'list_directory')); + t.true(isActionAllowed('comment-only', 'git_status')); + t.true(isActionAllowed('comment-only', 'git_diff')); + t.true(isActionAllowed('comment-only', 'git_log')); + t.true(isActionAllowed('comment-only', 'lsp_get_diagnostics')); + t.true(isActionAllowed('comment-only', 'web_search')); + t.true(isActionAllowed('comment-only', 'fetch_url')); +}); + +test('comment-only allows git_pr read/comment/review actions but not create', t => { + t.true(isActionAllowed('comment-only', 'git_pr', 'view')); + t.true(isActionAllowed('comment-only', 'git_pr', 'list')); + t.true(isActionAllowed('comment-only', 'git_pr', 'diff')); + t.true(isActionAllowed('comment-only', 'git_pr', 'comment')); + t.true(isActionAllowed('comment-only', 'git_pr', 'review')); + t.true(isActionAllowed('comment-only', 'git_pr', 'checks')); + t.true(isActionAllowed('comment-only', 'git_pr', 'logs')); + t.false(isActionAllowed('comment-only', 'git_pr', 'create')); +}); + +test('comment-only requires an explicit action for git_pr', t => { + t.false(isActionAllowed('comment-only', 'git_pr')); +}); + +test('comment-only denies unknown tools', t => { + t.false(isActionAllowed('comment-only', 'some_unknown_tool')); +}); + +// ============================================================================ +// auto-fix +// ============================================================================ + +test('auto-fix allows file mutation and local commit', t => { + t.true(isActionAllowed('auto-fix', 'write_file')); + t.true(isActionAllowed('auto-fix', 'string_replace')); + t.true(isActionAllowed('auto-fix', 'git_add')); + t.true(isActionAllowed('auto-fix', 'git_commit')); + t.true(isActionAllowed('auto-fix', 'execute_bash')); +}); + +test('auto-fix allows git_pr create (to open a draft PR)', t => { + t.true(isActionAllowed('auto-fix', 'git_pr', 'create')); +}); + +test('auto-fix still allows every comment-only tool', t => { + for (const tool of getAllowedToolNames('comment-only')) { + if (tool === 'git_pr') continue; // action set differs on purpose + t.true(isActionAllowed('auto-fix', tool)); + } +}); + +// ============================================================================ +// full-commit +// ============================================================================ + +test('full-commit allows everything auto-fix allows', t => { + t.true(isActionAllowed('full-commit', 'write_file')); + t.true(isActionAllowed('full-commit', 'execute_bash')); + t.true(isActionAllowed('full-commit', 'git_pr', 'create')); +}); + +// ============================================================================ +// getAllowedToolNames +// ============================================================================ + +test('getAllowedToolNames returns a strictly increasing surface across levels', t => { + const commentOnly = new Set(getAllowedToolNames('comment-only')); + const autoFix = new Set(getAllowedToolNames('auto-fix')); + const fullCommit = new Set(getAllowedToolNames('full-commit')); + + for (const tool of commentOnly) { + t.true(autoFix.has(tool)); + } + for (const tool of autoFix) { + t.true(fullCommit.has(tool)); + } + t.true(autoFix.has('write_file')); + t.false(commentOnly.has('write_file')); +}); + +test('getAllowedToolNames includes git_pr exactly once per level', t => { + for (const level of ['comment-only', 'auto-fix', 'full-commit'] as const) { + const names = getAllowedToolNames(level); + t.is(names.filter(n => n === 'git_pr').length, 1); + } +}); diff --git a/source/verify/trust.ts b/source/verify/trust.ts new file mode 100644 index 000000000..eb8d12b11 --- /dev/null +++ b/source/verify/trust.ts @@ -0,0 +1,143 @@ +/** + * Trust Levels + * + * Maps the three execution trust levels from the Agentic CI/CD Gate roadmap + * (issue #860) to strict tool allowlists. This module is intentionally + * standalone: it does not touch `ToolManager`, `resolveToolApproval`, or + * `DevelopmentMode`. It exists so the `verify` subcommand (Phase 2, #861) + * and the `--trust` CLI flag (Phase 4, #863) have a single, auditable place + * to look up "what can a run at this trust level touch" when they build the + * tool list for the `verify-pr-review`/`verify-ci-investigator` subagents — + * both `ToolManager.getFilteredTools(names)` and the subagent executor's + * `tools`/`disallowedTools` filtering already accept an explicit tool-name + * list, so `getAllowedToolNames()` below is designed to be dropped straight + * into either. + * + * `git_pr` is the one tool that needs sub-action granularity, since its + * actions (view/list/diff/comment/review/checks/logs/create) are selected by + * input shape rather than separate tool names — see `source/tools/git/git-pr.tsx`. + */ + +export type TrustLevel = 'comment-only' | 'auto-fix' | 'full-commit'; + +/** All `git_pr` action keys, matching `GitPrInput`'s fields in git-pr.tsx. */ +export type GitPrAction = + | 'create' + | 'view' + | 'list' + | 'diff' + | 'comment' + | 'review' + | 'checks' + | 'logs'; + +interface ToolAllowance { + tool: string; + /** Omitted = every action of this tool is allowed. */ + actions?: readonly GitPrAction[]; +} + +const GIT_PR_READ_AND_REVIEW_ACTIONS: readonly GitPrAction[] = [ + 'view', + 'list', + 'diff', + 'comment', + 'review', + 'checks', + 'logs', +]; + +const GIT_PR_ALL_ACTIONS: readonly GitPrAction[] = [ + ...GIT_PR_READ_AND_REVIEW_ACTIONS, + 'create', +]; + +// Read/investigate tools — safe at every trust level, never touch the +// working tree or push anything. +const INVESTIGATION_TOOLS: readonly ToolAllowance[] = [ + {tool: 'read_file'}, + {tool: 'find_files'}, + {tool: 'search_file_contents'}, + {tool: 'list_directory'}, + {tool: 'git_status'}, + {tool: 'git_diff'}, + {tool: 'git_log'}, + {tool: 'lsp_get_diagnostics'}, + {tool: 'web_search'}, + {tool: 'fetch_url'}, +]; + +// File-mutation tools — the write access the issue calls out explicitly as +// denied in comment-only mode. `write_file`/`string_replace` are this +// repo's primary editors (see CLAUDE.md); `git_add`/`git_commit` stage and +// commit the result; `execute_bash` covers running tests/build to verify a +// fix before committing it. +const MUTATION_TOOLS: readonly ToolAllowance[] = [ + {tool: 'write_file'}, + {tool: 'string_replace'}, + {tool: 'git_add'}, + {tool: 'git_commit'}, + {tool: 'execute_bash'}, +]; + +/** + * Per-level tool allowlists. + * + * - comment-only: investigate + comment/review on PRs. No working-tree + * writes, no `git_pr` create/merge. + * - auto-fix: comment-only + file mutation/local commit + `git_pr` create + * (Phase 4's flow opens a fresh branch and a *draft* PR rather than + * pushing to the original branch directly — that workflow constraint is + * enforced by the Phase 4 subagent, not by this flat allowlist). + * - full-commit: everything auto-fix has; the same tool surface, gated + * instead by explicit user opt-in and warnings at the CLI layer (Phase 4). + */ +const TRUST_POLICIES: Record = { + 'comment-only': [ + ...INVESTIGATION_TOOLS, + {tool: 'git_pr', actions: GIT_PR_READ_AND_REVIEW_ACTIONS}, + ], + 'auto-fix': [ + ...INVESTIGATION_TOOLS, + ...MUTATION_TOOLS, + {tool: 'git_pr', actions: GIT_PR_ALL_ACTIONS}, + ], + 'full-commit': [ + ...INVESTIGATION_TOOLS, + ...MUTATION_TOOLS, + {tool: 'git_pr', actions: GIT_PR_ALL_ACTIONS}, + ], +}; + +function findAllowance( + level: TrustLevel, + toolName: string, +): ToolAllowance | undefined { + return TRUST_POLICIES[level].find(a => a.tool === toolName); +} + +/** + * Whether `toolName` (optionally a specific `git_pr` `action`) is permitted + * at the given trust level. For tools other than `git_pr`, `action` is + * ignored — the allowlist is all-or-nothing by tool name. + */ +export function isActionAllowed( + level: TrustLevel, + toolName: string, + action?: GitPrAction, +): boolean { + const allowance = findAllowance(level, toolName); + if (!allowance) return false; + if (!allowance.actions) return true; + if (action === undefined) return false; + return allowance.actions.includes(action); +} + +/** + * Dedup'd tool names permitted at the given trust level, for callers that + * want a flat list (e.g. to pass into `ToolManager.getFilteredTools` or a + * subagent's `tools` config) rather than per-action checks. + */ +export function getAllowedToolNames(level: TrustLevel): string[] { + return TRUST_POLICIES[level].map(a => a.tool); +}