diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6800f6d..3027941 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,6 +123,16 @@ jobs: SMOKE_TIMEOUT_MS: '300000' run: npm run smoke:app + - name: Review suite (Linux, via virtual display) + if: runner.os == 'Linux' + env: + ELECTRON_DISABLE_SANDBOX: '1' + run: xvfb-run --auto-servernum npm run smoke:review + + - name: Review suite + if: runner.os != 'Linux' + run: npm run smoke:review + - name: Canvas interactions (real Chromium input and clipboard menus) if: runner.os == 'Linux' env: diff --git a/package.json b/package.json index 6053b63..4ebc42d 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "smoke:cliproxy": "esbuild test/cliproxy.ts --bundle --platform=node --format=cjs --packages=external --outfile=test/.out/cliproxy.cjs && electron test/.out/cliproxy.cjs", "worktree:setup": "npm ci --prefer-offline --no-audit --no-fund && electron-builder install-app-deps", "smoke:store": "node test/store-guard.mjs", + "smoke:review": "esbuild test/review.ts --bundle --platform=node --format=cjs --packages=external --outfile=test/.out/review.cjs && electron test/.out/review.cjs && esbuild test/session-review.ts --bundle --platform=node --format=cjs --packages=external --outfile=test/.out/session-review.cjs && electron test/.out/session-review.cjs", "smoke:cookies": "esbuild test/cookies.ts --bundle --platform=node --format=cjs --packages=external --outfile=test/.out/cookies.cjs && electron test/.out/cookies.cjs", "smoke:i18n": "esbuild test/i18n.ts --bundle --platform=node --format=cjs --outfile=test/.out/i18n.cjs && node test/.out/i18n.cjs", "i18n:translate": "node script/i18n-translate.mjs", diff --git a/src/main/db/migrations.ts b/src/main/db/migrations.ts index 401482d..e04c717 100644 --- a/src/main/db/migrations.ts +++ b/src/main/db/migrations.ts @@ -101,6 +101,15 @@ const REPAIR_SCHEMA_SQL = /* sql */ ` hidden_at INTEGER NOT NULL, PRIMARY KEY (provider_id, model) ); + CREATE TABLE IF NOT EXISTS session_review_baselines ( + session_id TEXT NOT NULL REFERENCES chats(id) ON DELETE CASCADE, + repo_key TEXT NOT NULL, + repo_root TEXT NOT NULL, + baseline_tree TEXT NOT NULL, + baseline_ref TEXT NOT NULL, + created_at INTEGER NOT NULL, + PRIMARY KEY (session_id, repo_key) + ); CREATE TABLE IF NOT EXISTS projects ( path TEXT PRIMARY KEY, sort_order INTEGER NOT NULL, @@ -493,6 +502,20 @@ export const MIGRATIONS: Migration[] = [ hidden_at INTEGER NOT NULL, PRIMARY KEY (provider_id, model) ); + `, + + // ---- v24: durable per-session review baselines ---- + // One Git tree per repository records the workspace before the first turn. + /* sql */ ` + CREATE TABLE session_review_baselines ( + session_id TEXT NOT NULL REFERENCES chats(id) ON DELETE CASCADE, + repo_key TEXT NOT NULL, + repo_root TEXT NOT NULL, + baseline_tree TEXT NOT NULL, + baseline_ref TEXT NOT NULL, + created_at INTEGER NOT NULL, + PRIMARY KEY (session_id, repo_key) + ); ` ] diff --git a/src/main/db/repo.ts b/src/main/db/repo.ts index c8af39d..959f610 100644 --- a/src/main/db/repo.ts +++ b/src/main/db/repo.ts @@ -54,6 +54,24 @@ interface ProviderRow { has_credential: number } +export interface SessionReviewBaseline { + sessionId: string + repoKey: string + repoRoot: string + baselineTree: string + baselineRef: string + createdAt: number +} + +interface SessionReviewBaselineRow { + session_id: string + repo_key: string + repo_root: string + baseline_tree: string + baseline_ref: string + created_at: number +} + interface ChatRow { id: string title: string @@ -1684,6 +1702,56 @@ export function recordActivityTurn(day: string, turns = 1): void { .run(day, Math.floor(turns)) } +export function getSessionReviewBaseline( + sessionId: string, + repoKey: string +): SessionReviewBaseline | undefined { + const row = getDb() + .prepare('SELECT * FROM session_review_baselines WHERE session_id = ? AND repo_key = ?') + .get(sessionId, repoKey) as SessionReviewBaselineRow | undefined + return row ? sessionReviewBaseline(row) : undefined +} + +export function listSessionReviewBaselines(sessionId: string): SessionReviewBaseline[] { + const rows = getDb() + .prepare('SELECT * FROM session_review_baselines WHERE session_id = ? ORDER BY repo_key') + .all(sessionId) as SessionReviewBaselineRow[] + return rows.map(sessionReviewBaseline) +} + +export function addSessionReviewBaseline( + input: Omit +): SessionReviewBaseline { + const createdAt = Date.now() + getDb() + .prepare( + `INSERT INTO session_review_baselines + (session_id, repo_key, repo_root, baseline_tree, baseline_ref, created_at) + VALUES (?, ?, ?, ?, ?, ?) + ON CONFLICT(session_id, repo_key) DO NOTHING` + ) + .run( + input.sessionId, + input.repoKey, + input.repoRoot, + input.baselineTree, + input.baselineRef, + createdAt + ) + return getSessionReviewBaseline(input.sessionId, input.repoKey) ?? { ...input, createdAt } +} + +function sessionReviewBaseline(row: SessionReviewBaselineRow): SessionReviewBaseline { + return { + sessionId: row.session_id, + repoKey: row.repo_key, + repoRoot: row.repo_root, + baselineTree: row.baseline_tree, + baselineRef: row.baseline_ref, + createdAt: row.created_at + } +} + /** Per-day turn counts from `fromDay` (inclusive, local YYYY-MM-DD) onward. */ export function listActivityDays(fromDay: string): Map { const rows = getDb() diff --git a/src/main/harness/agent.ts b/src/main/harness/agent.ts index 5fb2083..a4cc773 100644 --- a/src/main/harness/agent.ts +++ b/src/main/harness/agent.ts @@ -509,6 +509,19 @@ function gatherMcpRecords(cwd: string): McpServerRecord[] { /** OpenAI function schemas for the workspace/browser tools (the base toolset). */ const BASE_SCHEMAS = [ + fn( + 'code_review', + 'Show the active Git diff for the workspace. Unstaged includes untracked text files. Use this when the user asks you to review changes.', + { + scope: { + type: 'string', + enum: ['unstaged', 'staged', 'branch', 'commit'], + description: 'Review scope (default "unstaged").' + }, + commit: str('Commit or ref. Required when scope is "commit".') + }, + [] + ), fn( 'read', 'Read a file from the workspace.', @@ -2148,6 +2161,8 @@ function toolTitle(name: string, input: Record): string { return s(input.title) || s(input.name) || 'session metadata' case 'skill': return s(input.name) + case 'code_review': + return s(input.commit) || s(input.scope) || 'unstaged' default: return isMcpTool(name) ? mcpToolTitle(name) : '' } diff --git a/src/main/harness/tools.ts b/src/main/harness/tools.ts index 8f6bc74..10f4386 100644 --- a/src/main/harness/tools.ts +++ b/src/main/harness/tools.ts @@ -24,8 +24,10 @@ import { normalizeFetchUrl } from '../../shared/web' import * as browser from '../services/browser' +import * as gitService from '../services/git' import * as lsp from '../services/lsp' import * as repo from '../db/repo' +import { discoverRepos } from '../services/workspace' import { isManagedToolOutputPath } from '../services/tool-output-store' import { renderDiagnosticsBlock } from '../../shared/lsp' import { @@ -173,6 +175,7 @@ interface BgProc { } const bgProcs = new Map() let bgCounter = 0 +const MAX_REVIEW_PATCH = 50_000 export async function runTool( name: string, @@ -194,6 +197,8 @@ export async function runTool( return runBashOutput(str(input.id ?? input.process), owningSessionId(ctx)) case 'bash_kill': return runBashKill(str(input.id ?? input.process), owningSessionId(ctx)) + case 'code_review': + return await runCodeReview(str(input.scope), str(input.commit), ctx) case 'read': return await runRead(str(input.path ?? input.file), ctx.cwd) case 'write': @@ -301,6 +306,139 @@ export async function runTool( } } +async function runCodeReview(scope: string, commit: string, ctx: ToolContext): Promise { + const resolvedScope = scope || 'unstaged' + if (!['unstaged', 'staged', 'branch', 'commit'].includes(resolvedScope)) { + return { ok: false, output: 'Invalid review scope.' } + } + if (resolvedScope === 'commit' && !commit) { + return { ok: false, output: 'Commit scope requires a commit.' } + } + if (!(await gitService.isGitAvailable())) { + return { ok: false, output: 'Git is not available in this workspace.' } + } + + return untilAborted(ctx.signal, async () => { + const typedScope = resolvedScope as import('../../shared/api').GitReviewScope + const targets = resolveCodeReviewRepos(ctx) + if (!targets.length) return { ok: false, output: 'Git is not available in this workspace.' } + + const multi = targets.length > 1 + const sections: string[] = [] + let validRanges = 0 + for (const target of targets) { + const heading = multi ? `## Repository: ${target.name}\n` : '' + const root = target.cwd ? await gitService.repoRoot(target.cwd) : null + if (!root) { + sections.push(`${heading}Error: repository is unavailable.`) + continue + } + const revs = await gitService.revsForScope(root, typedScope, commit || undefined) + if (!revs) { + const detail = + typedScope === 'commit' + ? `commit ${commit} does not exist in this repository.` + : 'no valid commit range found for this scope.' + sections.push(`${heading}Error: ${detail}`) + continue + } + const r = await gitService.git( + [ + 'diff', + '--patch', + '--binary', + '--find-renames', + '--find-copies', + ...gitService.diffRange(revs) + ], + root + ) + if (!r.ok) { + sections.push(`${heading}Error: failed to generate diff.`) + continue + } + validRanges++ + + let output = r.stdout + if (typedScope === 'unstaged') { + const untracked = (await gitService.reviewFiles(root, 'unstaged')).filter( + (file) => file.status === 'untracked' + ) + for (const file of untracked) { + const diff = await gitService.reviewDiff(root, 'unstaged', file.path) + if (!diff || diff.binary) { + output += `\nUntracked binary or large file: ${file.path}\n` + continue + } + const body = diff.after + .split('\n') + .map((line) => `+${line}`) + .join('\n') + output += `\ndiff --git a/${file.path} b/${file.path}\nnew file mode 100644\n--- /dev/null\n+++ b/${file.path}\n@@ -0,0 +1,${file.additions} @@\n${body}\n` + } + } + sections.push(`${heading}${output.trim() ? output : 'No changes found.'}`) + } + + if (typedScope === 'commit' && validRanges === 0) { + return { + ok: false, + output: `Commit ${commit} was not found in any repository.\n\n${sections.join('\n\n')}` + } + } + const output = sections.join('\n\n') + return { + ok: true, + output: + output.length > MAX_REVIEW_PATCH + ? `${output.slice(0, MAX_REVIEW_PATCH)}\n... (diff truncated due to length)` + : output + } + }) +} + +interface CodeReviewRepo { + name: string + cwd: string +} + +interface CodeReviewOwner { + repos?: { name: string; worktreePath: string }[] | null + workspacePath?: string | null +} + +/** Pure selection half, exposed so multi-repo routing can be unit tested. */ +export function codeReviewReposForOwner( + owner: CodeReviewOwner | undefined, + cwd: string, + discover: typeof discoverRepos = discoverRepos +): CodeReviewRepo[] { + if (owner?.repos?.length) { + return owner.repos.map((link) => ({ name: link.name, cwd: link.worktreePath })) + } + if (owner?.workspacePath) { + const discovered = discover(owner.workspacePath) + if (discovered.layout === 'multi') { + return discovered.roots.map((repoCwd) => ({ name: path.basename(repoCwd), cwd: repoCwd })) + } + } + return cwd ? [{ name: path.basename(cwd), cwd }] : [] +} + +/** Resolve the repositories owned by a tool's root session without IPC. */ +export function resolveCodeReviewRepos( + ctx: Pick +): CodeReviewRepo[] { + if (!ctx.sessionId) return codeReviewReposForOwner(undefined, ctx.cwd) + try { + const root = repo.getChat(repo.rootSessionId(ctx.sessionId)) + return codeReviewReposForOwner(root, ctx.cwd) + } catch { + // Tests and keyless callers may not have initialized the database. + } + return codeReviewReposForOwner(undefined, ctx.cwd) +} + /** Whether a thrown value is an abort (ours, or a fetch/DOM AbortError). */ function isAbort(e: unknown): boolean { return e instanceof Error && e.name === 'AbortError' diff --git a/src/main/ipc/index.ts b/src/main/ipc/index.ts index 4ecc47c..b5dc8cf 100644 --- a/src/main/ipc/index.ts +++ b/src/main/ipc/index.ts @@ -1,4 +1,4 @@ -import { app, BrowserWindow, dialog, ipcMain, shell } from 'electron' +import { app, BrowserWindow, dialog, ipcMain, shell } from 'electron' import { CHANNELS } from '../../shared/ipc' import type { Language } from '../../shared/i18n' import { DEFAULT_MOTION, type MotionPreference } from '../../shared/motion' @@ -19,6 +19,10 @@ import type { LlmStartInput, McpServerView, RemoteStartInput, + ReviewCommit, + ReviewDiff, + ReviewFile, + ReviewTarget, SkillView, SkillWriteInput, SyncOutcome, @@ -54,6 +58,7 @@ import { import { sessionCwd, discoverRepos } from '../services/workspace' import nodePath from 'node:path' import * as git from '../services/git' +import * as sessionReview from '../services/session-review' import * as forge from '../services/forge' import type { ForgeKind } from '../../shared/forge' import { pruneWorktrees, removeWorktreeForChat, renameWorkstreamBranch } from '../services/worktree' @@ -116,7 +121,7 @@ const llmControllers = new Map() * * `llmControllers` alone was not enough for Stop to be reliable. The renderer * only learns a requestId once the turn is actually starting, and real work - * happens before that — most of all compaction, which is a full model call on a + * happens before that — most of all compaction, which is a full model call on a * long history and used to run with a hardcoded never-aborted signal. Stop * during that window found no requestId and silently did nothing, which is a * large part of why the button felt stuck. @@ -342,7 +347,10 @@ export function registerIpc(): void { // Fire-and-forget: deletion must never block on git, so a failure here is // logged and the session goes anyway (`git:prune-worktrees` sweeps up // whatever is left behind). It re-kills the session's processes internally - // and awaits them — the ordering that keeps removal working on Windows. + // and awaits them — the ordering that keeps removal working on Windows. + void sessionReview + .deleteSessionReviewBaselines(id) + .catch((e) => console.warn('[review] baseline cleanup failed:', e)) void removeWorktreeForChat(id).then( (r) => { if (!r.ok && r.error) console.warn('[worktree] remove on delete failed:', r.error) @@ -558,7 +566,7 @@ export function registerIpc(): void { state: getUpdateState() })) ipcMain.handle(CHANNELS.systemOpenExternal, async (_e, url: string) => { - // Only allow web URLs — never file:, javascript:, or other schemes. + // Only allow web URLs — never file:, javascript:, or other schemes. try { const parsed = new URL(url) if (parsed.protocol === 'http:' || parsed.protocol === 'https:') { @@ -804,7 +812,7 @@ export function registerIpc(): void { // If this session is shared to a phone, relay the turn there too so the phone // streams a desktop-typed reply live (the mirror of a phone turn on the PC). // The current prompt is the last user message; announce it so the phone shows - // the bubble it never echoed. `null` when nothing's shared → zero overhead. + // the bubble it never echoed. `null` when nothing's shared → zero overhead. const lastUser = [...input.messages].reverse().find((m) => m.role === 'user') const relay = remote.relayLocalTurnStart(input.sessionId, lastUser?.content) try { @@ -828,7 +836,7 @@ export function registerIpc(): void { llmControllers.get(requestId)?.abort() }) // Stop, as the UI means it: end everything this session has in flight, - // whatever stage it's at. Also cancels the session's delegates — stopping a + // whatever stage it's at. Also cancels the session's delegates — stopping a // turn while it waits on a subagent has to stop the subagent, or the work // carries on invisibly after the transcript says it stopped. ipcMain.handle(CHANNELS.llmAbortSession, (_e, sessionId: string) => { @@ -1057,7 +1065,7 @@ export function registerIpc(): void { * Per-repo status for a multi-repo session. * * Takes a SESSION id, not a path: the composite root is not a repository, so - * there is nothing at that path to interrogate — the session's `repos` links + * there is nothing at that path to interrogate — the session's `repos` links * are the only record of which repos it spans and where their checkouts are. * * Every repo is queried independently and a failure degrades to @@ -1143,9 +1151,9 @@ export function registerIpc(): void { ipcMain.handle( CHANNELS.gitCreateWorktree, async (_e, input: CreateWorktreeInput): Promise => { - if (!(await git.isGitAvailable())) return { ok: false, error: 'Git isn’t installed.' } + if (!(await git.isGitAvailable())) return { ok: false, error: 'Git isn’t installed.' } const root = await git.repoRoot(input.cwd) - if (!root) return { ok: false, error: 'This folder isn’t a git repository.' } + if (!root) return { ok: false, error: 'This folder isn’t a git repository.' } const r = input.mode === 'new' ? await git.createWorktree({ @@ -1158,7 +1166,7 @@ export function registerIpc(): void { ) ipcMain.handle(CHANNELS.gitRemoveWorktree, async (_e, worktreePath: string, force?: boolean) => { - if (!(await git.isGitAvailable())) return { ok: false, error: 'Git isn’t installed.' } + if (!(await git.isGitAvailable())) return { ok: false, error: 'Git isn’t installed.' } return git.removeWorktree(worktreePath, { force: force ?? false }) }) @@ -1169,6 +1177,125 @@ export function registerIpc(): void { pruneWorktrees(cwd, { dryRun: dryRun ?? true, force: true }) ) + type ReviewRepo = { name?: string; cwd: string } + + /** Resolve only repositories that belong to the requested session. */ + const reviewRepos = (sessionId: string, repoName?: string): ReviewRepo[] => { + if (!sessionId) return [] + const links = reposForSession(sessionId) + if (links.length) { + const selected = repoName ? links.filter((link) => link.name === repoName) : links + return selected.map((link) => ({ name: link.name, cwd: link.worktreePath })) + } + // A repo name on a single-repo session is invalid rather than a path escape + // hatch. The cwd itself is resolved from the session row in main. + if (repoName) return [] + const cwd = sessionCwd(sessionId) + return cwd ? [{ cwd }] : [] + } + + /** Review mutations are trusted main-window actions, never browser content. */ + const requireMainWindow = (event: Electron.IpcMainInvokeEvent): void => { + if (browser.keyForContents(event.sender)) throw new Error('Review action denied.') + } + + ipcMain.handle( + CHANNELS.reviewFiles, + async (event, target: ReviewTarget): Promise => { + requireMainWindow(event) + if (!target?.sessionId || !target.scope) return [] + const groups = await Promise.all( + reviewRepos(target.sessionId, target.repo).map(async ({ name, cwd }) => { + let files: ReviewFile[] + if (target.scope === 'session') { + files = await sessionReview.sessionReviewFiles(target.sessionId, [ + { key: name ?? cwd, name, cwd } + ]) + } else { + files = await git.reviewFiles(cwd, target.scope, target.commit) + } + return name ? files.map((file) => ({ ...file, repo: name })) : files + }) + ) + return groups.flat() + } + ) + + ipcMain.handle( + CHANNELS.reviewDiff, + async (event, target: ReviewTarget, file: string): Promise => { + requireMainWindow(event) + const selected = reviewRepos(target?.sessionId, target?.repo) + if (selected.length !== 1) return null + if (target.scope === 'session') { + const entry = selected[0] + return sessionReview.sessionReviewDiff( + target.sessionId, + { key: entry.name ?? entry.cwd, name: entry.name, cwd: entry.cwd }, + file, + target.oldPath + ) + } + return git.reviewDiff(selected[0].cwd, target.scope, file, target.commit, target.oldPath) + } + ) + + ipcMain.handle( + CHANNELS.reviewCommits, + async ( + event, + sessionId: string, + repoName?: string, + limit?: number + ): Promise => { + requireMainWindow(event) + const count = git.clampCommitLimit(limit) + const groups = await Promise.all( + reviewRepos(sessionId, repoName).map(async ({ name, cwd }) => { + const commits = await git.reviewCommits(cwd, count) + return name ? commits.map((commit) => ({ ...commit, repo: name })) : commits + }) + ) + return groups + .flat() + .sort((a, b) => b.date.localeCompare(a.date)) + .slice(0, count) + } + ) + + const mutateReviewRepos = async ( + target: ReviewTarget, + files: string[], + action: (cwd: string, paths: string[]) => Promise<{ ok: boolean; error?: string }> + ): Promise<{ ok: boolean; error?: string }> => { + const selected = reviewRepos(target?.sessionId, target?.repo) + if (!selected.length) return { ok: false, error: 'No repository for this session.' } + if (selected.length > 1 && files.length) + return { ok: false, error: 'Choose a repository before changing individual files.' } + const errors: string[] = [] + for (const item of selected) { + const result = await action(item.cwd, files) + if (!result.ok) errors.push(`${item.name ? `${item.name}: ` : ''}${result.error ?? 'Failed'}`) + } + return errors.length ? { ok: false, error: errors.join('\n') } : { ok: true } + } + + ipcMain.handle(CHANNELS.reviewStage, (event, target: ReviewTarget, files: string[]) => { + requireMainWindow(event) + return mutateReviewRepos(target, files, git.stageFiles) + }) + ipcMain.handle(CHANNELS.reviewUnstage, (event, target: ReviewTarget, files: string[]) => { + requireMainWindow(event) + return mutateReviewRepos(target, files, git.unstageFiles) + }) + ipcMain.handle(CHANNELS.reviewRevert, (event, target: ReviewTarget, files: string[]) => { + requireMainWindow(event) + if (target.scope !== 'unstaged' && target.scope !== 'staged') + return { ok: false, error: 'This review scope cannot be reverted.' } + const scope = target.scope + return mutateReviewRepos(target, files, (cwd, paths) => git.revertFiles(cwd, paths, scope)) + }) + // ---- forge (the git host behind `origin`: PR state for the branch) ---- // Same degrade-never-throw contract as the git handlers above: no remote, an // unknown host, no credential and a dead network all return a usable object. diff --git a/src/main/services/git.ts b/src/main/services/git.ts index d4de4f7..350bb7b 100644 --- a/src/main/services/git.ts +++ b/src/main/services/git.ts @@ -22,6 +22,7 @@ */ import { spawn } from 'node:child_process' import { promises as fs, realpathSync } from 'node:fs' +import os from 'node:os' import path from 'node:path' import { app } from 'electron' import { slugToBranchSegment } from '../../shared/slugs' @@ -32,7 +33,15 @@ import { placeholderBranchName } from '../../shared/branch' import * as repo from '../db/repo' -import type { RepoSyncTarget } from '../../shared/api' +import { + REVIEW_COMMITS, + REVIEW_COMMITS_MAX, + type GitReviewScope, + type RepoSyncTarget, + type ReviewCommit, + type ReviewDiff, + type ReviewFile +} from '../../shared/api' /** How long any single git command may run before it's killed. */ const GIT_TIMEOUT_MS = 30_000 @@ -165,7 +174,12 @@ function serialize(key: string, task: () => Promise): Promise { * Run one git command. Never throws — a missing binary, a non-zero exit and a * timeout all come back as `{ ok: false }` with whatever stderr git produced. */ -function execGit(args: string[], cwd: string, timeoutMs = GIT_TIMEOUT_MS): Promise { +function execGit( + args: string[], + cwd: string, + timeoutMs = GIT_TIMEOUT_MS, + env?: NodeJS.ProcessEnv +): Promise { return new Promise((resolve) => { let child: ReturnType try { @@ -182,7 +196,8 @@ function execGit(args: string[], cwd: string, timeoutMs = GIT_TIMEOUT_MS): Promi GIT_TERMINAL_PROMPT: '0', GIT_OPTIONAL_LOCKS: '0', GIT_EDITOR: 'true', - GCM_INTERACTIVE: 'never' + GCM_INTERACTIVE: 'never', + ...env } }) } catch (e) { @@ -226,10 +241,20 @@ function execGit(args: string[], cwd: string, timeoutMs = GIT_TIMEOUT_MS): Promi } /** Run a git command serialized against everything else touching this repo. */ -function git(args: string[], cwd: string, timeoutMs?: number): Promise { +export function git(args: string[], cwd: string, timeoutMs?: number): Promise { return serialize(cwd, () => execGit(args, cwd, timeoutMs)) } +/** Run Git with a controlled environment, still serialized with normal commands. */ +function gitWithEnv( + args: string[], + cwd: string, + env: NodeJS.ProcessEnv, + timeoutMs?: number +): Promise { + return serialize(cwd, () => execGit(args, cwd, timeoutMs, env)) +} + // --------------------------------------------------------------------------- // Availability // --------------------------------------------------------------------------- @@ -753,6 +778,522 @@ export async function resetToUpstream(cwd: string): Promise { return { ok: true, upstream: ref.ref, updated: true, stashed } } +// --------------------------------------------------------------------------- +// Reviewing changes +// --------------------------------------------------------------------------- + +const MAX_DIFF_BYTES = 400_000 +const BINARY_SNIFF_BYTES = 8_000 +const EMPTY_TREE_SHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904' + +/** The two revisions compared by a review scope. Empty string means the index. */ +export async function revsForScope( + cwd: string, + scope: GitReviewScope, + commit?: string +): Promise<{ from: string; to: string | null } | null> { + const root = await repoRoot(cwd) + return root ? revsForScopeAtRoot(root, scope, commit) : null +} + +async function revsForScopeAtRoot( + root: string, + scope: GitReviewScope, + commit?: string +): Promise<{ from: string; to: string | null } | null> { + switch (scope) { + case 'unstaged': + return { from: '', to: null } + case 'staged': + return { from: (await resolveCommit(root)) ?? EMPTY_TREE_SHA, to: '' } + case 'branch': { + const base = await mergeBaseForWorkstream(root) + // Branch review is committed history only. Comparing the merge-base to + // the worktree would silently mix staged and unstaged edits into the + // branch tab (and into the code_review tool). + return base ? { from: base, to: 'HEAD' } : null + } + case 'commit': { + const sha = await validCommit(root, commit) + if (!sha) return null + const parent = await git(['rev-parse', '--verify', '--end-of-options', `${sha}^`], root) + return { + from: + parent.ok && /^[0-9a-f]{40}$/i.test(parent.stdout.trim()) + ? parent.stdout.trim() + : EMPTY_TREE_SHA, + to: sha + } + } + } +} + +async function mergeBaseForWorkstream(cwd: string): Promise { + const branch = await currentBranch(cwd) + const base = (branch ? await baseBranchFor(cwd, branch) : null) ?? (await defaultBranch(cwd)) + if (!base) return null + for (const ref of [`origin/${base}`, base]) { + const r = await git(['merge-base', ref, 'HEAD'], cwd) + const sha = r.stdout.trim() + if (r.ok && sha) return sha + } + return null +} + +/** Arguments after `git diff` for a review range. */ +export function diffRange(revs: { from: string; to: string | null }): string[] { + if (revs.to === null) return revs.from === '' ? [] : [revs.from] + if (revs.to === '') return ['--cached', revs.from] + return [revs.from, revs.to] +} + +/** Changed files and line counts for one review scope. */ +export async function reviewFiles( + cwd: string, + scope: GitReviewScope, + commit?: string +): Promise { + if (!cwd || !(await isGitAvailable())) return [] + const root = await repoRoot(cwd) + return root ? reviewFilesAtRoot(root, scope, commit) : [] +} + +async function reviewFilesAtRoot( + root: string, + scope: GitReviewScope, + commit?: string +): Promise { + const revs = await revsForScopeAtRoot(root, scope, commit) + if (!revs) return [] + const range = diffRange(revs) + const [names, nums] = await Promise.all([ + git(['diff', '--name-status', '-z', '--find-renames', '--find-copies', ...range], root), + git(['diff', '--numstat', '-z', '--find-renames', '--find-copies', ...range], root) + ]) + if (!names.ok) return [] + const counts = parseNumstat(nums.ok ? nums.stdout : '') + const files = parseNameStatus(names.stdout).map((file) => ({ + ...file, + ...(counts.get(file.path) ?? { additions: 0, deletions: 0, binary: false }) + })) + if (scope === 'unstaged') files.push(...(await untrackedEntries(root))) + return files +} + +/** Keep a caller-supplied path lexically inside the repository checkout. */ +function reviewPath(cwd: string, file: string): string | null { + if (!file || file.includes('\0')) return null + const abs = path.resolve(cwd, file) + const rel = path.relative(cwd, abs) + return rel.startsWith('..') || path.isAbsolute(rel) ? null : abs +} + +/** Reject parent symlinks that leave the checkout. The leaf may itself be a symlink. */ +async function safeWorktreePath(cwd: string, file: string): Promise { + const abs = reviewPath(cwd, file) + if (!abs) return null + try { + const [root, parent] = await Promise.all([fs.realpath(cwd), fs.realpath(path.dirname(abs))]) + const rel = path.relative(root, parent) + if (rel.startsWith('..') || path.isAbsolute(rel)) return null + return path.join(parent, path.basename(abs)) + } catch { + return null + } +} + +async function validCommit(cwd: string, commit: string | undefined): Promise { + if (!commit) return null + const r = await git(['rev-parse', '--verify', '--end-of-options', `${commit}^{commit}`], cwd) + const sha = r.stdout.trim() + return r.ok && /^[0-9a-f]{40}$/i.test(sha) ? sha : null +} + +function parseNameStatus(out: string): ReviewFile[] { + const parts = out.split('\0').filter(Boolean) + const files: ReviewFile[] = [] + for (let i = 0; i < parts.length; i++) { + const kind = parts[i][0] + if (kind === 'R' || kind === 'C') { + const oldPath = parts[++i] + const newPath = parts[++i] + if (!newPath) break + files.push({ + path: newPath, + oldPath, + status: kind === 'R' ? 'renamed' : 'copied', + additions: 0, + deletions: 0, + binary: false + }) + continue + } + const file = parts[++i] + if (!file) break + files.push({ + path: file, + status: kind === 'A' ? 'added' : kind === 'D' ? 'deleted' : 'modified', + additions: 0, + deletions: 0, + binary: false + }) + } + return files +} + +function parseNumstat( + out: string +): Map { + const counts = new Map() + const parts = out.split('\0').filter(Boolean) + for (let i = 0; i < parts.length; i++) { + const match = /^(-|\d+)\t(-|\d+)\t(.*)$/.exec(parts[i]) + if (!match) continue + const [, addRaw, delRaw, tail] = match + let file = tail + if (tail === '') { + i += 2 + file = parts[i] + } + if (!file) break + counts.set(file, { + additions: addRaw === '-' ? 0 : Number(addRaw), + deletions: delRaw === '-' ? 0 : Number(delRaw), + binary: addRaw === '-' && delRaw === '-' + }) + } + return counts +} + +async function untrackedEntries(cwd: string): Promise { + const r = await git(['ls-files', '--others', '--exclude-standard', '-z'], cwd) + if (!r.ok) return [] + return Promise.all( + r.stdout + .split('\0') + .filter(Boolean) + .map(async (file): Promise => { + const text = await readWorktreeFile(cwd, file) + return { + path: file, + status: 'untracked', + additions: text === null ? 0 : countLines(text), + deletions: 0, + binary: text === null + } + }) + ) +} + +function countLines(text: string): number { + if (!text) return 0 + const lines = text.split('\n').length + return text.endsWith('\n') ? lines - 1 : lines +} + +/** Full before/after contents for a file. `oldPath` supplies a rename's source. */ +export async function reviewDiff( + cwd: string, + scope: GitReviewScope, + file: string, + commit?: string, + oldPath?: string +): Promise { + if (!cwd || !file) return null + const root = await repoRoot(cwd) + if (!root || !reviewPath(root, file) || (oldPath !== undefined && !reviewPath(root, oldPath))) + return null + const revs = await revsForScopeAtRoot(root, scope, commit) + if (!revs) return null + const sourcePath = oldPath ?? file + const untracked = scope === 'unstaged' && (await isUntracked(root, file)) + const before = untracked ? '' : await fileAt(root, revs.from, sourcePath) + const after = + revs.to === null ? await readWorktreeFile(root, file) : await fileAt(root, revs.to, file) + if (before === null || after === null) return { path: file, before: '', after: '', binary: true } + return { + path: file, + before: normalizeEol(before), + after: normalizeEol(after), + binary: false + } +} + +function normalizeEol(text: string): string { + return text.includes('\r') ? text.replace(/\r\n?/g, '\n') : text +} + +async function isUntracked(cwd: string, file: string): Promise { + if (!reviewPath(cwd, file)) return false + const r = await git(['ls-files', '--others', '--exclude-standard', '-z', '--', file], cwd) + return r.ok && r.stdout.split('\0').includes(file) +} + +async function isTracked(cwd: string, file: string): Promise { + if (!reviewPath(cwd, file)) return false + const indexed = await git(['ls-files', '--error-unmatch', '--', file], cwd) + if (indexed.ok) return true + const committed = await git(['ls-tree', '-r', '--name-only', '-z', 'HEAD', '--', file], cwd) + return committed.ok && committed.stdout.split('\0').includes(file) +} + +async function fileAt(cwd: string, rev: string, file: string): Promise { + const r = await git(['show', '--end-of-options', `${rev}:${file}`], cwd) + if (!r.ok) return '' + return renderable(r.stdout) +} + +async function readWorktreeFile(cwd: string, file: string): Promise { + const abs = await safeWorktreePath(cwd, file) + if (!abs) return null + try { + const stat = await fs.lstat(abs) + if (stat.isSymbolicLink()) return null + const buf = await fs.readFile(abs) + if (buf.length > MAX_DIFF_BYTES) return null + if (buf.subarray(0, BINARY_SNIFF_BYTES).includes(0)) return null + return buf.toString('utf8') + } catch { + return '' + } +} + +function renderable(text: string): string | null { + if (Buffer.byteLength(text) > MAX_DIFF_BYTES) return null + if (text.slice(0, BINARY_SNIFF_BYTES).includes('\0')) return null + return text +} + +/** + * Write the current tracked + untracked, non-ignored workspace into Git's + * object database without changing the real index. A copied index avoids + * re-hashing unchanged tracked files on every poll. The returned tree survives + * commits, staging and restarts, making it a stable session baseline. + */ +export async function snapshotWorktreeTree(cwd: string): Promise { + if (!cwd || !(await isGitAvailable())) return null + const root = await repoRoot(cwd) + if (!root) return null + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'roxy-review-index-')) + const index = path.join(dir, 'index') + const env = { GIT_INDEX_FILE: index } + try { + const sourceIndex = await git(['rev-parse', '--git-path', 'index'], root) + const sourcePath = sourceIndex.stdout.trim() + let copied = false + if (sourceIndex.ok && sourcePath) { + const absolute = path.isAbsolute(sourcePath) ? sourcePath : path.resolve(root, sourcePath) + copied = await fs + .copyFile(absolute, index) + .then(() => true) + .catch(() => false) + } + if (!copied) { + const empty = await gitWithEnv(['read-tree', '--empty'], root, env) + if (!empty.ok) return null + } + const add = await gitWithEnv(['add', '-A', '--ignore-errors', '--', '.'], root, env) + if (!add.ok) return null + const tree = await gitWithEnv(['write-tree'], root, env) + const sha = tree.stdout.trim() + return tree.ok && /^[0-9a-f]{40}$/i.test(sha) ? sha : null + } finally { + await fs.rm(dir, { recursive: true, force: true }).catch(() => undefined) + } +} + +/** Keep an otherwise-unreachable snapshot tree alive behind a private ref. */ +export async function setReviewBaselineRef( + cwd: string, + ref: string, + tree: string +): Promise { + if (!/^refs\/roxy\/sessions\/[A-Za-z0-9._/-]+$/.test(ref)) return false + const root = await repoRoot(cwd) + if (!root || !/^[0-9a-f]{40}$/i.test(tree)) return false + return (await git(['update-ref', ref, tree], root)).ok +} + +export async function deleteReviewBaselineRef(cwd: string, ref: string): Promise { + if (!/^refs\/roxy\/sessions\/[A-Za-z0-9._/-]+$/.test(ref)) return + const root = await repoRoot(cwd) + if (root) await git(['update-ref', '-d', ref], root) +} + +/** Changed files between a persisted session baseline tree and the current workspace. */ +export async function reviewFilesFromTree(cwd: string, from: string): Promise { + if (!/^[0-9a-f]{40}$/i.test(from)) return [] + const root = await repoRoot(cwd) + const current = root ? await snapshotWorktreeTree(root) : null + if (!root || !current) return [] + const [names, nums] = await Promise.all([ + git(['diff', '--name-status', '-z', '--find-renames', '--find-copies', from, current], root), + git(['diff', '--numstat', '-z', '--find-renames', '--find-copies', from, current], root) + ]) + if (!names.ok) return [] + const counts = parseNumstat(nums.ok ? nums.stdout : '') + return parseNameStatus(names.stdout).map((file) => ({ + ...file, + ...(counts.get(file.path) ?? { additions: 0, deletions: 0, binary: false }) + })) +} + +/** Full before/after contents for one session-scoped file. */ +export async function reviewDiffFromTree( + cwd: string, + from: string, + file: string, + oldPath?: string +): Promise { + if (!/^[0-9a-f]{40}$/i.test(from) || !file) return null + const root = await repoRoot(cwd) + if (!root || !reviewPath(root, file) || (oldPath !== undefined && !reviewPath(root, oldPath))) + return null + const before = await fileAt(root, from, oldPath ?? file) + const after = await readWorktreeFile(root, file) + if (before === null || after === null) return { path: file, before: '', after: '', binary: true } + return { + path: file, + before: normalizeEol(before), + after: normalizeEol(after), + binary: false + } +} + +export function clampCommitLimit(limit: number | undefined): number { + return Math.min(Math.max(Math.trunc(Number(limit) || REVIEW_COMMITS), 1), REVIEW_COMMITS_MAX) +} + +export async function reviewCommits(cwd: string, limit = REVIEW_COMMITS): Promise { + if (!cwd || !(await isGitAvailable())) return [] + const root = await repoRoot(cwd) + if (!root) return [] + const r = await git( + ['log', `-${clampCommitLimit(limit)}`, '--format=%H%x00%s%x00%an%x00%aI'], + root + ) + if (!r.ok) return [] + return r.stdout + .split('\n') + .filter((line) => line.trim() !== '') + .map((line) => { + const [sha, subject, author, date] = line.split('\0') + return { sha, subject: subject ?? '', author: author ?? '', date: date ?? '' } + }) + .filter((commit) => !!commit.sha) +} + +export async function stageFiles( + cwd: string, + files: string[] +): Promise<{ ok: boolean; error?: string }> { + const root = await repoRoot(cwd) + if (!root) return { ok: false, error: 'Not a repository.' } + if (files.length && files.some((file) => !reviewPath(root, file))) + return { ok: false, error: 'Path escapes repository.' } + const r = await git(files.length ? ['add', '--', ...files] : ['add', '-A'], root) + return r.ok ? { ok: true } : { ok: false, error: cleanGitError(r, 'Could not stage') } +} + +export async function unstageFiles( + cwd: string, + files: string[] +): Promise<{ ok: boolean; error?: string }> { + const root = await repoRoot(cwd) + if (!root) return { ok: false, error: 'Not a repository.' } + if (files.length && files.some((file) => !reviewPath(root, file))) + return { ok: false, error: 'Path escapes repository.' } + const hasHead = !!(await resolveCommit(root)) + const args = hasHead + ? files.length + ? ['restore', '--staged', '--', ...files] + : ['restore', '--staged', ':/'] + : files.length + ? ['rm', '--cached', '-f', '--', ...files] + : ['rm', '--cached', '-r', '-f', '--', '.'] + const r = await git(args, root) + return r.ok ? { ok: true } : { ok: false, error: cleanGitError(r, 'Could not unstage') } +} + +/** + * Discard visible changes. Unstaged restores from the index, preserving staged + * content. Staged discards only the indexed delta and never writes the + * worktree, so overlapping unstaged edits and staged additions/deletions/ + * renames remain recoverable as unstaged or untracked work. + */ +export async function revertFiles( + cwd: string, + files: string[], + scope: Extract = 'unstaged' +): Promise<{ ok: boolean; error?: string }> { + if (!files.length) return { ok: true } + const root = await repoRoot(cwd) + if (!root) return { ok: false, error: 'Not a repository.' } + if (files.some((file) => !reviewPath(root, file))) + return { ok: false, error: 'Path escapes repository.' } + + const paths = [...new Set(files)] + if (scope === 'staged') return revertStagedFiles(root, paths) + + const tracked: string[] = [] + const untracked: string[] = [] + for (const file of paths) { + if (await isTracked(root, file)) tracked.push(file) + else if (await isUntracked(root, file)) untracked.push(file) + } + + if (tracked.length) { + // The index is deliberately the source: discarding worktree edits must + // not erase an earlier staged version of the same path. This also works + // before the repository has its first commit. + const result = await git(['restore', '--worktree', '--', ...tracked], root) + if (!result.ok) return { ok: false, error: cleanGitError(result, 'Could not revert changes') } + } + + for (const file of untracked) { + try { + const abs = await safeWorktreePath(root, file) + if (!abs) return { ok: false, error: 'Path escapes repository.' } + await fs.rm(abs, { recursive: true, force: true }) + } catch (error) { + return { + ok: false, + error: error instanceof Error ? error.message : 'Could not delete file' + } + } + } + return { ok: true } +} + +async function revertStagedFiles( + root: string, + files: string[] +): Promise<{ ok: boolean; error?: string }> { + const hasHead = !!(await resolveCommit(root)) + // Snapshot the staged status first and restrict the operation to entries + // that are actually part of that delta. Besides avoiding pathspec failures + // for unrelated untracked inputs, this captures both sides of a rename even + // when a caller supplied only one of them. + const requested = new Set(files) + const affected = new Set() + for (const file of await reviewFilesAtRoot(root, 'staged')) { + if (!requested.has(file.path) && (!file.oldPath || !requested.has(file.oldPath))) continue + affected.add(file.path) + if (file.oldPath) affected.add(file.oldPath) + } + if (!affected.size) return { ok: true } + + // Crucially, neither command writes the worktree. A staged addition becomes + // untracked, a staged deletion becomes an unstaged deletion, and a staged + // rename leaves its current paths/content exactly where they are. + const args = hasHead + ? ['restore', '--source=HEAD', '--staged', '--', ...affected] + : ['rm', '--cached', '-r', '-f', '--', ...affected] + const result = await git(args, root) + if (!result.ok) return { ok: false, error: cleanGitError(result, 'Could not revert changes') } + return { ok: true } +} + // --------------------------------------------------------------------------- // Branch naming // --------------------------------------------------------------------------- diff --git a/src/main/services/session-review.ts b/src/main/services/session-review.ts new file mode 100644 index 0000000..ddbf372 --- /dev/null +++ b/src/main/services/session-review.ts @@ -0,0 +1,107 @@ +import { createHash } from 'node:crypto' +import path from 'node:path' +import type { ReviewDiff, ReviewFile } from '../../shared/api' +import * as repo from '../db/repo' +import * as git from './git' + +export interface SessionReviewRepo { + key: string + name?: string + cwd: string +} + +function ownerSessionId(sessionId: string): string { + try { + return repo.rootSessionId(sessionId) + } catch { + return sessionId + } +} + +function repoKey(cwd: string): string { + const resolved = path.resolve(cwd) + return process.platform === 'win32' ? resolved.toLowerCase() : resolved +} + +function baselineRef(sessionId: string, key: string): string { + const digest = createHash('sha256').update(key).digest('hex').slice(0, 24) + return `refs/roxy/sessions/${sessionId}/${digest}` +} + +export async function ensureSessionReviewBaselines( + sessionId: string, + repos: SessionReviewRepo[] +): Promise { + const ownerId = ownerSessionId(sessionId) + await Promise.all( + repos.map(async (entry) => { + const root = await git.repoRoot(entry.cwd) + if (!root) return + const key = repoKey(root) + const existing = repo.getSessionReviewBaseline(ownerId, key) + if (existing) { + await git.setReviewBaselineRef(root, existing.baselineRef, existing.baselineTree) + return + } + const tree = await git.snapshotWorktreeTree(root) + if (!tree) return + const ref = baselineRef(ownerId, key) + if (!(await git.setReviewBaselineRef(root, ref, tree))) return + const baseline = repo.addSessionReviewBaseline({ + sessionId: ownerId, + repoKey: key, + repoRoot: root, + baselineTree: tree, + baselineRef: ref + }) + if (baseline.baselineTree !== tree) + await git.setReviewBaselineRef(root, ref, baseline.baselineTree) + }) + ) +} + +export async function sessionReviewFiles( + sessionId: string, + repos: SessionReviewRepo[] +): Promise { + const ownerId = ownerSessionId(sessionId) + return ( + await Promise.all( + repos.map(async (entry) => { + const root = await git.repoRoot(entry.cwd) + if (!root) return [] + const baseline = repo.getSessionReviewBaseline(ownerId, repoKey(root)) + if (!baseline) return [] + const files = await git.reviewFilesFromTree(root, baseline.baselineTree) + return entry.name ? files.map((file) => ({ ...file, repo: entry.name })) : files + }) + ) + ).flat() +} + +export async function sessionReviewDiff( + sessionId: string, + entry: SessionReviewRepo, + file: string, + oldPath?: string +): Promise { + const ownerId = ownerSessionId(sessionId) + const root = await git.repoRoot(entry.cwd) + if (!root) return null + const baseline = repo.getSessionReviewBaseline(ownerId, repoKey(root)) + return baseline ? git.reviewDiffFromTree(root, baseline.baselineTree, file, oldPath) : null +} + +export async function deleteSessionReviewBaselines(sessionId: string): Promise { + let baselines: repo.SessionReviewBaseline[] + try { + baselines = repo.listSessionReviewBaselines(sessionId) + } catch { + return + } + await Promise.all( + baselines.map((baseline) => + git.deleteReviewBaselineRef(baseline.repoRoot, baseline.baselineRef).catch(() => undefined) + ) + ) +} diff --git a/src/main/services/session-turn.ts b/src/main/services/session-turn.ts index 79df555..f016c36 100644 --- a/src/main/services/session-turn.ts +++ b/src/main/services/session-turn.ts @@ -15,8 +15,10 @@ import { runAgentTurn } from '../harness' import { activeBackgroundSubChatIds } from './background-tasks' import { protectedSubChatIds } from './subagent-stream' import { setLabel as setBrowserLabel } from './browser' -import { sessionCwd } from './workspace' +import { discoverRepos, sessionCwd } from './workspace' import { materializePendingWorktree } from './worktree' +import { ensureSessionReviewBaselines } from './session-review' +import * as git from './git' import { markActivation, track, trackFeature, trackToolUse } from './track' import { beginTurn, finishTurn } from './turn-metrics' import { modelFamily, reportableAgent } from '../../shared/telemetry' @@ -29,6 +31,20 @@ import path from 'node:path' * touches the filesystem, and a turn that can't resolve a worktree should still * run — just without one. */ +async function reviewReposForSession(sessionId: string, cwd: string) { + const chat = repo.getChat(repo.rootSessionId(sessionId)) + if (chat?.repos?.length) { + return chat.repos.map((link) => ({ key: link.name, name: link.name, cwd: link.worktreePath })) + } + const root = await git.repoRoot(cwd) + if (root) return [{ key: root, cwd: root }] + return discoverRepos(cwd).roots.map((repoRoot) => ({ + key: path.basename(repoRoot), + name: path.basename(repoRoot), + cwd: repoRoot + })) +} + function safeSessionCwd(sessionId: string): string { try { return sessionCwd(sessionId) @@ -181,6 +197,14 @@ async function runTurn( // Where this session's tools run — its worktree when it has one, else the // project folder. The single resolver; never read workspace_path directly. const cwd = safeSessionCwd(input.sessionId) + try { + await ensureSessionReviewBaselines( + input.sessionId, + await reviewReposForSession(input.sessionId, cwd) + ) + } catch (e) { + console.warn('[review] could not capture the session baseline:', e) + } // Name this session's browser window after its project so concurrent windows // are tellable apart (a no-op until/unless the agent opens the browser). if (cwd) setBrowserLabel(input.sessionId, path.basename(cwd)) diff --git a/src/main/services/worktree.ts b/src/main/services/worktree.ts index 1d5aa45..31370e5 100644 --- a/src/main/services/worktree.ts +++ b/src/main/services/worktree.ts @@ -27,6 +27,7 @@ import { ensureDevPort } from './ports' import { startBackground, killSessionBackground } from '../harness' import { activeBackgroundSubChatIds, hasActiveBackgroundJobs } from './background-tasks' import { emitSessionsUpdated } from './session-events' +import { ensureSessionReviewBaselines } from './session-review' import type { WorktreeIntent } from '../../shared/types' /** @@ -149,6 +150,16 @@ export async function materializePendingWorktree(chatId: string): Promise ({ key: link.name, name: link.name, cwd: link.worktreePath })) + : [{ key: result.worktreePath, cwd: result.worktreePath }] + try { + await ensureSessionReviewBaselines(chatId, baselineRepos) + } catch (e) { + console.warn('[review] could not capture the worktree baseline:', e) + } + // Give the session its own dev port before the setup script runs, so an // install that builds against a port sees the right one. Allocation failure // (range exhausted) is not fatal — the session just has no reserved port. diff --git a/src/preload/index.ts b/src/preload/index.ts index 9637d3f..aaefb24 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -282,6 +282,15 @@ const roxy: RoxyApi = { renameBranch: (sessionId, to) => ipcRenderer.invoke(CHANNELS.gitRenameBranch, sessionId, to), pruneWorktrees: (cwd, dryRun) => ipcRenderer.invoke(CHANNELS.gitPruneWorktrees, cwd, dryRun) }, + review: { + files: (target) => ipcRenderer.invoke(CHANNELS.reviewFiles, target), + diff: (target, file) => ipcRenderer.invoke(CHANNELS.reviewDiff, target, file), + commits: (sessionId, repo, limit) => + ipcRenderer.invoke(CHANNELS.reviewCommits, sessionId, repo, limit), + stage: (target, files) => ipcRenderer.invoke(CHANNELS.reviewStage, target, files), + unstage: (target, files) => ipcRenderer.invoke(CHANNELS.reviewUnstage, target, files), + revert: (target, files) => ipcRenderer.invoke(CHANNELS.reviewRevert, target, files) + }, forge: { status: (cwd, force) => ipcRenderer.invoke(CHANNELS.forgeStatus, cwd, force), push: (cwd) => ipcRenderer.invoke(CHANNELS.forgePush, cwd), diff --git a/src/renderer/src/components/ChangesChip.tsx b/src/renderer/src/components/ChangesChip.tsx new file mode 100644 index 0000000..5db3910 --- /dev/null +++ b/src/renderer/src/components/ChangesChip.tsx @@ -0,0 +1,91 @@ +import { FileDiff } from 'lucide-react' +import { useEffect, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { api } from '../lib/api' +import { cn } from '../lib/cn' +import { GIT_POLL_MS } from '../lib/polling' + +export interface ReviewCounts { + files: number + additions: number + deletions: number +} + +/** Store-free review entry point. Its polling state cannot re-render the transcript. */ +export function ChangesChip({ + sessionId, + open, + onToggle, + onCounts +}: { + sessionId: string | null + open: boolean + onToggle: () => void + onCounts?: (counts: ReviewCounts | null) => void +}): JSX.Element | null { + const { t } = useTranslation() + const [snapshot, setSnapshot] = useState<{ + sessionId: string + counts: ReviewCounts + } | null>(null) + const counts = snapshot?.sessionId === sessionId ? snapshot.counts : null + + useEffect(() => { + if (!sessionId) { + setSnapshot(null) + onCounts?.(null) + return + } + let alive = true + let timer: ReturnType | undefined + if (open) return () => undefined + const load = async (): Promise => { + try { + const files = await api.review.files({ sessionId, scope: 'session' }) + if (!alive) return + const next = files.reduce( + (total, file) => ({ + files: total.files + 1, + additions: total.additions + file.additions, + deletions: total.deletions + file.deletions + }), + { files: 0, additions: 0, deletions: 0 } + ) + setSnapshot({ sessionId, counts: next }) + onCounts?.(next) + } catch { + // Keep the last honest count through a transient Git failure. + } finally { + if (alive) timer = setTimeout(() => void load(), GIT_POLL_MS) + } + } + void load() + return () => { + alive = false + if (timer) clearTimeout(timer) + } + }, [sessionId, open, onCounts]) + + if (!sessionId || !counts?.files) return null + + return ( + + ) +} diff --git a/src/renderer/src/components/ChatView.tsx b/src/renderer/src/components/ChatView.tsx index d501b52..95624b6 100644 --- a/src/renderer/src/components/ChatView.tsx +++ b/src/renderer/src/components/ChatView.tsx @@ -1,4 +1,4 @@ -import { useEffect, useLayoutEffect, useState } from 'react' +import { useEffect, useLayoutEffect, useState, type ReactNode } from 'react' import { Check, ChevronRight, @@ -61,7 +61,7 @@ import roxy from '../assets/roxy.png' * painter. */ -export function ChatView(): JSX.Element { +export function ChatView({ headerActions }: { headerActions?: ReactNode }): JSX.Element { const { t } = useTranslation() const messages = useRoxyStore((s) => s.messages) const messagesChatId = useRoxyStore((s) => s.messagesChatId) @@ -257,6 +257,7 @@ export function ChatView(): JSX.Element { {t('chat.settings')} )} + {headerActions} diff --git a/src/renderer/src/lib/polling.ts b/src/renderer/src/lib/polling.ts new file mode 100644 index 0000000..aefacf7 --- /dev/null +++ b/src/renderer/src/lib/polling.ts @@ -0,0 +1,2 @@ +/** Shared cadence for renderer surfaces that read live Git state. */ +export const GIT_POLL_MS = 5_000 diff --git a/src/renderer/src/locales/ar.json b/src/renderer/src/locales/ar.json index cc8b9fd..0e2aa91 100644 --- a/src/renderer/src/locales/ar.json +++ b/src/renderer/src/locales/ar.json @@ -236,8 +236,8 @@ "title": "خوادم MCP" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "كل النماذج مخفية. أعد بعضها في الإعدادات ← النماذج.", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "تعذر تحميل النماذج من models.dev — لا يزال بإمكانك الإرسال بالنموذج الحالي.", "loading": "جارٍ تحميل النماذج…", "noMatch": "لا توجد نماذج تطابق \"{{query}}\".", @@ -331,6 +331,45 @@ "viewing": "عرض", "waitingForPhone": "في انتظار اتصال هاتفك…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} انتهت", "failed": "{{count}} فشلت", diff --git a/src/renderer/src/locales/de.json b/src/renderer/src/locales/de.json index d767505..c06da53 100644 --- a/src/renderer/src/locales/de.json +++ b/src/renderer/src/locales/de.json @@ -236,8 +236,8 @@ "title": "MCP-Server" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "Alle Modelle sind ausgeblendet. Blenden Sie einige wieder ein unter Einstellungen → Modelle.", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "Modelle konnten nicht von models.dev geladen werden – Sie können weiterhin mit dem aktuellen Modell senden.", "loading": "Modelle werden geladen…", "noMatch": "Keine Modelle stimmen mit „{{query}}“ überein.", @@ -331,6 +331,45 @@ "viewing": "Anzeigen", "waitingForPhone": "Warte auf die Verbindung Ihres Telefons…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} abgeschlossen", "failed": "{{count}} fehlgeschlagen", diff --git a/src/renderer/src/locales/default.json b/src/renderer/src/locales/default.json index 0810f1d..498f7d0 100644 --- a/src/renderer/src/locales/default.json +++ b/src/renderer/src/locales/default.json @@ -310,6 +310,45 @@ "errorFallback": "Something went wrong.", "tryAgain": "Try again" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptySession": "No changes from this session", + "emptyCommit": "That commit changed nothing", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "session": "Session", + "sessionHint": "Changes since this chat started", + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "segmentTitle": "Processes this session is running", "menuHeader": "Processes", diff --git a/src/renderer/src/locales/es.json b/src/renderer/src/locales/es.json index e8c5b43..767a753 100644 --- a/src/renderer/src/locales/es.json +++ b/src/renderer/src/locales/es.json @@ -236,8 +236,8 @@ "title": "Servidores MCP" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "Todos los modelos están ocultos. Vuelve a habilitar algunos en Ajustes → Modelos.", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "No se pudieron cargar los modelos de models.dev; aún puedes enviar con el modelo actual.", "loading": "Cargando los modelos…", "noMatch": "Ningún modelo coincide con «{{query}}».", @@ -331,6 +331,45 @@ "viewing": "Viendo", "waitingForPhone": "Esperando a que se conecte tu móvil…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} terminados", "failed": "{{count}} con errores", diff --git a/src/renderer/src/locales/fr.json b/src/renderer/src/locales/fr.json index 0b33441..a198dc2 100644 --- a/src/renderer/src/locales/fr.json +++ b/src/renderer/src/locales/fr.json @@ -236,8 +236,8 @@ "title": "Serveurs MCP" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "Tous les modèles sont masqués. Réactivez-en dans Paramètres → Modèles.", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "Impossible de charger les modèles depuis models.dev — vous pouvez toujours envoyer avec le modèle actuel.", "loading": "Chargement des modèles…", "noMatch": "Aucun modèle ne correspond à « {{query}} ».", @@ -331,6 +331,45 @@ "viewing": "Affichage", "waitingForPhone": "En attente de la connexion de votre téléphone…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} terminés", "failed": "{{count}} échecs", diff --git a/src/renderer/src/locales/hi.json b/src/renderer/src/locales/hi.json index 6af57a7..03d6e85 100644 --- a/src/renderer/src/locales/hi.json +++ b/src/renderer/src/locales/hi.json @@ -236,8 +236,8 @@ "title": "MCP सर्वर" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "हर मॉडल छिपा हुआ है। कुछ को सेटिंग्स → मॉडल में वापस लाएँ।", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "models.dev से मॉडल लोड नहीं हो सके — आप अभी भी वर्तमान मॉडल के साथ भेज सकते हैं।", "loading": "मॉडल लोड हो रहे हैं…", "noMatch": "“{{query}}” से कोई मॉडल मेल नहीं खाता।", @@ -331,6 +331,45 @@ "viewing": "देख रहा है", "waitingForPhone": "आपके फ़ोन के कनेक्ट होने का इंतज़ार कर रहा है…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} हो गया", "failed": "{{count}} विफल", diff --git a/src/renderer/src/locales/ja.json b/src/renderer/src/locales/ja.json index c8bb65a..3f9c757 100644 --- a/src/renderer/src/locales/ja.json +++ b/src/renderer/src/locales/ja.json @@ -236,8 +236,8 @@ "title": "MCP サーバー" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "すべてのモデルが非表示になっています。設定 → モデルで一部を元に戻してください。", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "models.dev からモデルを読み込めませんでした — 現在のモデルで送信することはできます。", "loading": "モデルを読み込み中…", "noMatch": "「{{query}}」に一致するモデルはありません。", @@ -331,6 +331,45 @@ "viewing": "表示中", "waitingForPhone": "スマートフォンの接続を待っています…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}}件完了", "failed": "{{count}}件失敗", diff --git a/src/renderer/src/locales/pt.json b/src/renderer/src/locales/pt.json index 6ac1eae..4ee9f14 100644 --- a/src/renderer/src/locales/pt.json +++ b/src/renderer/src/locales/pt.json @@ -236,8 +236,8 @@ "title": "Servidores MCP" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "Todos os modelos estão ocultos. Reative alguns em Definições → Modelos.", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "Não foi possível carregar modelos de models.dev — você ainda pode enviar com o modelo atual.", "loading": "Carregando modelos…", "noMatch": "Nenhum modelo corresponde a “{{query}}”.", @@ -331,6 +331,45 @@ "viewing": "Visualizando", "waitingForPhone": "Aguardando seu telefone conectar…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} concluído", "failed": "{{count}} falhou", diff --git a/src/renderer/src/locales/ru.json b/src/renderer/src/locales/ru.json index 34c5247..3141612 100644 --- a/src/renderer/src/locales/ru.json +++ b/src/renderer/src/locales/ru.json @@ -236,8 +236,8 @@ "title": "MCP-серверы" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "Все модели скрыты. Верните некоторые в Настройки → Модели.", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "Не удалось загрузить модели с models.dev — вы все еще можете отправлять запросы с текущей моделью.", "loading": "Загрузка моделей…", "noMatch": "Нет моделей, соответствующих «{{query}}».", @@ -331,6 +331,45 @@ "viewing": "Просмотр", "waitingForPhone": "Ожидание подключения вашего телефона…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} завершено", "failed": "{{count}} сбой", diff --git a/src/renderer/src/locales/zh.json b/src/renderer/src/locales/zh.json index b998d8a..816cc81 100644 --- a/src/renderer/src/locales/zh.json +++ b/src/renderer/src/locales/zh.json @@ -236,8 +236,8 @@ "title": "MCP 服务器" }, "models": { - "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "allHidden": "所有模型都已隐藏。在设置 → 模型中恢复一些模型。", + "copilotUnavailable": "No available GitHub Copilot model is selected. Choose an enabled model, or check your account's model access and connection, then try again.", "loadFailed": "无法从 models.dev 加载模型 — 您仍然可以使用当前模型发送。", "loading": "正在加载模型…", "noMatch": "没有模型与“{{query}}”匹配。", @@ -331,6 +331,45 @@ "viewing": "正在查看", "waitingForPhone": "正在等待您的手机连接…" }, + "review": { + "binary": "Binary or very large file - not shown.", + "chipTitle": "Review these changes", + "diffReadFailed": "Could not read this file", + "discardFile": "Discard this file's changes", + "emptyBranch": "This branch matches its base", + "emptyCommit": "That commit changed nothing", + "emptySession": "No changes from this session", + "emptyStaged": "Nothing staged", + "emptyUnstaged": "No uncommitted changes", + "fileUpdateFailed": "Could not update this file", + "files_one": "{{count}} file", + "files_other": "{{count}} files", + "loadFailed": "Could not read changes", + "loadingDiff": "Loading diff...", + "noSession": "Open this from a session to review its changes", + "pickCommit": "Pick a commit", + "pickCommitPlaceholder": "Pick a commit...", + "reading": "Reading changes...", + "refresh": "Refresh", + "revertConfirm": "Sure?", + "scope": { + "branch": "Branch", + "branchHint": "Everything on this branch", + "commit": "Commit", + "commitHint": "A single commit", + "session": "Session", + "sessionHint": "Changes since this chat started", + "staged": "Staged", + "stagedHint": "What's in the index", + "unstaged": "Uncommitted", + "unstagedHint": "Edited but not staged" + }, + "stage": "Stage", + "stageAll": "Stage all", + "unstage": "Unstage", + "unstageAll": "Unstage all", + "updateFailed": "Could not update changes" + }, "services": { "done": "{{count}} 个已完成", "failed": "{{count}} 个失败", diff --git a/src/renderer/src/review/ReviewFileRow.tsx b/src/renderer/src/review/ReviewFileRow.tsx new file mode 100644 index 0000000..0951312 --- /dev/null +++ b/src/renderer/src/review/ReviewFileRow.tsx @@ -0,0 +1,220 @@ +import { useEffect, useMemo, useState } from 'react' +import { ChevronRight, Loader2, RotateCcw } from 'lucide-react' +import { useTranslation } from 'react-i18next' +import type { ReviewDiff, ReviewFile, ReviewScope, ReviewTarget } from '@shared/api' +import { DiffViewer } from '../components/diff/DiffViewer' +import { api } from '../lib/api' +import { cn } from '../lib/cn' + +export function ReviewFileRow({ + file, + scope, + target, + onChanged +}: { + file: ReviewFile + scope: ReviewScope + target: ReviewTarget | null + onChanged: () => Promise +}): JSX.Element { + const { t } = useTranslation() + const [open, setOpen] = useState(false) + const [diff, setDiff] = useState(null) + const [busy, setBusy] = useState(false) + const [error, setError] = useState(null) + const [confirmRevert, setConfirmRevert] = useState(false) + const rowTarget = useMemo( + () => (target ? { ...target, repo: file.repo ?? target.repo, oldPath: file.oldPath } : null), + [target, file.repo, file.oldPath] + ) + const fingerprint = useMemo( + () => + JSON.stringify([ + file.path, + file.oldPath, + file.additions, + file.deletions, + file.status, + file.binary, + rowTarget + ]), + [file, rowTarget] + ) + const [diffFingerprint, setDiffFingerprint] = useState('') + const visibleDiff = diffFingerprint === fingerprint ? diff : null + const paths = useMemo( + () => (file.status === 'renamed' && file.oldPath ? [file.path, file.oldPath] : [file.path]), + [file.path, file.oldPath, file.status] + ) + + useEffect(() => { + setDiff(null) + setError(null) + setConfirmRevert(false) + }, [fingerprint]) + + useEffect(() => { + if (!open || visibleDiff || file.binary || !rowTarget) return + let alive = true + void api.review + .diff(rowTarget, file.path) + .then((next) => { + if (!alive) return + if (next) { + setDiff(next) + setDiffFingerprint(fingerprint) + } else setError(t('review.diffReadFailed')) + }) + .catch(() => alive && setError(t('review.diffReadFailed'))) + return () => { + alive = false + } + }, [open, visibleDiff, file.binary, file.path, fingerprint, rowTarget, t]) + + const act = async (fn: () => Promise<{ ok: boolean; error?: string }>): Promise => { + setBusy(true) + setError(null) + try { + const result = await fn() + if (!result.ok) return setError(t('review.fileUpdateFailed')) + await onChanged() + } catch { + setError(t('review.fileUpdateFailed')) + } finally { + setBusy(false) + } + } + + const canAct = (scope === 'unstaged' || scope === 'staged') && !!rowTarget + const lineCount = visibleDiff + ? Math.max(visibleDiff.before.split('\n').length, visibleDiff.after.split('\n').length) + : 0 + const viewport = typeof window === 'undefined' ? 720 : window.innerHeight + const diffHeight = Math.min( + Math.max(240, 120 + lineCount * 18), + Math.max(320, viewport - 220), + 760 + ) + + return ( +
+
+ + + {file.additions > 0 && +{file.additions}} + {file.deletions > 0 && -{file.deletions}} + + {canAct && ( + + {busy ? ( + + ) : ( + <> + + {scope === 'unstaged' && ( + + )} + + )} + + )} +
+ {open && ( +
+ {file.binary || visibleDiff?.binary ? ( +

{t('review.binary')}

+ ) : error ? ( +

{error}

+ ) : !visibleDiff ? ( +

+ {t('review.loadingDiff')} +

+ ) : ( + + )} +
+ )} + {!open && error &&

{error}

} +
+ ) +} + +function StatusMark({ status }: { status: ReviewFile['status'] }): JSX.Element { + const mark = { + added: { character: 'A', className: 'text-success' }, + untracked: { character: 'U', className: 'text-success' }, + modified: { character: 'M', className: 'text-warning' }, + deleted: { character: 'D', className: 'text-danger' }, + renamed: { character: 'R', className: 'text-text-muted' }, + copied: { character: 'C', className: 'text-text-muted' } + }[status] + return ( + + {mark.character} + + ) +} diff --git a/src/renderer/src/review/ReviewPane.tsx b/src/renderer/src/review/ReviewPane.tsx new file mode 100644 index 0000000..922ec77 --- /dev/null +++ b/src/renderer/src/review/ReviewPane.tsx @@ -0,0 +1,248 @@ +import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react' +import { FileDiff, Loader2, RefreshCw } from 'lucide-react' +import { useTranslation } from 'react-i18next' +import { REVIEW_COMMITS } from '@shared/api' +import type { ReviewCommit, ReviewFile, ReviewScope, ReviewTarget } from '@shared/api' +import { api } from '../lib/api' +import { cn } from '../lib/cn' +import { GIT_POLL_MS } from '../lib/polling' +import { ReviewFileRow } from './ReviewFileRow' + +const SCOPES: ReviewScope[] = ['session', 'unstaged', 'staged', 'branch', 'commit'] + +export function ReviewPane({ + sessionId, + className, + action +}: { + sessionId: string | null + className?: string + action?: ReactNode +}): JSX.Element { + const { t } = useTranslation() + const [scope, setScope] = useState('session') + const [commitKey, setCommitKey] = useState('') + const [files, setFiles] = useState(null) + const [commits, setCommits] = useState(null) + const [commitsError, setCommitsError] = useState(false) + const [busy, setBusy] = useState(false) + const [error, setError] = useState(null) + const loadSeq = useRef(0) + const selectedCommit = useMemo( + () => commits?.find((candidate) => commitKeyOf(candidate) === commitKey), + [commits, commitKey] + ) + const target = useMemo( + () => + sessionId + ? { + sessionId, + scope, + commit: selectedCommit?.sha, + repo: scope === 'commit' ? selectedCommit?.repo : undefined + } + : null, + [sessionId, scope, selectedCommit] + ) + + const load = useCallback(async (): Promise => { + const seq = ++loadSeq.current + if (!target) return setFiles([]) + if (target.scope === 'commit' && !target.commit) return setFiles([]) + try { + const next = await api.review.files(target) + if (seq === loadSeq.current) { + setFiles(next) + setError(null) + } + } catch { + if (seq === loadSeq.current) { + setError(t('review.loadFailed')) + setFiles((current) => current ?? []) + } + } + }, [target, t]) + + useEffect(() => { + setFiles(null) + void load() + const timer = setInterval(() => void load(), GIT_POLL_MS) + return () => clearInterval(timer) + }, [load]) + + useEffect(() => { + if (scope !== 'commit' || commits || !sessionId) return + let alive = true + void api.review + .commits(sessionId, undefined, REVIEW_COMMITS) + .then((next) => { + if (!alive) return + setCommits(next) + setCommitsError(false) + }) + .catch(() => { + if (!alive) return + setCommits([]) + setCommitsError(true) + }) + return () => { + alive = false + } + }, [scope, commits, sessionId, t]) + + useEffect(() => { + setCommits(null) + setCommitsError(false) + setCommitKey('') + }, [sessionId]) + + const bulk = async ( + fn: (target: ReviewTarget, files: string[]) => Promise<{ ok: boolean; error?: string }> + ): Promise => { + if (!target) return + setBusy(true) + setError(null) + try { + const result = await fn(target, []) + if (!result.ok) setError(t('review.updateFailed')) + await load() + } catch { + setError(t('review.updateFailed')) + } finally { + setBusy(false) + } + } + + const additions = files?.reduce((count, file) => count + file.additions, 0) ?? 0 + const deletions = files?.reduce((count, file) => count + file.deletions, 0) ?? 0 + + return ( +
+
+ {SCOPES.map((id) => ( + + ))} + + {additions > 0 && +{additions}} + {deletions > 0 && -{deletions}} + {!!files?.length && ( + {t('review.files', { count: files.length })} + )} + +
+ {(scope === 'unstaged' || scope === 'staged') && !!files?.length && ( + + )} + + {action} +
+
+ + {scope === 'commit' && ( +
+ + {commitsError &&

{t('review.loadFailed')}

} +
+ )} + + {error && ( +

+ {error} +

+ )} +
+ {!sessionId ? ( + + {t('review.noSession')} + + ) : !files ? ( + + {t('review.reading')} + + ) : !files.length ? ( + + {t(emptyKey(scope, selectedCommit?.sha))} + + ) : ( + files.map((file) => ( + + )) + )} +
+
+ ) +} + +function emptyKey(scope: ReviewScope, commit: string | undefined) { + if (scope === 'commit') + return commit ? ('review.emptyCommit' as const) : ('review.pickCommit' as const) + if (scope === 'session') return 'review.emptySession' as const + if (scope === 'staged') return 'review.emptyStaged' as const + if (scope === 'branch') return 'review.emptyBranch' as const + return 'review.emptyUnstaged' as const +} + +function commitKeyOf(commit: ReviewCommit): string { + return `${commit.repo ?? ''}:${commit.sha}` +} + +function Empty({ children }: { children: ReactNode }): JSX.Element { + return ( +
+ {children} +
+ ) +} diff --git a/src/renderer/src/routes/Chat.tsx b/src/renderer/src/routes/Chat.tsx index 535697b..8bb7e30 100644 --- a/src/renderer/src/routes/Chat.tsx +++ b/src/renderer/src/routes/Chat.tsx @@ -1,12 +1,124 @@ -import { memo } from 'react' +import { + memo, + useCallback, + useEffect, + useMemo, + useRef, + useState, + type MouseEvent as ReactMouseEvent +} from 'react' +import { X } from 'lucide-react' +import { useTranslation } from 'react-i18next' import { Sidebar } from '../components/Sidebar' import { ChatView } from '../components/ChatView' +import { ChangesChip } from '../components/ChangesChip' +import { ReviewPane } from '../review/ReviewPane' +import { useRoxyStore } from '../lib/store' + +const MIN_REVIEW_WIDTH = 360 +const MAX_REVIEW_WIDTH = 1000 +const REVIEW_SIBLING_MIN_WIDTH = 840 function Chat(): JSX.Element { + const { t } = useTranslation() + const activeChatId = useRoxyStore((state) => state.activeChatId) + const [reviewOpen, setReviewOpen] = useState(false) + const [reviewWidth, setReviewWidth] = useState(520) + const [contentWidth, setContentWidth] = useState(0) + const contentRef = useRef(null) + const reviewOverlay = contentWidth < REVIEW_SIBLING_MIN_WIDTH + // Deliberately subscribe only to the active id. Transcript activity updates + // chat rows frequently; reading the stable owner from the snapshot keeps + // those updates from walking the ChatView subtree. + const reviewSessionId = useMemo(() => { + const chats = useRoxyStore.getState().chats + const activeChat = chats.find((chat) => chat.id === activeChatId) ?? null + const owner = + activeChat?.kind === 'sub' && activeChat.parentId + ? (chats.find((chat) => chat.id === activeChat.parentId) ?? null) + : activeChat + return owner?.workspacePath ? owner.id : null + }, [activeChatId]) + + useEffect(() => setReviewOpen(false), [activeChatId]) + + useEffect(() => { + const element = contentRef.current + if (!element) return + const measure = (): void => setContentWidth(element.getBoundingClientRect().width) + measure() + const observer = new ResizeObserver(measure) + observer.observe(element) + return () => observer.disconnect() + }, []) + + const toggleReview = useCallback(() => setReviewOpen((open) => !open), []) + const startResize = (event: ReactMouseEvent): void => { + if (reviewOverlay) return + event.preventDefault() + const startX = event.clientX + const startWidth = reviewWidth + const onMove = (move: MouseEvent): void => { + const viewportMax = Math.max(MIN_REVIEW_WIDTH, contentWidth - 320) + const next = startWidth - (move.clientX - startX) + setReviewWidth(Math.min(Math.max(MIN_REVIEW_WIDTH, next), MAX_REVIEW_WIDTH, viewportMax)) + } + const onUp = (): void => { + document.body.style.cursor = '' + window.removeEventListener('mousemove', onMove) + window.removeEventListener('mouseup', onUp) + } + document.body.style.cursor = 'col-resize' + window.addEventListener('mousemove', onMove) + window.addEventListener('mouseup', onUp) + } + return (
- +
+ + } + /> + {reviewOpen && reviewSessionId && ( + + )} +
) } diff --git a/src/shared/api.ts b/src/shared/api.ts index 142a31a..303e021 100644 --- a/src/shared/api.ts +++ b/src/shared/api.ts @@ -324,6 +324,63 @@ export interface MultiSyncOutcome { error?: string } +/** Which set of Git changes the review pane is showing. */ +export type GitReviewScope = 'unstaged' | 'staged' | 'branch' | 'commit' +export type ReviewScope = 'session' | GitReviewScope + +/** How many commits the picker requests, and the largest limit main accepts. */ +export const REVIEW_COMMITS = 30 +export const REVIEW_COMMITS_MAX = 100 + +/** How a file came to be in the review. */ +export type ReviewFileStatus = 'added' | 'modified' | 'deleted' | 'renamed' | 'copied' | 'untracked' + +/** One changed file, as listed in the review pane. */ +export interface ReviewFile { + /** Repo-relative path, using Git's forward-slash spelling. */ + path: string + /** Previous path, set for renames and copies. */ + oldPath?: string + status: ReviewFileStatus + additions: number + deletions: number + /** Binary or too large to render safely. */ + binary: boolean + /** Repository name in a multi-repo session. */ + repo?: string +} + +/** Both sides of one file, ready for the canvas diff viewer. */ +export interface ReviewDiff { + path: string + before: string + after: string + binary: boolean +} + +/** A commit offered in the review pane's commit picker. */ +export interface ReviewCommit { + sha: string + subject: string + author: string + /** ISO 8601. */ + date: string + /** Repository name in a multi-repo session. */ + repo?: string +} + +/** Which repository and scope a review operation targets. */ +export interface ReviewTarget { + sessionId: string + scope: ReviewScope + /** Required to disambiguate a file in a multi-repo session. */ + repo?: string + /** Required by commit scope. */ + commit?: string + /** Previous path for a rename/copy diff. */ + oldPath?: string +} + export interface PruneWorktreesResult { ok: boolean candidates: { path: string; branch: string | null }[] @@ -1176,6 +1233,15 @@ export interface RoxyApi { */ pruneWorktrees(cwd: string, dryRun?: boolean): Promise } + /** Session-keyed Git changes and mutations used by the review pane. */ + review: { + files(target: ReviewTarget): Promise + diff(target: ReviewTarget, file: string): Promise + commits(sessionId: string, repo?: string, limit?: number): Promise + stage(target: ReviewTarget, files: string[]): Promise<{ ok: boolean; error?: string }> + unstage(target: ReviewTarget, files: string[]): Promise<{ ok: boolean; error?: string }> + revert(target: ReviewTarget, files: string[]): Promise<{ ok: boolean; error?: string }> + } remote: { /** Mint a room on roxy.gg + open the host relay socket for a session. */ start(input: RemoteStartInput): Promise diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index a2af080..2db7f57 100644 --- a/src/shared/ipc.ts +++ b/src/shared/ipc.ts @@ -254,6 +254,13 @@ export const CHANNELS = { gitPruneWorktrees: 'git:prune-worktrees', gitRenameBranch: 'git:rename-branch', + reviewFiles: 'review:files', + reviewDiff: 'review:diff', + reviewCommits: 'review:commits', + reviewStage: 'review:stage', + reviewUnstage: 'review:unstage', + reviewRevert: 'review:revert', + /** Forge = the git host (GitHub/Azure DevOps/GitLab/Bitbucket) behind `origin`. */ forgeStatus: 'forge:status', forgePush: 'forge:push', diff --git a/test/review.ts b/test/review.ts new file mode 100644 index 0000000..5785334 --- /dev/null +++ b/test/review.ts @@ -0,0 +1,417 @@ +/** Smoke tests the review Git layer against real temporary repositories. */ +import { app } from 'electron' +import { promises as fs } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import * as review from '../src/main/services/git' +import { codeReviewReposForOwner, runTool } from '../src/main/harness/tools' + +let failed = 0 + +function check(condition: unknown, message: string): void { + if (condition) console.log(`PASS: ${message}`) + else { + failed++ + console.error(`FAIL: ${message}`) + } +} + +function text(value: string): string { + return value.replace(/\r\n?/g, '\n').trim() +} + +async function command(cwd: string, ...args: string[]): Promise { + const result = await review.git(args, cwd) + if (!result.ok) throw new Error(`${args.join(' ')}: ${result.stderr}`) + return result.stdout.trim() +} + +async function write(cwd: string, file: string, contents: string | Uint8Array): Promise { + const target = path.join(cwd, file) + await fs.mkdir(path.dirname(target), { recursive: true }) + await fs.writeFile(target, contents) +} + +async function main(): Promise { + await app.whenReady() + const routed = codeReviewReposForOwner( + { + repos: [ + { name: 'api', worktreePath: '/work/api' }, + { name: 'web', worktreePath: '/work/web' } + ] + }, + '/fallback' + ) + check( + routed.map((item) => `${item.name}:${item.cwd}`).join('|') === 'api:/work/api|web:/work/web', + 'code_review routes a composite session to every persisted repo' + ) + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), 'roxy-review-')) + try { + await command(cwd, 'init') + await command(cwd, 'config', 'user.name', 'Review Test') + await command(cwd, 'config', 'user.email', 'review@example.test') + await write(cwd, 'alpha file.txt', 'one\ntwo\n') + await write(cwd, 'rename-old.txt', 'rename me\n') + await write(cwd, 'binary.bin', Uint8Array.from([0, 1, 2, 3])) + await command(cwd, 'add', '.') + await command(cwd, 'commit', '-m', 'base') + + const baseBranch = await command(cwd, 'branch', '--show-current') + await command(cwd, 'switch', '-c', 'review-branch') + await command(cwd, 'config', 'branch.review-branch.roxy-base', baseBranch) + await write(cwd, 'branch commit.txt', 'committed on branch\n') + await command(cwd, 'add', '--', 'branch commit.txt') + await command(cwd, 'commit', '-m', 'branch change') + await write(cwd, 'branch commit.txt', 'committed on branch\nstaged local line\n') + await command(cwd, 'add', '--', 'branch commit.txt') + await write( + cwd, + 'branch commit.txt', + 'committed on branch\nstaged local line\nunstaged local line\n' + ) + await write(cwd, 'branch staged local.txt', 'staged only\n') + await command(cwd, 'add', '--', 'branch staged local.txt') + await write(cwd, 'branch unstaged local.txt', 'unstaged only\n') + const branchFiles = await review.reviewFiles(cwd, 'branch') + const branchDiff = await review.reviewDiff(cwd, 'branch', 'branch commit.txt') + const branchPatch = await runTool('code_review', { scope: 'branch' }, { cwd }) + check( + branchFiles.some((file) => file.path === 'branch commit.txt') && + !branchFiles.some((file) => file.path.includes('local.txt')), + 'branch files compare merge-base to HEAD and exclude local staged/unstaged changes' + ) + check( + branchDiff?.after === 'committed on branch\n' && !branchDiff.after.includes('local line'), + 'branch diff reads committed HEAD content instead of the worktree' + ) + check( + branchPatch.ok && + branchPatch.output.includes('branch commit.txt') && + !branchPatch.output.includes('branch staged local.txt') && + !branchPatch.output.includes('branch unstaged local.txt'), + 'code_review branch scope excludes local staged/unstaged changes' + ) + await command(cwd, 'restore', '--staged', '--', 'branch staged local.txt') + await command(cwd, 'restore', '--staged', '--worktree', '--', 'branch commit.txt') + await fs.rm(path.join(cwd, 'branch staged local.txt')) + await fs.rm(path.join(cwd, 'branch unstaged local.txt')) + + const subdir = path.join(cwd, 'workspace', 'nested') + const subdirFile = 'workspace/subdir review.txt' + await fs.mkdir(subdir, { recursive: true }) + await write(cwd, subdirFile, 'before\n') + await command(cwd, 'add', '--', subdirFile) + await command(cwd, 'commit', '-m', 'subdirectory fixture') + await write(cwd, subdirFile, 'after\n') + const subdirFiles = await review.reviewFiles(subdir, 'unstaged') + const subdirDiff = await review.reviewDiff(subdir, 'unstaged', subdirFile) + check( + subdirFiles.some((file) => file.path === subdirFile), + 'reviewFiles from a subdirectory returns repository-relative paths' + ) + check( + subdirDiff?.before === 'before\n' && subdirDiff.after === 'after\n', + 'reviewDiff from a subdirectory reads repository-relative before and after content' + ) + check( + (await review.stageFiles(subdir, [subdirFile])).ok, + 'stageFiles works from a subdirectory' + ) + check( + (await command(cwd, 'status', '--short', '--', subdirFile)) === + 'M "workspace/subdir review.txt"', + 'stageFiles from a subdirectory stages the repository-relative path' + ) + check( + (await review.unstageFiles(subdir, [subdirFile])).ok, + 'unstageFiles works from a subdirectory' + ) + check( + (await review.reviewFiles(subdir, 'unstaged')).some((file) => file.path === subdirFile) && + !(await review.reviewFiles(subdir, 'staged')).some((file) => file.path === subdirFile), + 'unstageFiles from a subdirectory unstages the repository-relative path' + ) + await review.stageFiles(subdir, [subdirFile]) + await write(cwd, subdirFile, 'after\nunstaged\n') + check( + (await review.revertFiles(subdir, [subdirFile], 'unstaged')).ok && + text(await fs.readFile(path.join(cwd, subdirFile), 'utf8')) === 'after', + 'unstaged revert from a subdirectory restores the indexed version' + ) + check( + (await review.revertFiles(subdir, [subdirFile], 'staged')).ok && + text(await fs.readFile(path.join(cwd, subdirFile), 'utf8')) === 'after', + 'staged revert from a subdirectory preserves the worktree version' + ) + check( + !(await review.stageFiles(subdir, ['../escape.txt'])).ok && + (await review.reviewDiff(subdir, 'unstaged', '../escape.txt')) === null, + 'repository-relative traversal remains rejected from a subdirectory' + ) + const subdirTool = await runTool('code_review', { scope: 'unstaged' }, { cwd: subdir }) + check( + subdirTool.ok && subdirTool.output.includes(subdirFile), + 'code_review works when its cwd is a subdirectory' + ) + await command(cwd, 'restore', '--worktree', '--', subdirFile) + + await write(cwd, 'alpha file.txt', 'one\nstaged\n') + await command(cwd, 'add', '--', 'alpha file.txt') + await write(cwd, 'alpha file.txt', 'one\nstaged\nunstaged\n') + const stagedBefore = await command(cwd, 'show', ':alpha file.txt') + const revertedUnstaged = await review.revertFiles(cwd, ['alpha file.txt'], 'unstaged') + const worktreeAfter = await fs.readFile(path.join(cwd, 'alpha file.txt'), 'utf8') + const stagedAfter = await command(cwd, 'show', ':alpha file.txt') + check(revertedUnstaged.ok, 'tracked unstaged revert succeeds') + check(text(worktreeAfter) === stagedBefore, 'unstaged revert restores the indexed version') + check(stagedAfter === stagedBefore, 'unstaged revert preserves staged changes') + + await write(cwd, 'alpha file.txt', 'one\nstaged\nstill unstaged\n') + const revertedStaged = await review.revertFiles(cwd, ['alpha file.txt'], 'staged') + const headText = await command(cwd, 'show', 'HEAD:alpha file.txt') + const indexText = await command(cwd, 'show', ':alpha file.txt') + const diskText = (await fs.readFile(path.join(cwd, 'alpha file.txt'), 'utf8')).trim() + check(revertedStaged.ok, 'staged revert succeeds') + check( + indexText === headText && text(diskText) === 'one\nstaged\nstill unstaged', + 'staged revert resets only the index and preserves overlapping worktree edits' + ) + check( + (await command(cwd, 'status', '--short', '--', 'alpha file.txt')) === 'M "alpha file.txt"', + 'staged revert exposes the preserved edit as unstaged' + ) + await command(cwd, 'restore', '--worktree', '--', 'alpha file.txt') + + await write(cwd, 'staged addition.txt', 'temporary\n') + await command(cwd, 'add', '--', 'staged addition.txt') + const revertedAddition = await review.revertFiles(cwd, ['staged addition.txt'], 'staged') + check(revertedAddition.ok, 'staged addition revert succeeds') + check( + text(await fs.readFile(path.join(cwd, 'staged addition.txt'), 'utf8')) === 'temporary' && + (await command(cwd, 'status', '--short', '--', 'staged addition.txt')) === + '?? "staged addition.txt"', + 'staged addition revert keeps the file as untracked' + ) + await fs.rm(path.join(cwd, 'staged addition.txt')) + + await write(cwd, 'unrelated untracked.txt', 'keep me\n') + const ignoredUnrelated = await review.revertFiles(cwd, ['unrelated untracked.txt'], 'staged') + check(ignoredUnrelated.ok, 'staged revert ignores a path not present in HEAD or index') + check( + text(await fs.readFile(path.join(cwd, 'unrelated untracked.txt'), 'utf8')) === 'keep me', + 'staged revert does not delete an unrelated untracked file' + ) + await fs.rm(path.join(cwd, 'unrelated untracked.txt')) + + await command(cwd, 'rm', '--', 'alpha file.txt') + const revertedDeletion = await review.revertFiles(cwd, ['alpha file.txt'], 'staged') + check(revertedDeletion.ok, 'staged deletion revert succeeds') + check( + (await command(cwd, 'status', '--short', '--', 'alpha file.txt')) === 'D "alpha file.txt"', + 'staged deletion revert leaves the deletion unstaged' + ) + check( + !(await fs.stat(path.join(cwd, 'alpha file.txt')).catch(() => null)), + 'staged deletion revert does not recreate deleted worktree content' + ) + await command(cwd, 'restore', '--worktree', '--', 'alpha file.txt') + + await command(cwd, 'rm', '--', 'alpha file.txt') + await write(cwd, 'alpha file.txt', 'recreated after staged deletion\n') + const revertedRecreatedDeletion = await review.revertFiles(cwd, ['alpha file.txt'], 'staged') + check(revertedRecreatedDeletion.ok, 'recreated staged deletion revert succeeds') + check( + text(await fs.readFile(path.join(cwd, 'alpha file.txt'), 'utf8')) === + 'recreated after staged deletion' && + text(await command(cwd, 'show', ':alpha file.txt')) === headText, + 'staged deletion revert preserves recreated worktree content while restoring the index' + ) + await command(cwd, 'restore', '--worktree', '--', 'alpha file.txt') + + await command(cwd, 'mv', 'rename-old.txt', 'rename new.txt') + const staged = await review.reviewFiles(cwd, 'staged') + const renamed = staged.find((file) => file.status === 'renamed') + check( + renamed?.oldPath === 'rename-old.txt' && renamed.path === 'rename new.txt', + 'rename retains old and new paths' + ) + const renameDiff = renamed + ? await review.reviewDiff(cwd, 'staged', renamed.path, undefined, renamed.oldPath) + : null + check( + renameDiff?.before === renameDiff?.after && renameDiff?.before === 'rename me\n', + 'rename diff reads the old path before and new path after' + ) + const revertedRename = await review.revertFiles( + cwd, + ['rename new.txt', 'rename-old.txt'], + 'staged' + ) + check(revertedRename.ok, 'staged rename revert succeeds') + check( + (await command(cwd, 'status', '--short', '--', 'rename-old.txt', 'rename new.txt')) === + 'D rename-old.txt\n?? "rename new.txt"', + 'staged rename revert preserves the worktree paths as unstaged/untracked' + ) + check( + !(await fs.stat(path.join(cwd, 'rename-old.txt')).catch(() => null)) && + text(await fs.readFile(path.join(cwd, 'rename new.txt'), 'utf8')) === 'rename me', + 'staged rename revert does not lose the renamed path or its content' + ) + check( + text(await command(cwd, 'show', ':rename-old.txt')) === 'rename me', + 'staged rename revert restores the original path in the index' + ) + await fs.rm(path.join(cwd, 'rename new.txt')) + await command(cwd, 'restore', '--worktree', '--', 'rename-old.txt') + + await write(cwd, 'untracked space.txt', 'new\nfile\n') + const unstaged = await review.reviewFiles(cwd, 'unstaged') + check( + unstaged.some((file) => file.path === 'untracked space.txt' && file.status === 'untracked'), + 'unstaged includes untracked paths with spaces' + ) + check( + unstaged.some((file) => file.path === 'untracked space.txt' && file.additions === 2), + 'untracked line count is useful' + ) + + check( + review.clampCommitLimit(0) === 30 && review.clampCommitLimit(500) === 100, + 'commit limits are clamped' + ) + check((await review.revsForScope(cwd, 'commit')) === null, 'commit scope requires a commit') + check( + (await review.reviewDiff(cwd, 'unstaged', '../escape.txt')) === null, + 'diff rejects paths outside the repository' + ) + check( + !(await review.stageFiles(cwd, ['../escape.txt'])).ok, + 'mutations reject paths outside the repository' + ) + + const invalidTool = await runTool('code_review', { scope: 'wat' }, { cwd }) + const missingCommit = await runTool('code_review', { scope: 'commit' }, { cwd }) + const untrackedTool = await runTool('code_review', { scope: 'unstaged' }, { cwd }) + check(!invalidTool.ok, 'code_review validates its scope') + check(!missingCommit.ok, 'code_review requires a commit for commit scope') + check( + untrackedTool.output.includes('untracked space.txt'), + 'code_review includes untracked text files' + ) + + await fs.rm(path.join(cwd, 'untracked space.txt')) + const originalIndex = await fs.readFile(path.join(cwd, '.git', 'index')) + const baseline = await review.snapshotWorktreeTree(cwd) + check(!!baseline, 'session baseline snapshots the current worktree') + check( + Buffer.compare(originalIndex, await fs.readFile(path.join(cwd, '.git', 'index'))) === 0, + 'session baseline does not modify the real index' + ) + await write(cwd, 'session file.txt', 'first\nsecond\n') + const sessionFiles = baseline ? await review.reviewFilesFromTree(cwd, baseline) : [] + check( + sessionFiles.some( + (file) => + file.path === 'session file.txt' && file.status === 'added' && file.additions === 2 + ), + 'session review includes new worktree files' + ) + const sessionDiff = baseline + ? await review.reviewDiffFromTree(cwd, baseline, 'session file.txt') + : null + check( + sessionDiff?.before === '' && sessionDiff.after === 'first\nsecond\n', + 'session diff renders a newly added file' + ) + await command(cwd, 'add', '--', 'session file.txt') + await command(cwd, 'commit', '-m', 'session commit') + check( + baseline + ? (await review.reviewFilesFromTree(cwd, baseline)).some( + (file) => file.path === 'session file.txt' + ) + : false, + 'session changes remain visible after commit' + ) + const afterCommit = await review.snapshotWorktreeTree(cwd) + check( + afterCommit === (await command(cwd, 'rev-parse', 'HEAD^{tree}')), + 'session snapshot matches committed HEAD when the worktree is clean' + ) + + const dirtyBaseline = await review.snapshotWorktreeTree(cwd) + await write(cwd, 'preexisting dirty.txt', 'already here\n') + const dirtyStart = await review.snapshotWorktreeTree(cwd) + await write(cwd, 'created later.txt', 'later\n') + const sinceDirtyStart = dirtyStart ? await review.reviewFilesFromTree(cwd, dirtyStart) : [] + check( + !sinceDirtyStart.some((file) => file.path === 'preexisting dirty.txt') && + sinceDirtyStart.some((file) => file.path === 'created later.txt'), + 'session baseline excludes changes that predated the session' + ) + check(!!dirtyBaseline, 'session snapshots also work from a dirty repository') + await fs.rm(path.join(cwd, 'preexisting dirty.txt')) + await fs.rm(path.join(cwd, 'created later.txt')) + + const deletionBaseline = await review.snapshotWorktreeTree(cwd) + await fs.rm(path.join(cwd, 'session file.txt')) + const deletionDiff = deletionBaseline + ? await review.reviewDiffFromTree(cwd, deletionBaseline, 'session file.txt') + : null + check( + deletionDiff?.before === 'first\nsecond\n' && deletionDiff.after === '', + 'session diff renders deleted files' + ) + + const unborn = await fs.mkdtemp(path.join(os.tmpdir(), 'roxy-review-unborn-')) + try { + await command(unborn, 'init') + await write(unborn, 'new file.txt', 'staged version\n') + await command(unborn, 'add', '--', 'new file.txt') + await write(unborn, 'new file.txt', 'unstaged version\n') + const unbornUnstaged = await review.revertFiles(unborn, ['new file.txt'], 'unstaged') + check(unbornUnstaged.ok, 'unstaged revert succeeds in a repository without HEAD') + check( + text(await fs.readFile(path.join(unborn, 'new file.txt'), 'utf8')) === 'staged version' && + text(await command(unborn, 'show', ':new file.txt')) === 'staged version', + 'unstaged revert without HEAD restores the worktree from the index and keeps it staged' + ) + + await write(unborn, 'new file.txt', 'preserved worktree\n') + const unbornStaged = await review.revertFiles(unborn, ['new file.txt'], 'staged') + check(unbornStaged.ok, 'staged revert succeeds in a repository without HEAD') + check( + text(await fs.readFile(path.join(unborn, 'new file.txt'), 'utf8')) === + 'preserved worktree' && + (await command(unborn, 'status', '--short', '--', 'new file.txt')) === + '?? "new file.txt"', + 'staged revert without HEAD preserves the worktree and leaves the file untracked' + ) + + await write(unborn, 'purely untracked.txt', 'delete me\n') + const removedUntracked = await review.revertFiles( + unborn, + ['purely untracked.txt'], + 'unstaged' + ) + check( + removedUntracked.ok && + !(await fs.stat(path.join(unborn, 'purely untracked.txt')).catch(() => null)), + 'unstaged revert without HEAD deletes a purely untracked file' + ) + } finally { + await fs.rm(unborn, { recursive: true, force: true }) + } + } finally { + await fs.rm(cwd, { recursive: true, force: true }) + } + app.exit(failed ? 1 : 0) +} + +void main().catch((error) => { + console.error(error) + app.exit(1) +}) diff --git a/test/session-review.ts b/test/session-review.ts new file mode 100644 index 0000000..41336a3 --- /dev/null +++ b/test/session-review.ts @@ -0,0 +1,99 @@ +/** Integration tests for durable per-session review baselines. */ +import { app } from 'electron' +import { promises as fs } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { getDb } from '../src/main/db/database' +import * as repo from '../src/main/db/repo' +import * as git from '../src/main/services/git' +import { + deleteSessionReviewBaselines, + ensureSessionReviewBaselines, + sessionReviewDiff, + sessionReviewFiles +} from '../src/main/services/session-review' + +let failed = 0 + +function check(condition: unknown, label: string): void { + if (condition) console.log(`PASS: ${label}`) + else { + failed++ + console.error(`FAIL: ${label}`) + } +} + +async function command(cwd: string, ...args: string[]): Promise { + const result = await git.git(args, cwd) + if (!result.ok) throw new Error(result.stderr || `git ${args.join(' ')} failed`) + return result.stdout.trim() +} + +async function write(cwd: string, file: string, content: string): Promise { + const target = path.join(cwd, file) + await fs.mkdir(path.dirname(target), { recursive: true }) + await fs.writeFile(target, content) +} + +async function main(): Promise { + const userData = await fs.mkdtemp(path.join(os.tmpdir(), 'roxy-session-review-db-')) + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), 'roxy-session-review-repo-')) + app.setPath('userData', userData) + try { + await command(cwd, 'init') + await command(cwd, 'config', 'user.email', 'review@example.com') + await command(cwd, 'config', 'user.name', 'Review Test') + await write(cwd, 'existing.txt', 'before\n') + await command(cwd, 'add', '.') + await command(cwd, 'commit', '-m', 'base') + + const chat = repo.createChat({ title: 'Session review', workspacePath: cwd }) + await ensureSessionReviewBaselines(chat.id, [{ key: cwd, cwd }]) + await write(cwd, 'existing.txt', 'after\n') + await write(cwd, 'new.txt', 'new\n') + + const files = await sessionReviewFiles(chat.id, [{ key: cwd, cwd }]) + check(files.length === 2, 'service returns changes since the session baseline') + check( + files.some((file) => file.path === 'new.txt'), + 'service includes added files' + ) + const diff = await sessionReviewDiff(chat.id, { key: cwd, cwd }, 'existing.txt') + check( + diff?.before === 'before\n' && diff.after === 'after\n', + 'service returns baseline and current contents' + ) + + const baseline = repo.listSessionReviewBaselines(chat.id)[0] + check(!!baseline, 'baseline metadata is persisted') + check( + baseline ? (await command(cwd, 'cat-file', '-t', baseline.baselineRef)) === 'tree' : false, + 'private Git ref keeps the baseline tree alive' + ) + + const sub = repo.createChat({ + title: 'Subagent', + kind: 'sub', + parentId: chat.id, + workspacePath: cwd + }) + const subFiles = await sessionReviewFiles(sub.id, [{ key: cwd, cwd }]) + check(subFiles.length === files.length, 'subagents use their owning session baseline') + + await deleteSessionReviewBaselines(chat.id) + check( + baseline ? !(await git.git(['show-ref', '--verify', baseline.baselineRef], cwd)).ok : false, + 'deleting a session removes its private Git baseline ref' + ) + } finally { + getDb().close() + await fs.rm(cwd, { recursive: true, force: true }) + await fs.rm(userData, { recursive: true, force: true }) + } + app.exit(failed ? 1 : 0) +} + +void main().catch((error) => { + console.error(error) + app.exit(1) +}) diff --git a/test/smoke.ts b/test/smoke.ts index c92d1cb..2dd2b66 100644 --- a/test/smoke.ts +++ b/test/smoke.ts @@ -1305,7 +1305,15 @@ async function main(): Promise { // Every table the app depends on must come back, not just the ones a // previously-reported bug happened to name. - for (const table of ['projects', 'usage', 'queue', 'mcp_servers', 'settings', 'activity']) { + for (const table of [ + 'projects', + 'usage', + 'queue', + 'mcp_servers', + 'settings', + 'activity', + 'session_review_baselines' + ]) { const db = healthy() db.exec(`DROP TABLE ${table}`) check(`self-heal: ${table} is missing before repair`, !tablesOf(db).includes(table)) @@ -1443,6 +1451,31 @@ async function main(): Promise { } } + // ---- session review baselines ---- + { + const session = repo.createChat({ title: 'Review baseline', workspacePath: tmp }) + const baseline = repo.addSessionReviewBaseline({ + sessionId: session.id, + repoKey: 'main', + repoRoot: tmp, + baselineTree: 'a'.repeat(40), + baselineRef: `refs/roxy/sessions/${session.id}/main` + }) + check( + 'session review baseline persists', + repo.getSessionReviewBaseline(session.id, 'main')?.baselineTree === baseline.baselineTree + ) + check( + 'session review baselines list by owner', + repo.listSessionReviewBaselines(session.id).length === 1 + ) + repo.removeChat(session.id) + check( + 'session review baselines cascade on chat deletion', + repo.listSessionReviewBaselines(session.id).length === 0 + ) + } + // ---- sessionCwd (the one working-directory resolver) ---- { check('sessionCwd: an unknown chat resolves to empty', sessionCwd('nope') === '')