diff --git a/apps/cli-docs/src/fragments/commands/issue.md b/apps/cli-docs/src/fragments/commands/issue.md index 0cdf0d463..602f63b00 100644 --- a/apps/cli-docs/src/fragments/commands/issue.md +++ b/apps/cli-docs/src/fragments/commands/issue.md @@ -432,7 +432,9 @@ The chat asks Seer to investigate the issue and generate code changes for review. In a terminal, the CLI shows new messages, asks Seer's questions, and lets you approve or reject each proposed file change. Approval lets Seer continue generating the fix. Completed fixes preview -all generated diffs; local files are not modified and no pull request is created. +all generated diffs. Without delivery flags, JSON and non-interactive sessions +only preview the fix. In a terminal, the CLI then offers Apply locally, Create PR, +Both, and the default Skip. Cancelling this optional choice also skips delivery. The organization needs Seer coding access, linked repositories, and code mappings. Interactive updates require a signed-in Sentry user. Use @@ -485,3 +487,39 @@ returns the run ID and resume command because Seer may still be working. Runs that fail, cancel, or time out return a nonzero exit status with their JSON result. A resumed completed run without patches explains that no code changes were produced and offers a new full fix. + +### Apply a fix locally or create pull requests + +Request delivery explicitly for scripts and agent callers: + +```bash +sentry issue fix my-org/FRONT-123 --run-id --apply --json +sentry issue fix my-org/FRONT-123 --run-id --pr --json +sentry issue fix my-org/FRONT-123 --run-id --apply --pr --json +``` + +`--apply` validates the entire patch batch against the current Git checkout before +writing. The checkout's remote must match the patch repository. Target files must +have no dirty or untracked conflicts; unrelated local changes are preserved. +Unsupported paths, symlink escapes, metadata/diff mismatches, and patches for +multiple repositories are refused. The CLI uses `git apply --check` before +applying and does not commit or push local changes. +Paths that differ only by case or Unicode normalization are refused to avoid +ambiguous writes on macOS and Windows. + +`--pr` requests a pull request for each affected repository and waits for its +repository-specific PR state, independently of the chat's completed status. +Existing completed PRs are reused, and PR creation already in progress is awaited. +An uncertain create request is never repeated automatically; inspect the run in +Sentry before retrying. + +Delivery requires a completed fix with patches and no pending input. The CLI +refreshes the pinned chat before writing, verifying that its patches still match +the reviewed result. A new question or changed patch pauses delivery. Incomplete +incremental diffs require a merged diff before delivery. Analysis and delivery +share one timeout budget, and a delivery choice after its deadline +never starts a mutation. + +JSON output remains a single result. Successful local application adds +`applied_files`; delivery failures add `delivery_errors` and return a nonzero exit +status while retaining generated diffs, run ID, and any completed PR URLs. diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md index e2cd75f50..afdcb3e1d 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md @@ -195,6 +195,8 @@ Generate and review code changes using Seer AI **Flags:** - `-f, --fresh - Bypass cache, re-detect projects, and fetch fresh data` +- `--apply - Apply the validated patch batch to the current checkout` +- `--pr - Create or await pull requests for affected repositories` - `--instruction - Context for a new fix` - `--run-id - Resume this Seer run` - `--response - JSON file with input_id and response_data (requires --run-id)` @@ -209,6 +211,10 @@ sentry issue fix @latest --instruction "Keep the existing API compatible" sentry issue fix my-org/FRONT-123 --json sentry issue fix my-org/FRONT-123 --run-id --json sentry issue fix my-org/FRONT-123 --run-id --response answer.json --json + +sentry issue fix my-org/FRONT-123 --run-id --apply --json +sentry issue fix my-org/FRONT-123 --run-id --pr --json +sentry issue fix my-org/FRONT-123 --run-id --apply --pr --json ``` ### `sentry issue view ` diff --git a/packages/cli/src/commands/issue/fix.ts b/packages/cli/src/commands/issue/fix.ts index 5c9a8924a..20844b03a 100644 --- a/packages/cli/src/commands/issue/fix.ts +++ b/packages/cli/src/commands/issue/fix.ts @@ -13,7 +13,8 @@ import { CommandOutput } from "../../lib/formatters/output.js"; import { handleSeerApiError } from "../../lib/formatters/seer.js"; import { createSeerBlockRenderer, - formatSeerFix, + createSeerFixRenderer, + promptSeerDelivery, promptSeerInput, } from "../../lib/formatters/seer-fix.js"; import { @@ -21,12 +22,18 @@ import { FRESH_ALIASES, FRESH_FLAG, } from "../../lib/list-command.js"; +import { + deliverSeerFix, + type FixDeliveryResult, +} from "../../lib/seer-fix-delivery.js"; import { runSeerFix } from "../../lib/seer-fix.js"; import { issueIdPositional, resolveOrgAndIssueId } from "./utils.js"; type FixFlags = { readonly json: boolean; readonly fresh: boolean; + readonly apply: boolean; + readonly pr: boolean; readonly instruction?: string; readonly "run-id"?: string; readonly response?: string; @@ -74,13 +81,23 @@ export const fixCommand = buildCommand({ docs: { brief: "Generate and review code changes using Seer AI", fullDescription: - "Generate code changes for an issue using Seer AI. Review questions and proposed file changes in an interactive terminal.\n\nJSON and non-interactive sessions pause with a run ID and pending input. Resume using --run-id and --response with a JSON file containing input_id and response_data.\n\nCompleted fixes preview diffs. No files are changed locally. Requires Seer coding access, linked repositories, and a signed-in Sentry user.\n\nEach invocation starts a new interactive chat. Use --run-id to resume a specific chat; the issue argument resolves its organization. --instruction supplies context for a new fix.\n\nExamples:\n sentry issue fix @latest\n sentry issue fix my-org/CLI-G --instruction 'Preserve compatibility'\n sentry issue fix my-org/CLI-G --json\n sentry issue fix my-org/CLI-G --run-id --response answer.json --json", + "Generate code changes for an issue using Seer AI. Review questions and proposed file changes in an interactive terminal.\n\nJSON and non-interactive sessions pause with a run ID and pending input. Resume using --run-id and --response with a JSON file containing input_id and response_data.\n\nCompleted fixes preview diffs. Use --apply to apply the validated patch batch locally or --pr to create pull requests. In an interactive terminal, choose apply, PR, both, or the default Skip after reviewing the preview. JSON and non-interactive sessions deliver only with explicit flags. Requires Seer coding access, linked repositories, and a signed-in Sentry user.\n\nEach invocation starts a new interactive chat. Use --run-id to resume a specific chat; the issue argument resolves its organization. --instruction supplies context for a new fix.\n\nExamples:\n sentry issue fix @latest\n sentry issue fix my-org/CLI-G --instruction 'Preserve compatibility'\n sentry issue fix my-org/CLI-G --json\n sentry issue fix my-org/CLI-G --run-id --response answer.json --json", }, - output: { human: formatSeerFix }, + output: { human: createSeerFixRenderer }, parameters: { positional: issueIdPositional, flags: { fresh: FRESH_FLAG, + apply: { + kind: "boolean", + brief: "Apply the validated patch batch to the current checkout", + default: false, + }, + pr: { + kind: "boolean", + brief: "Create or await pull requests for affected repositories", + default: false, + }, instruction: { kind: "parsed", parse: String, @@ -127,6 +144,7 @@ export const fixCommand = buildCommand({ command: "fix", }); org = resolved.org; + const deadline = Date.now() + flags.timeout * 1000; const result = await runSeerFix({ org, issueId: resolved.issueId, @@ -145,7 +163,52 @@ export const fixCommand = buildCommand({ if (["ERROR", "CANCELLED", "TIMEOUT"].includes(result.status)) { throw new OutputError(result); } - yield new CommandOutput(result); + const canDeliver = + result.status === "COMPLETED" && + result.pending_user_input === null && + result.patches.length > 0; + let choice = { apply: flags.apply, pr: flags.pr }; + let previewed = false; + if (canDeliver && interactive && !choice.apply && !choice.pr) { + yield new CommandOutput(result); + previewed = true; + choice = await promptSeerDelivery(); + } + if (!(canDeliver && (choice.apply || choice.pr))) { + if (!previewed) { + yield new CommandOutput(result); + } + return; + } + // JSON callers receive one aggregate document. Human output previews before writes. + if (!previewed && !flags.json) { + yield new CommandOutput(result); + } + const remaining = deadline - Date.now(); + if (remaining <= 0 || abort.signal.aborted) { + const timedOut: FixDeliveryResult = { + ...result, + status: abort.signal.aborted ? "CANCELLED" : "TIMEOUT", + next_action: `Delivery did not start. Review the fix with sentry issue fix ${issueArg} --run-id ${result.run_id}.`, + }; + throw new OutputError(timedOut); + } + const delivered = await deliverSeerFix({ + org, + result, + cwd: this.cwd, + apply: choice.apply, + pr: choice.pr, + timeoutMs: remaining, + signal: abort.signal, + }); + if ( + ["ERROR", "CANCELLED", "TIMEOUT"].includes(delivered.status) || + (delivered.delivery_errors?.length ?? 0) > 0 + ) { + throw new OutputError(delivered); + } + yield new CommandOutput(delivered); } catch (error) { if (error instanceof ApiError) { throw handleSeerApiError(error.status, error.detail, org); diff --git a/packages/cli/src/lib/formatters/seer-fix.ts b/packages/cli/src/lib/formatters/seer-fix.ts index 82d1804c3..2ebd800dd 100644 --- a/packages/cli/src/lib/formatters/seer-fix.ts +++ b/packages/cli/src/lib/formatters/seer-fix.ts @@ -9,6 +9,8 @@ import { } from "../../types/seer-fix.js"; import type { Writer } from "../../types/index.js"; import type { ExplorerBlock } from "../../types/seer-fix.js"; +import type { FixDeliveryResult } from "../seer-fix-delivery.js"; +import type { HumanRenderer } from "./output.js"; import { logger } from "../logger.js"; import { escapeMarkdownInline, renderMarkdown } from "./markdown.js"; @@ -130,3 +132,71 @@ export function createSeerBlockRenderer( } }; } + +export type SeerDeliveryChoice = { apply: boolean; pr: boolean }; + +/** Delivery is optional; selecting or cancelling the default keeps the fix read-only. */ +export async function promptSeerDelivery(): Promise { + const selection = await logger + .withTag("issue.fix") + .prompt("What would you like to do with this fix?", { + type: "select", + initial: "Skip", + cancel: "symbol", + options: ["Apply locally", "Create PR", "Both", "Skip"], + }); + return { + apply: selection === "Apply locally" || selection === "Both", + pr: selection === "Create PR" || selection === "Both", + }; +} + +function formatDeliverySummary( + result: FixDeliveryResult, + includeContext: boolean, +): string { + const lines: string[] = []; + if (includeContext) { + lines.push(`# Delivery`, `Status: ${escapeMarkdownInline(result.status)}`); + } + if (result.applied_files && result.applied_files.length > 0) { + lines.push( + "Applied locally:", + ...result.applied_files.map((path) => `- ${escapeMarkdownInline(path)}`), + ); + } + if (includeContext) { + for (const url of result.pr_urls) { + lines.push(url); + } + } + const errors = result.delivery_errors ?? []; + for (const error of errors) { + if (includeContext || error !== result.error) { + lines.push(`Delivery failed: ${escapeMarkdownInline(error)}`); + } + } + if (includeContext && result.error && !errors.includes(result.error)) { + lines.push(escapeMarkdownInline(result.error)); + } + if (includeContext && result.next_action) { + lines.push("", result.next_action); + } + return lines.length > 0 ? renderMarkdown(lines.join("\n\n")) : ""; +} + +/** Each invocation previews its diffs once, followed only by delivery summaries. */ +export function createSeerFixRenderer(): HumanRenderer { + let previewed = false; + return { + render(result) { + if (previewed) { + return formatDeliverySummary(result, true); + } + previewed = true; + return [formatSeerFix(result), formatDeliverySummary(result, false)] + .filter(Boolean) + .join("\n\n"); + }, + }; +} diff --git a/packages/cli/src/lib/seer-fix-delivery.ts b/packages/cli/src/lib/seer-fix-delivery.ts new file mode 100644 index 000000000..f7e12c3fb --- /dev/null +++ b/packages/cli/src/lib/seer-fix-delivery.ts @@ -0,0 +1,338 @@ +/** Deliver completed Seer changes to a local checkout or repository pull requests. */ +import { setTimeout as sleep } from "node:timers/promises"; +import type { ExplorerSession, FixResult } from "../types/seer-fix.js"; +import { apiRequestToRegion, getSeerExplorerState } from "./api-client.js"; +import { redactCredentialText } from "./credential-redaction.js"; +import { CliError } from "./errors.js"; +import { resolveOrgRegion } from "./region.js"; +import { extractFixPatches } from "./seer-fix.js"; +import { applySeerPatches } from "./seer-patch.js"; + +/** Preserve the generated fix when delivery succeeds only partially or fails. */ +export type FixDeliveryResult = FixResult & { + /** Repository-relative paths successfully changed in the local checkout. */ + applied_files?: string[]; + /** Delivery failures, with recognizable credentials removed. */ + delivery_errors?: string[]; +}; + +/** Explicit delivery choices for a completed fix. Neither action is implicit. */ +export type FixDeliveryOptions = { + /** Organization that owns the pinned Seer conversation. */ + org: string; + /** Generated fix to deliver; must be completed and have no pending approval. */ + result: FixResult; + /** Directory inside the target checkout, used only for local application. */ + cwd: string; + /** Apply the complete patch batch to the local working tree. */ + apply: boolean; + /** Create or await a PR for each affected repository. */ + pr: boolean; + /** Total operation budget in milliseconds; defaults to six minutes. */ + timeoutMs?: number; + /** Interval between safe PR state reads; defaults to two seconds. */ + pollIntervalMs?: number; + /** Caller cancellation signal, combined with the total operation budget. */ + signal?: AbortSignal; +}; + +type RepoPrState = NonNullable[string]; + +/** Bound even region resolution or clients that have not yet observed cancellation. */ +async function withAbort( + promise: Promise, + signal: AbortSignal, +): Promise { + let abort: (() => void) | undefined; + const cancelled = new Promise((_resolve, reject) => { + abort = () => reject(signal.reason); + if (signal.aborted) { + abort(); + } else { + signal.addEventListener("abort", abort, { once: true }); + } + }); + try { + return await Promise.race([promise, cancelled]); + } finally { + if (abort) { + signal.removeEventListener("abort", abort); + } + } +} + +/** Compare complete patch metadata without depending on JSON object key order. */ +function patchSnapshot(patches: FixResult["patches"]): string { + const files = patches.map(({ repo_name, diff, patch }) => ({ + repo_name, + diff, + patch, + })); + files.sort( + (left, right) => + left.repo_name.localeCompare(right.repo_name) || + left.patch.path.localeCompare(right.patch.path), + ); + return JSON.stringify(files, (_key, value: unknown) => { + if (value !== null && typeof value === "object" && !Array.isArray(value)) { + return Object.fromEntries( + Object.entries(value as Record).sort( + ([left], [right]) => left.localeCompare(right), + ), + ); + } + return value; + }); +} + +/** Raw patches are per-edit increments; repeated files require an authoritative merged diff. */ +function requireUnambiguousPatches(session: ExplorerSession): void { + const blocks = session.blocks ?? []; + if ( + blocks.some( + (block) => + block.merged_file_patches !== undefined && + block.merged_file_patches !== null, + ) + ) { + return; + } + const seen = new Set(); + for (const block of blocks) { + for (const patch of block.file_patches ?? []) { + const key = `${patch.repo_name}:${patch.patch.path}`; + if (seen.has(key)) { + throw new CliError( + "Seer returned repeated incremental edits to one file without merged diffs. Inspect the full changes in Sentry and wait for merged patches before requesting delivery.", + ); + } + seen.add(key); + } + } +} + +/** Read repository completion independently of the conversation's overall status. */ +function isPrComplete(state: RepoPrState | undefined, repo: string): boolean { + if (!state) { + return false; + } + if (state.repo_name !== repo) { + throw new CliError( + `PR state does not match repository ${repo}; inspect the run in Sentry.`, + ); + } + if (state.pr_creation_status === "error") { + throw new CliError( + `PR creation for ${repo} failed: ${state.pr_creation_error ?? "Inspect the repository state in Sentry."}`, + ); + } + if (state.pr_creation_status === "completed") { + if (!state.pr_url) { + throw new CliError( + `PR creation for ${repo} completed without a PR URL; inspect the run in Sentry.`, + ); + } + return true; + } + if (state.pr_creation_status === "creating") { + return false; + } + if ( + state.pr_creation_status !== null && + state.pr_creation_status !== undefined + ) { + throw new CliError( + `Unknown PR creation state for ${repo}; inspect the run in Sentry before retrying.`, + ); + } + return Boolean(state.pr_url); +} + +/** + * Apply changes and/or create repository PRs only after a completed approved fix. + * Each PR request is submitted once with retries disabled. Fresh state reads avoid + * duplicating existing or in-progress PRs. Errors retain patches and prior delivery. + * @param options - Pinned fix result and explicitly requested delivery actions. + * @returns Original fix plus applied files, PR URLs, and any delivery failure. + */ +export async function deliverSeerFix( + options: FixDeliveryOptions, +): Promise { + const delivered: FixDeliveryResult = { + ...options.result, + pr_urls: [...options.result.pr_urls], + }; + if (!options.apply && !options.pr) { + return delivered; + } + const timeoutMs = Math.ceil(options.timeoutMs ?? 360_000); + const validBudget = + Number.isFinite(timeoutMs) && timeoutMs > 0 && timeoutMs <= 0xffffffff; + const timeoutSignal = AbortSignal.timeout(validBudget ? timeoutMs : 1); + const signal = options.signal + ? AbortSignal.any([options.signal, timeoutSignal]) + : timeoutSignal; + const deadline = Date.now() + timeoutMs; + const assertActive = (): void => { + signal.throwIfAborted(); + if (Date.now() >= deadline) { + throw new CliError( + "Seer fix delivery timed out. Inspect PR state in Sentry before retrying.", + ); + } + }; + try { + if (!validBudget) { + throw new CliError( + "Delivery timeout must be a positive finite number of milliseconds.", + ); + } + if ( + options.result.status !== "COMPLETED" || + options.result.pending_user_input !== null || + options.result.patches.length === 0 + ) { + throw new CliError( + "Delivery requires a completed Seer fix with code changes and no pending user approval.", + ); + } + const readState = async (): Promise => { + assertActive(); + const session = await withAbort( + getSeerExplorerState(options.org, options.result.run_id, { signal }), + signal, + ); + assertActive(); + if (!session) { + throw new CliError( + "Cannot find the pinned Seer conversation; inspect the run in Sentry before retrying delivery.", + ); + } + delivered.pr_urls = [ + ...new Set([ + ...delivered.pr_urls, + ...Object.values(session.repo_pr_states ?? {}).flatMap((state) => + state.pr_url ? [state.pr_url] : [], + ), + ]), + ]; + if (session.pending_user_input) { + delivered.pending_user_input = session.pending_user_input; + throw new CliError( + "Seer is waiting for user input; resume the conversation before requesting delivery.", + ); + } + return session; + }; + const expectedPatches = patchSnapshot(options.result.patches); + const assertSnapshot = ( + session: ExplorerSession, + allowProcessing = false, + ): void => { + requireUnambiguousPatches(session); + if ( + session.status !== "COMPLETED" && + !(allowProcessing && session.status === "PROCESSING") + ) { + throw new CliError( + "The pinned Seer conversation is no longer completed; resume it before requesting delivery.", + ); + } + if (patchSnapshot(extractFixPatches(session)) !== expectedPatches) { + throw new CliError( + "Seer code changes changed since this result was generated; inspect the current diffs and request delivery again.", + ); + } + }; + let current = await readState(); + assertSnapshot(current); + assertActive(); + if (options.apply) { + const applied = await applySeerPatches( + options.result.patches, + options.cwd, + { signal, deadline }, + ); + delivered.applied_files = applied.applied; + assertActive(); + } + if (options.pr) { + const repositories = [ + ...new Set(options.result.patches.map((patch) => patch.repo_name)), + ]; + const pending = new Set(); + for (const [index, repo] of repositories.entries()) { + if (index > 0 || options.apply) { + current = await readState(); + assertSnapshot(current, pending.size > 0); + } + const state = current.repo_pr_states?.[repo]; + if (isPrComplete(state, repo)) { + continue; + } + pending.add(repo); + if (state?.pr_creation_status === "creating") { + continue; + } + const region = await withAbort(resolveOrgRegion(options.org), signal); + assertActive(); + try { + await withAbort( + apiRequestToRegion( + region, + `/organizations/${encodeURIComponent(options.org)}/seer/explorer-update/${encodeURIComponent(String(options.result.run_id))}/`, + { + method: "POST", + retry: false, + signal, + body: { payload: { type: "create_pr", repo_name: repo } }, + }, + ), + signal, + ); + } catch (error) { + const reason = + error instanceof Error ? error.message : "Request failed."; + throw new CliError( + `PR creation request for ${repo} failed and was not repeated: ${reason} Inspect PR state in Sentry because the outcome may be uncertain.`, + ); + } + } + while (pending.size > 0) { + const session = await readState(); + for (const repo of pending) { + if (isPrComplete(session.repo_pr_states?.[repo], repo)) { + pending.delete(repo); + } + } + if (pending.size > 0) { + await sleep( + Math.max( + 1, + Math.min(options.pollIntervalMs ?? 2000, deadline - Date.now()), + ), + undefined, + { signal }, + ); + } + } + } + return delivered; + } catch (error) { + const message = options.signal?.aborted + ? "Seer fix delivery was cancelled. Inspect PR state in Sentry before retrying." + : timeoutSignal.aborted + ? "Seer fix delivery timed out. Inspect PR state in Sentry before retrying." + : error instanceof Error + ? error.message + : "Seer fix delivery failed."; + const safeMessage = redactCredentialText(message); + return { + ...delivered, + status: "ERROR", + error: safeMessage, + delivery_errors: [...(delivered.delivery_errors ?? []), safeMessage], + next_action: `Inspect the generated diffs and PR state in Sentry, then resume the issue fix with --run-id ${options.result.run_id}.`, + }; + } +} diff --git a/packages/cli/src/lib/seer-patch.ts b/packages/cli/src/lib/seer-patch.ts new file mode 100644 index 000000000..f2d10ca75 --- /dev/null +++ b/packages/cli/src/lib/seer-patch.ts @@ -0,0 +1,502 @@ +/** Validate Seer patches and apply a complete batch to its matching Git checkout. */ +import { execFileSync } from "node:child_process"; +import { lstat, realpath } from "node:fs/promises"; +import { join } from "node:path"; +import { CliError, ValidationError } from "./errors.js"; +import { parseRemoteUrl } from "./git.js"; + +/** Structural subset of an Explorer file patch required for local application. */ +export type SeerPatch = { + /** Repository identity, in owner/name form. */ + repo_name: string; + /** One complete unified/Git diff for this file. */ + diff: string; + /** File identity and operation reported by Seer. */ + patch: { + /** Repository-relative file path (source or target for a rename). */ + path: string; + /** Addition, modification (including rename), or deletion. */ + type: "A" | "M" | "D"; + /** Original file, optionally with the conventional a/ prefix. */ + source_file?: string; + /** Resulting file, optionally with the conventional b/ prefix. */ + target_file?: string; + }; +}; + +/** Cancellation and absolute deadline checked before local mutation. */ +export type SeerPatchOptions = { + /** Caller cancellation signal. */ + signal?: AbortSignal; + /** Absolute deadline, in milliseconds since the Unix epoch. */ + deadline?: number; +}; + +/** Validated source and destination, where null denotes /dev/null. */ +type FilePaths = { source: string | null; target: string | null }; + +/** Portable alias keys include every directory prefix and Unicode normalization. */ +function pathPrefixes(path: string): string[] { + return path + .split("/") + .map((_segment, index, segments) => segments.slice(0, index + 1).join("/")); +} + +/** Case and Unicode aliases can collide on macOS/Windows even with distinct Git names. */ +function aliasKey(path: string): string { + return path.normalize("NFC").toLowerCase(); +} + +/** Require one consistent spelling for every affected file and directory. */ +function validateAliases( + path: string, + aliases: Map, + register: boolean, +): void { + for (const prefix of pathPrefixes(path)) { + const key = aliasKey(prefix); + const previous = aliases.get(key); + if (previous !== undefined && previous !== prefix) { + throw new ValidationError( + `Seer patch path aliases another file or directory: ${prefix} and ${previous}. No files were changed.`, + ); + } + if (register) { + aliases.set(key, prefix); + } + } +} + +/** Run Git without a shell; input is passed on stdin rather than a command line. */ +function git(args: string[], cwd: string, input?: string): string { + return execFileSync("git", args, { + cwd, + input, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + maxBuffer: 20 * 1024 * 1024, + }); +} + +/** Reject paths whose interpretation could escape or change across platforms. */ +function validatePath(path: string): void { + const segments = path.split("/"); + if ( + !path || + /[\\:]/.test(path) || + [...path].some( + (char) => char.charCodeAt(0) < 32 || char.charCodeAt(0) === 127, + ) || + segments.some( + (segment) => + !segment || + segment === "." || + segment === ".." || + /[. ]$/.test(segment) || + segment.toLowerCase() === ".git", + ) + ) { + throw new ValidationError( + `Unsafe Seer patch path: ${JSON.stringify(path)}.`, + ); + } +} + +/** Decode Git's C-quoted paths, including octal-escaped UTF-8 bytes. */ +function decodePath(value: string): string { + if (!value.startsWith('"')) { + return value; + } + if (!value.endsWith('"')) { + throw new ValidationError("Invalid quoted path in Seer patch."); + } + const bytes: number[] = []; + const content = value.slice(1, -1); + for (let i = 0; i < content.length; i++) { + const char = content[i]; + if (char === "\\") { + const escape = content.slice(i + 1).match(/^[0-7]{1,3}/)?.[0]; + if (escape) { + bytes.push(Number.parseInt(escape, 8)); + i += escape.length; + continue; + } + const next = content[++i]; + if (next !== '"' && next !== "\\") { + throw new ValidationError("Unsafe escaped path in Seer patch."); + } + bytes.push(...Buffer.from(next)); + } else { + const point = content.codePointAt(i); + if (point === undefined) { + throw new ValidationError("Invalid path in Seer patch."); + } + bytes.push(...Buffer.from(String.fromCodePoint(point))); + if (point > 0xffff) { + i++; + } + } + } + return Buffer.from(bytes).toString("utf8"); +} + +/** Header path, with timestamps removed and the required strip-one prefix checked. */ +function headerPath(value: string, side: "a" | "b"): string | null { + const path = decodePath(value.split("\t")[0] ?? ""); + if (path === "/dev/null") { + return null; + } + if (!path.startsWith(`${side}/`)) { + throw new ValidationError(`Seer diff paths must use the ${side}/ prefix.`); + } + const relative = path.slice(2); + validatePath(relative); + return relative; +} + +/** Equivalent literal/C-quoted spellings accepted in a Git diff header. */ +function pathSpellings(path: string): string[] { + const octal = [...Buffer.from(path)] + .map((byte) => { + if (byte > 0x7e) { + return `\\${byte.toString(8).padStart(3, "0")}`; + } + const char = String.fromCharCode(byte); + return char === '"' ? '\\"' : char; + }) + .join(""); + return [path, JSON.stringify(path), `"${octal}"`]; +} + +/** Match both Seer metadata and actual diff paths, before trusting either. */ +function validateDiff(file: SeerPatch): FilePaths { + validatePath(file.patch.path); + const lines = file.diff.split("\n"); + const firstHunk = lines.findIndex((line) => line.startsWith("@@ ")); + const headers = (firstHunk < 0 ? lines : lines.slice(0, firstHunk)).map( + (line) => line.replace(/\r$/, ""), + ); + if ( + !file.diff.endsWith("\n") || + lines.filter((line) => line.startsWith("diff --git ")).length > 1 || + lines.some( + (line) => line === "GIT binary patch" || line.startsWith("Binary files "), + ) + ) { + throw new ValidationError( + "Seer patches must contain one complete text-file diff each.", + ); + } + const oldHeaders = headers.filter((line) => line.startsWith("--- ")); + const newHeaders = headers.filter((line) => line.startsWith("+++ ")); + const from = headers.filter((line) => line.startsWith("rename from ")); + const to = headers.filter((line) => line.startsWith("rename to ")); + if ( + oldHeaders.length > 1 || + newHeaders.length > 1 || + from.length > 1 || + to.length > 1 || + headers.some( + (line) => line.startsWith("copy from ") || line.startsWith("copy to "), + ) + ) { + throw new ValidationError( + "Seer patches must describe exactly one file operation.", + ); + } + let paths: FilePaths; + const [oldHeader] = oldHeaders; + const [newHeader] = newHeaders; + const [renameFrom] = from; + const [renameTo] = to; + if (oldHeader !== undefined && newHeader !== undefined) { + paths = { + source: headerPath(oldHeader.slice(4), "a"), + target: headerPath(newHeader.slice(4), "b"), + }; + } else if ( + oldHeaders.length === 0 && + newHeaders.length === 0 && + renameFrom !== undefined && + renameTo !== undefined + ) { + paths = { + source: decodePath(renameFrom.slice(12)), + target: decodePath(renameTo.slice(10)), + }; + } else { + throw new ValidationError( + "Seer patch is missing its source or target diff header.", + ); + } + for (const path of [paths.source, paths.target]) { + if (path !== null) { + validatePath(path); + } + } + if ( + (file.patch.type === "A" && + (paths.source !== null || paths.target === null)) || + (file.patch.type === "D" && + (paths.source === null || paths.target !== null)) || + (file.patch.type === "M" && + (paths.source === null || paths.target === null)) || + ![paths.source, paths.target].includes(file.patch.path) + ) { + throw new ValidationError( + "Seer patch operation or path does not match its diff headers.", + ); + } + for (const [metadata, path, side] of [ + [file.patch.source_file, paths.source, "a"], + [file.patch.target_file, paths.target, "b"], + ] as const) { + if (metadata !== undefined) { + if (metadata !== "/dev/null") { + validatePath(metadata); + } + if ( + path === null + ? metadata !== "/dev/null" + : metadata !== path && metadata !== `${side}/${path}` + ) { + throw new ValidationError( + "Seer patch metadata does not match its diff headers.", + ); + } + } + } + const original = paths.source ?? paths.target; + const destination = paths.target ?? paths.source; + if (original === null || destination === null) { + throw new ValidationError( + "Seer patch must describe at least one file path.", + ); + } + const gitHeaders = headers.filter((line) => line.startsWith("diff --git ")); + if ( + gitHeaders.some( + (line) => + !pathSpellings(`a/${original}`).some((old) => + pathSpellings(`b/${destination}`).some( + (next) => line === `diff --git ${old} ${next}`, + ), + ), + ) + ) { + throw new ValidationError( + "Seer Git diff header does not match its file paths.", + ); + } + if ( + from.length !== to.length || + (renameFrom !== undefined && + renameTo !== undefined && + (decodePath(renameFrom.slice(12)) !== paths.source || + decodePath(renameTo.slice(10)) !== paths.target)) + ) { + throw new ValidationError( + "Seer rename headers do not match its file paths.", + ); + } + for (const line of headers) { + const mode = + line.match( + /^(?:new file mode|deleted file mode|old mode|new mode) (\d+)$/, + )?.[1] ?? line.match(/^index \S+ (\d+)$/)?.[1]; + if (mode !== undefined && mode !== "100644" && mode !== "100755") { + throw new ValidationError( + "Seer patches cannot create or modify symlinks or submodules.", + ); + } + } + return paths; +} + +/** Reject symlinks at every existing segment, including the file itself. */ +async function validateFilesystemPath( + root: string, + path: string, +): Promise { + let current = root; + for (const segment of path.split("/")) { + current = join(current, segment); + try { + const stat = await lstat(current); + if (stat.isSymbolicLink()) { + throw new ValidationError( + `Seer patch path crosses a symlink: ${path}.`, + ); + } + } catch (error) { + if ( + error instanceof Error && + "code" in error && + error.code === "ENOENT" + ) { + return; + } + throw error; + } + } +} + +/** Verify Git sees the same single file as the metadata/header validation. */ +function validateGitPaths( + root: string, + file: SeerPatch, + paths: FilePaths, +): void { + let output: string; + try { + output = git(["apply", "--numstat", "-z", "-"], root, file.diff); + } catch { + throw new ValidationError( + "Seer patch is not a valid text-file diff. No files were changed.", + ); + } + const records = output.split("\0"); + const header = records[0]?.match(/^\d+\t\d+\t(.*)$/); + const reported = header?.[1]; + const valid = + reported === "" + ? records.length === 4 && + records[1] === paths.source && + records[2] === paths.target && + records[3] === "" + : records.length === 2 && + reported === (paths.target ?? paths.source) && + records[1] === ""; + if (!valid) { + throw new ValidationError( + "Seer patch contains unexpected file paths or additional file diffs. No files were changed.", + ); + } +} + +/** + * Apply all patches after validating repository identity, paths, and local conflicts. + * Git checks the entire batch before writes. Unrelated local edits are preserved. + * @param patches - File diffs from one Seer result. + * @param cwd - Any directory inside the target Git checkout. + * @param options - Cancellation/deadline checked at entry and before Git writes. + * @returns All affected repository-relative paths, including both sides of renames. + * @throws ValidationError for unsafe/conflicting input; CliError for Git failures. + */ +export async function applySeerPatches( + patches: readonly SeerPatch[], + cwd: string, + options: SeerPatchOptions = {}, +): Promise<{ applied: string[] }> { + const assertActive = (): void => { + if (options.signal?.aborted) { + throw new CliError( + "Seer patch application was cancelled. No files were changed.", + ); + } + if ( + options.deadline !== undefined && + (!Number.isFinite(options.deadline) || Date.now() >= options.deadline) + ) { + throw new CliError( + "Seer patch application timed out. No files were changed.", + ); + } + }; + assertActive(); + if (patches.length === 0) { + return { applied: [] }; + } + if (new Set(patches.map((patch) => patch.repo_name)).size > 1) { + throw new ValidationError( + "Cannot apply a multi-repository Seer patch batch to one checkout. Select code changes from one repository. No files were changed.", + ); + } + let root: string; + let remotes: Set; + try { + root = await realpath(git(["rev-parse", "--show-toplevel"], cwd).trim()); + const remoteNames = git(["remote"], root) + .trim() + .split("\n") + .filter((name) => name === "origin" || name === "upstream"); + remotes = new Set( + remoteNames + .map((name) => + parseRemoteUrl(git(["remote", "get-url", name], root).trim()), + ) + .filter((name): name is string => name !== undefined), + ); + } catch { + throw new CliError( + "Cannot apply Seer patches: run this command inside a Git checkout with an origin or upstream remote.", + ); + } + if (patches.some((patch) => !remotes.has(patch.repo_name))) { + throw new ValidationError( + "Cannot apply Seer patches: every patch repository must match this checkout's origin or upstream remote. No files were changed.", + ); + } + const affected = new Set(); + const aliases = new Map(); + for (const patch of patches) { + const paths = validateDiff(patch); + validateGitPaths(root, patch, paths); + for (const path of new Set([paths.source, paths.target])) { + if (path !== null) { + if (affected.has(path)) { + throw new ValidationError( + `Seer patch batch affects the same path more than once: ${path}.`, + ); + } + affected.add(path); + validateAliases(path, aliases, true); + await validateFilesystemPath(root, path); + } + } + } + const changed = git( + ["diff", "--no-ext-diff", "--name-only", "-z"], + root, + ).split("\0"); + const staged = git( + ["diff", "--cached", "--no-ext-diff", "--name-only", "-z"], + root, + ).split("\0"); + const untracked = git(["ls-files", "--others", "-z"], root).split("\0"); + const tracked = git(["ls-files", "-z"], root).split("\0"); + for (const path of [...tracked, ...untracked]) { + if (path) { + validateAliases(path, aliases, false); + } + } + const conflicts = new Set([...changed, ...staged, ...untracked]); + for (const path of affected) { + if (conflicts.has(path)) { + throw new ValidationError( + `Cannot apply Seer patch to ${path}: the file has local changes or is untracked. Commit or move those changes first.`, + ); + } + } + const diff = patches.map((patch) => patch.diff).join("\n"); + try { + git( + ["apply", "--check", "--index", "--whitespace=nowarn", "-"], + root, + diff, + ); + } catch { + throw new ValidationError( + "Seer patches could not be applied cleanly. No files were changed; review the diffs and checkout before retrying.", + ); + } + assertActive(); + try { + git(["apply", "--whitespace=nowarn", "-"], root, diff); + } catch { + throw new CliError( + "Git could not apply the Seer patches. Review the checkout and saved diffs before retrying.", + ); + } + return { applied: [...affected] }; +} diff --git a/packages/cli/test/commands/issue/fix.func.test.ts b/packages/cli/test/commands/issue/fix.func.test.ts new file mode 100644 index 000000000..9e0916901 --- /dev/null +++ b/packages/cli/test/commands/issue/fix.func.test.ts @@ -0,0 +1,202 @@ +/** Explicit and interactive Seer fix delivery through the real command wrapper. */ +import { isatty } from "node:tty"; +import { afterEach, beforeEach, expect, test, vi } from "vitest"; +import { fixCommand } from "../../../src/commands/issue/fix.js"; +import { resolveOrgAndIssueId } from "../../../src/commands/issue/utils.js"; +import { OutputError } from "../../../src/lib/errors.js"; +import { runSeerFix } from "../../../src/lib/seer-fix.js"; +import { deliverSeerFix } from "../../../src/lib/seer-fix-delivery.js"; +import type { FixResult } from "../../../src/types/seer-fix.js"; +import { useTestConfigDir } from "../../helpers.js"; +const log = vi.hoisted(() => ({ prompt: vi.fn() })); +vi.mock("node:tty", () => ({ isatty: vi.fn() })); +vi.mock("../../../src/lib/logger.js", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + logger: { + ...actual.logger, + withTag: (tag: string) => { + const scoped = actual.logger.withTag(tag); + scoped.prompt = log.prompt; + return scoped; + }, + }, + }; +}); +vi.mock("../../../src/lib/seer-fix.js", () => ({ runSeerFix: vi.fn() })); +vi.mock("../../../src/lib/seer-fix-delivery.js", () => ({ + deliverSeerFix: vi.fn(), +})); +vi.mock("../../../src/commands/issue/utils.js", () => ({ + resolveOrgAndIssueId: vi.fn(), + issueIdPositional: { + kind: "tuple", + parameters: [{ placeholder: "issue", parse: String }], + }, +})); +useTestConfigDir("test-issue-fix-delivery-"); +const ready: FixResult = { + run_id: "run-1", + status: "COMPLETED", + pending_user_input: null, + patches: [ + { + repo_name: "example/app", + diff: "--- a/index.ts\n+++ b/index.ts\n@@ -1 +1 @@\n-old\n+new\n", + patch: { + added: 1, + removed: 1, + path: "index.ts", + source_file: "index.ts", + target_file: "index.ts", + type: "M", + hunks: [], + }, + }, + ], + pr_urls: [], +}; +const flags = { + json: false, + fresh: false, + timeout: 360, + apply: false, + pr: false, +}; +function context() { + return { + cwd: "/tmp", + stdout: { write: vi.fn((_value: string) => true) }, + stderr: { write: vi.fn((_value: string) => true) }, + }; +} +function output(ctx: ReturnType): string { + return ctx.stdout.write.mock.calls.map(([value]) => value).join(""); +} +async function run( + ctx: ReturnType, + overrides: Partial = {}, +) { + const func = await fixCommand.loader(); + return func.call(ctx, { ...flags, ...overrides }, "test-org/12345"); +} +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(isatty).mockReturnValue(false); + vi.mocked(resolveOrgAndIssueId).mockResolvedValue({ + org: "test-org", + issueId: "12345", + }); + vi.mocked(runSeerFix).mockResolvedValue(ready); + vi.mocked(deliverSeerFix).mockResolvedValue({ + ...ready, + applied_files: ["index.ts"], + }); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); +test("JSON defaults to preview without prompting or delivering", async () => { + const ctx = context(); + vi.mocked(isatty).mockReturnValue(true); + await run(ctx, { json: true }); + expect(JSON.parse(output(ctx))).toEqual(ready); + expect(deliverSeerFix).not.toHaveBeenCalled(); + expect(log.prompt).not.toHaveBeenCalled(); +}); +test("explicit apply delivers and yields one aggregate JSON document", async () => { + const ctx = context(); + await run(ctx, { json: true, apply: true }); + expect(deliverSeerFix).toHaveBeenCalledWith( + expect.objectContaining({ + org: "test-org", + cwd: "/tmp", + result: ready, + apply: true, + pr: false, + }), + ); + expect(JSON.parse(output(ctx))).toMatchObject({ + patches: ready.patches, + applied_files: ["index.ts"], + }); + expect(log.prompt).not.toHaveBeenCalled(); +}); +test("previews diffs before an interactive delivery choice", async () => { + const ctx = context(); + vi.mocked(isatty).mockReturnValue(true); + let previewVisible = false; + log.prompt.mockImplementation(async () => { + previewVisible = output(ctx).includes("index.ts"); + return "Both"; + }); + await run(ctx); + expect(previewVisible).toBe(true); + expect(deliverSeerFix).toHaveBeenCalledWith( + expect.objectContaining({ apply: true, pr: true }), + ); + expect(output(ctx).match(/\+new/g)).toHaveLength(1); +}); +test.each(["Skip", Symbol("cancel")])( + "skips delivery when the interactive selection is %s", + async (selection) => { + const ctx = context(); + vi.mocked(isatty).mockReturnValue(true); + log.prompt.mockResolvedValue(selection); + await run(ctx); + expect(deliverSeerFix).not.toHaveBeenCalled(); + expect(log.prompt).toHaveBeenCalledWith( + "What would you like to do with this fix?", + expect.objectContaining({ initial: "Skip", cancel: "symbol" }), + ); + }, +); +test("a non-TTY preview never prompts or delivers by default", async () => { + const ctx = context(); + await run(ctx); + expect(output(ctx)).toContain("index.ts"); + expect(log.prompt).not.toHaveBeenCalled(); + expect(deliverSeerFix).not.toHaveBeenCalled(); +}); +test("pending input blocks explicit delivery", async () => { + const paused = { + ...ready, + status: "WAITING_FOR_USER_RESPONSE", + pending_user_input: { id: "q1", input_type: "ask_user_question", data: {} }, + }; + vi.mocked(runSeerFix).mockResolvedValue(paused); + const ctx = context(); + await run(ctx, { json: true, apply: true, pr: true }); + expect(JSON.parse(output(ctx))).toEqual(paused); + expect(deliverSeerFix).not.toHaveBeenCalled(); +}); +test("delivery failures retain diffs in JSON and exit nonzero", async () => { + vi.mocked(deliverSeerFix).mockResolvedValue({ + ...ready, + status: "ERROR", + delivery_errors: ["Dirty target file"], + }); + const ctx = context(); + await expect(run(ctx, { json: true, apply: true })).rejects.toBeInstanceOf( + OutputError, + ); + expect(JSON.parse(output(ctx))).toMatchObject({ + patches: ready.patches, + delivery_errors: ["Dirty target file"], + }); +}); +test("an expired interactive delivery budget prevents every mutation", async () => { + const ctx = context(); + vi.mocked(isatty).mockReturnValue(true); + let now = 1000; + vi.spyOn(Date, "now").mockImplementation(() => now); + log.prompt.mockImplementation(async () => { + now = 3000; + return "Apply locally"; + }); + await expect(run(ctx, { timeout: 1 })).rejects.toBeInstanceOf(OutputError); + expect(deliverSeerFix).not.toHaveBeenCalled(); + expect(output(ctx)).toContain("TIMEOUT"); +}); diff --git a/packages/cli/test/e2e/seer-fix.test.ts b/packages/cli/test/e2e/seer-fix.test.ts index 0f58331d4..2e78ec249 100644 --- a/packages/cli/test/e2e/seer-fix.test.ts +++ b/packages/cli/test/e2e/seer-fix.test.ts @@ -3,10 +3,19 @@ * Sentry's interactive explorer chat/update endpoint contracts. */ -import { writeFile } from "node:fs/promises"; +import { execFileSync, spawn } from "node:child_process"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { createRequire } from "node:module"; import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import type { ExplorerFilePatch } from "../../src/types/seer-fix.js"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; -import { createE2EContext, type E2EContext } from "../fixture.js"; +import { + createE2EContext, + type E2EContext, + getCliCommand, + type CliResult, +} from "../fixture.js"; import issueFixture from "../fixtures/issue.json"; import { useTestConfigDir } from "../helpers.js"; import { @@ -44,7 +53,7 @@ const QUESTION = { ], }, }; -const PATCH = { +const PATCH: ExplorerFilePatch = { repo_name: "example/backend", code_url: null, diff: "--- a/users.ts\n+++ b/users.ts\n@@ -1 +1 @@\n-return user.name;\n+return user?.name;\n", @@ -192,6 +201,165 @@ async function responseFile(value: unknown): Promise { return path; } +/** Run source or bundled CLI from a real checkout rather than the CLI package. */ +async function runInCheckout(cwd: string, args: string[]): Promise { + const cliDir = join(import.meta.dirname, "../.."); + const [bin, ...commandArgs] = getCliCommand(); + const require = createRequire(import.meta.url); + const resolvedArgs = process.env.SENTRY_CLI_BINARY + ? commandArgs + : commandArgs.map((arg, index) => { + const resolved = + arg === "tsx" + ? require.resolve("tsx") + : arg.startsWith("./") || arg === "src/bin.ts" + ? join(cliDir, arg) + : arg; + // Node's ESM --import loader requires file URLs for Windows drive paths. + return commandArgs[index - 1] === "--import" + ? pathToFileURL(resolved).href + : resolved; + }); + const proc = spawn(bin, [...resolvedArgs, ...args], { + cwd, + env: { + ...process.env, + SENTRY_AUTH_TOKEN: "", + SENTRY_TOKEN: "", + SENTRY_CONFIG_DIR: getConfigDir(), + SENTRY_URL: mockServer.url, + SENTRY_CLI_NO_TELEMETRY: "1", + SENTRY_CLI_NO_UPDATE_CHECK: "1", + }, + stdio: ["pipe", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + proc.stdout.on("data", (chunk: Buffer) => { + stdout += chunk.toString(); + }); + proc.stderr.on("data", (chunk: Buffer) => { + stderr += chunk.toString(); + }); + const exitCode = await new Promise((resolve, reject) => { + proc.once("error", reject); + proc.once("close", (code) => resolve(code ?? 1)); + }); + return { stdout, stderr, exitCode }; +} + +/** Create an isolated matching Git repo; useTestConfigDir removes it after the test. */ +async function checkout( + remote = "https://github.com/example/backend.git", +): Promise { + const cwd = join(getConfigDir(), "checkout"); + await mkdir(cwd); + const git = (args: string[]) => + execFileSync("git", args, { + cwd, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + }); + git(["init"]); + git(["config", "core.autocrlf", "false"]); + git(["remote", "add", "origin", remote]); + await writeFile(join(cwd, "users.ts"), "return user.name;\n", "utf8"); + await writeFile(join(cwd, "remove.txt"), "remove\n", "utf8"); + await writeFile(join(cwd, "unrelated.txt"), "baseline\n", "utf8"); + git(["add", "."]); + git([ + "-c", + "user.name=Seer E2E", + "-c", + "user.email=seer-e2e@example.invalid", + "-c", + "commit.gpgsign=false", + "-c", + "core.hooksPath=", + "commit", + "-m", + "fixture baseline", + ]); + return cwd; +} + +/** Use real backend-shaped patches in completed explorer blocks. */ +function completed(patches: ExplorerFilePatch[] = [PATCH]): Session { + return { + ...session("completed"), + blocks: [ + { + id: "delivery-patches", + message: { role: "assistant", content: "Fix ready" }, + merged_file_patches: patches, + }, + ], + }; +} + +const ADDED_PATCH: ExplorerFilePatch = { + repo_name: "example/backend", + code_url: null, + diff: "diff --git a/new.txt b/new.txt\nnew file mode 100644\n--- /dev/null\n+++ b/new.txt\n@@ -0,0 +1 @@\n+new\n", + patch: { + added: 1, + removed: 0, + path: "new.txt", + source_file: "/dev/null", + target_file: "b/new.txt", + type: "A", + hunks: [ + { + source_start: 0, + source_length: 0, + target_start: 1, + target_length: 1, + section_header: "", + lines: [ + { + line_type: "+", + value: "new", + source_line_no: null, + target_line_no: 1, + diff_line_no: 1, + }, + ], + }, + ], + }, +}; +const DELETED_PATCH: ExplorerFilePatch = { + repo_name: "example/backend", + code_url: null, + diff: "diff --git a/remove.txt b/remove.txt\ndeleted file mode 100644\n--- a/remove.txt\n+++ /dev/null\n@@ -1 +0,0 @@\n-remove\n", + patch: { + added: 0, + removed: 1, + path: "remove.txt", + source_file: "a/remove.txt", + target_file: "/dev/null", + type: "D", + hunks: [ + { + source_start: 1, + source_length: 1, + target_start: 0, + target_length: 0, + section_header: "", + lines: [ + { + line_type: "-", + value: "remove", + source_line_no: 1, + target_line_no: null, + diff_line_no: 1, + }, + ], + }, + ], + }, +}; + describe("sentry issue fix", () => { test("starts coding and returns one JSON question result pinned to the UUID", async () => { const result = await ctx.run([ @@ -554,3 +722,301 @@ describe("sentry issue fix", () => { expect(mutations()[0]?.path).toBe(UPDATE.replace(":runId", RUN_ID)); }); }); + +describe("sentry issue fix delivery", () => { + test("JSON preview leaves the checkout unchanged until delivery is requested", async () => { + const cwd = await checkout(); + activeSession = completed(); + const result = await runInCheckout(cwd, [ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--json", + ]); + + expect(result.exitCode, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + status: "COMPLETED", + patches: [PATCH], + pr_urls: [], + }); + expect(await readFile(join(cwd, "users.ts"), "utf8")).toBe( + "return user.name;\n", + ); + expect(mutations()).toEqual([]); + }); + test("applies an entire add/modify/delete batch from a nested checkout and preserves unrelated work", async () => { + const cwd = await checkout(); + await writeFile(join(cwd, "unrelated.txt"), "my local work\n", "utf8"); + const nested = join(cwd, "nested"); + await mkdir(nested); + activeSession = completed([PATCH, ADDED_PATCH, DELETED_PATCH]); + + const result = await runInCheckout(nested, [ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--apply", + "--json", + ]); + + expect(result.exitCode, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + run_id: RUN_ID, + status: "COMPLETED", + patches: [PATCH, ADDED_PATCH, DELETED_PATCH], + applied_files: ["users.ts", "new.txt", "remove.txt"], + }); + expect(await readFile(join(cwd, "users.ts"), "utf8")).toBe( + "return user?.name;\n", + ); + expect(await readFile(join(cwd, "new.txt"), "utf8")).toBe("new\n"); + await expect(readFile(join(cwd, "remove.txt"))).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(await readFile(join(cwd, "unrelated.txt"), "utf8")).toBe( + "my local work\n", + ); + expect(mutations()).toEqual([]); + }); + + test.each(["dirty", "mismatch", "unsafe"] as const)( + "refuses %s local application, preserves the fix and does not create a PR", + async (kind) => { + const cwd = await checkout( + kind === "mismatch" + ? "https://github.com/example/other.git" + : undefined, + ); + const before = + kind === "dirty" ? "my local changes\n" : "return user.name;\n"; + await writeFile(join(cwd, "users.ts"), before, "utf8"); + const outside = join(getConfigDir(), "outside.txt"); + await writeFile(outside, "keep outside\n", "utf8"); + const patch = + kind === "unsafe" + ? { ...PATCH, patch: { ...PATCH.patch, path: "../outside.txt" } } + : PATCH; + activeSession = completed([patch]); + + const result = await runInCheckout(cwd, [ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--apply", + "--pr", + "--json", + ]); + + expect(result.exitCode).not.toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + run_id: RUN_ID, + status: "ERROR", + patches: [patch], + delivery_errors: [expect.any(String)], + }); + expect(await readFile(join(cwd, "users.ts"), "utf8")).toBe(before); + expect(await readFile(outside, "utf8")).toBe("keep outside\n"); + expect(mutations()).toEqual([]); + }, + ); + + test("creates one wrapped PR request and polls repository state even when the run is completed", async () => { + let reads = 0; + chatHandler = () => { + reads += 1; + const state = completed(); + if (mutations().length) { + state.repo_pr_states = { + "example/backend": { + repo_name: "example/backend", + pr_creation_status: reads < 4 ? "creating" : "completed", + pr_url: + reads < 4 ? null : "https://github.com/example/backend/pull/42", + }, + }; + } + return { body: { session: state, sentry_run_id: RUN_ID } }; + }; + const result = await ctx.run([ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--pr", + "--json", + ]); + + expect(result.exitCode, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + status: "COMPLETED", + patches: [PATCH], + pr_urls: ["https://github.com/example/backend/pull/42"], + }); + expect(mutations()).toEqual([ + { + method: "POST", + path: UPDATE.replace(":runId", RUN_ID), + body: { payload: { type: "create_pr", repo_name: "example/backend" } }, + }, + ]); + expect(reads).toBeGreaterThanOrEqual(4); + }); + + test("waits for an existing in-flight PR without creating another one", async () => { + let reads = 0; + chatHandler = () => { + reads += 1; + return { + body: { + sentry_run_id: RUN_ID, + session: { + ...completed(), + repo_pr_states: { + "example/backend": { + repo_name: "example/backend", + pr_creation_status: reads < 3 ? "creating" : "completed", + pr_url: + reads < 3 + ? null + : "https://github.com/example/backend/pull/42", + }, + }, + }, + }, + }; + }; + const result = await ctx.run([ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--pr", + "--json", + ]); + + expect(result.exitCode, result.stderr).toBe(0); + expect(JSON.parse(result.stdout).pr_urls).toEqual([ + "https://github.com/example/backend/pull/42", + ]); + expect(mutations()).toEqual([]); + }); + + test("retains generated diffs and returns a nonzero aggregate when PR creation fails", async () => { + chatHandler = () => ({ + body: { + sentry_run_id: RUN_ID, + session: { + ...completed(), + repo_pr_states: mutations().length + ? { + "example/backend": { + repo_name: "example/backend", + pr_creation_status: "error", + pr_creation_error: "GitHub denied repository access", + }, + } + : {}, + }, + }, + }); + const result = await ctx.run([ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--pr", + "--json", + ]); + + expect(result.exitCode).not.toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + run_id: RUN_ID, + status: "ERROR", + patches: [PATCH], + delivery_errors: [expect.stringMatching(/GitHub denied/i)], + }); + expect(mutations()).toHaveLength(1); + }); + + test("does not replay an uncertain create-PR POST after a service error", async () => { + activeSession = completed(); + updateResponse = { + status: 503, + body: { detail: "Seer temporarily unavailable" }, + }; + const result = await ctx.run([ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--pr", + "--json", + ]); + + expect(result.exitCode).not.toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + run_id: RUN_ID, + status: "ERROR", + patches: [PATCH], + delivery_errors: [expect.any(String)], + }); + expect(mutations()).toHaveLength(1); + expect(mutations()[0]?.body).toEqual({ + payload: { type: "create_pr", repo_name: "example/backend" }, + }); + }); + + test("does not apply files or create a PR when fresh preflight discovers a pending approval", async () => { + const cwd = await checkout(); + let reads = 0; + chatHandler = () => { + reads += 1; + return { + body: { + sentry_run_id: RUN_ID, + session: + reads === 1 + ? completed() + : { + ...completed(), + status: "awaiting_user_input", + pending_user_input: QUESTION, + }, + }, + }; + }; + const result = await runInCheckout(cwd, [ + "issue", + "fix", + ISSUE, + "--run-id", + RUN_ID, + "--apply", + "--pr", + "--json", + ]); + + expect(result.exitCode).not.toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + run_id: RUN_ID, + status: "ERROR", + patches: [PATCH], + pending_user_input: QUESTION, + }); + expect(mutations()).toEqual([]); + expect(await readFile(join(cwd, "users.ts"), "utf8")).toBe( + "return user.name;\n", + ); + }); +}); diff --git a/packages/cli/test/lib/formatters/seer-fix.test.ts b/packages/cli/test/lib/formatters/seer-fix.test.ts index 0546f4e3b..a01425fdd 100644 --- a/packages/cli/test/lib/formatters/seer-fix.test.ts +++ b/packages/cli/test/lib/formatters/seer-fix.test.ts @@ -1,6 +1,8 @@ import { beforeEach, expect, test, vi } from "vitest"; import { createSeerBlockRenderer, + createSeerFixRenderer, + promptSeerDelivery, promptSeerInput, } from "../../../src/lib/formatters/seer-fix.js"; const log = vi.hoisted(() => ({ prompt: vi.fn(), info: vi.fn() })); @@ -102,3 +104,67 @@ test("allows a custom answer for a question and stops on cancellation", async () log.prompt.mockResolvedValueOnce(Symbol("cancel")); expect(await promptSeerInput(input, { write: () => {} })).toBeNull(); }); + +test("renders preview diffs once and then shows only delivery results", () => { + const renderer = createSeerFixRenderer(); + const result = { + run_id: "run-1", + status: "COMPLETED", + pending_user_input: null, + patches: [ + { + repo_name: "private/repo", + diff: "--- a/index.ts\n+++ b/index.ts\n@@ -1 +1 @@\n-old\n+private-code\n", + patch: { + added: 1, + removed: 1, + path: "index.ts", + source_file: "index.ts", + target_file: "index.ts", + type: "M" as const, + hunks: [], + }, + }, + ], + pr_urls: [], + }; + const preview = renderer.render(result); + const delivered = renderer.render({ + ...result, + applied_files: ["index.ts"], + pr_urls: ["https://github.com/private/repo/pull/1"], + }); + expect(preview).toContain("+private-code"); + expect(delivered).not.toContain("+private-code"); + expect(delivered).toContain("index.ts"); + expect(delivered).toContain("https://github.com/private/repo/pull/1"); + expect(log.info).not.toHaveBeenCalled(); +}); +test("preserves partial delivery evidence alongside failures", () => { + const renderer = createSeerFixRenderer(); + const output = renderer.render({ + run_id: "run-1", + status: "ERROR", + pending_user_input: null, + patches: [], + pr_urls: ["https://github.com/private/repo/pull/1"], + applied_files: ["index.ts"], + delivery_errors: ["Second repository failed"], + }); + expect(output).toContain("index.ts"); + expect(output).toContain("Second repository failed"); + expect(output).toContain("https://github.com/private/repo/pull/1"); +}); +test("delivery prompt defaults to skip and treats cancellation as skip", async () => { + log.prompt.mockResolvedValue(Symbol("cancel")); + expect(await promptSeerDelivery()).toEqual({ apply: false, pr: false }); + expect(log.prompt).toHaveBeenCalledWith( + "What would you like to do with this fix?", + { + type: "select", + initial: "Skip", + cancel: "symbol", + options: ["Apply locally", "Create PR", "Both", "Skip"], + }, + ); +}); diff --git a/packages/cli/test/lib/seer-fix-delivery.test.ts b/packages/cli/test/lib/seer-fix-delivery.test.ts new file mode 100644 index 000000000..a0c07e04f --- /dev/null +++ b/packages/cli/test/lib/seer-fix-delivery.test.ts @@ -0,0 +1,653 @@ +/** Delivery orchestration coverage using mocked network requests and real Git. */ +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { afterEach, beforeEach, expect, test, vi } from "vitest"; +import type { ExplorerSession, FixResult } from "../../src/types/seer-fix.js"; +import { + apiRequestToRegion, + getSeerExplorerState, +} from "../../src/lib/api-client.js"; +import { resolveOrgRegion } from "../../src/lib/region.js"; +import { deliverSeerFix } from "../../src/lib/seer-fix-delivery.js"; +import { useTestConfigDir } from "../helpers.js"; + +vi.mock("../../src/lib/api-client.js", () => ({ + apiRequestToRegion: vi.fn(), + getSeerExplorerState: vi.fn(), + getAutofixState: vi.fn(), + respondToSeerUserInput: vi.fn(), + triggerIssueFix: vi.fn(), +})); +vi.mock("../../src/lib/region.js", () => ({ resolveOrgRegion: vi.fn() })); + +const getDir = useTestConfigDir("seer-fix-delivery-"); +const patch: FixResult["patches"][number] = { + repo_name: "example/project", + diff: "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-before\n+after\n", + patch: { + added: 1, + removed: 1, + hunks: [], + path: "file.txt", + source_file: "a/file.txt", + target_file: "b/file.txt", + type: "M", + }, +}; +const result: FixResult = { + run_id: "run-uuid", + status: "COMPLETED", + pending_user_input: null, + patches: [patch], + pr_urls: [], +}; +const options = { + org: "example-org", + cwd: "/unused", + apply: false, + pr: true, + timeoutMs: 1000, + pollIntervalMs: 1, +}; + +function ready(patches = result.patches): ExplorerSession { + return { + status: "COMPLETED", + blocks: [{ id: "changes", merged_file_patches: patches }], + }; +} + +function session( + status: "creating" | "completed" | "error", + repo = "example/project", +): ExplorerSession { + return { + ...ready(), + repo_pr_states: { + [repo]: { + repo_name: repo, + pr_creation_status: status, + ...(status === "completed" + ? { pr_url: "https://github.com/example/project/pull/1" } + : {}), + }, + }, + }; +} + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(resolveOrgRegion).mockResolvedValue("https://de.sentry.io"); + vi.mocked(apiRequestToRegion).mockResolvedValue({ data: {} }); + vi.mocked(getSeerExplorerState).mockResolvedValue(ready()); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +async function checkout(): Promise { + const repo = join(getDir(), "repo"); + await mkdir(repo); + const git = (args: string[]) => + execFileSync("git", args, { + cwd: repo, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + }); + git(["init"]); + git(["config", "core.autocrlf", "false"]); + git(["remote", "add", "origin", "https://github.com/example/project.git"]); + await writeFile(join(repo, "file.txt"), "before\n"); + git(["add", "."]); + git([ + "-c", + "user.name=Audit", + "-c", + "user.email=audit@example.invalid", + "-c", + "commit.gpgsign=false", + "-c", + "core.hooksPath=", + "commit", + "-m", + "test baseline", + ]); + return repo; +} + +test("creates one PR using the wrapped no-retry request and polls beyond run completion", async () => { + vi.mocked(getSeerExplorerState) + .mockResolvedValueOnce(ready()) + .mockResolvedValueOnce(session("creating")) + .mockResolvedValue(session("completed")); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered).toMatchObject({ + status: "COMPLETED", + patches: result.patches, + pr_urls: ["https://github.com/example/project/pull/1"], + }); + expect(resolveOrgRegion).toHaveBeenCalledWith("example-org"); + expect(apiRequestToRegion).toHaveBeenCalledExactlyOnceWith( + "https://de.sentry.io", + "/organizations/example-org/seer/explorer-update/run-uuid/", + expect.objectContaining({ + method: "POST", + retry: false, + signal: expect.any(AbortSignal), + body: { payload: { type: "create_pr", repo_name: "example/project" } }, + }), + ); + expect(getSeerExplorerState).toHaveBeenCalledTimes(3); +}); + +test("preserves the generated patch when a creation POST has an uncertain outcome", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue(ready()); + vi.mocked(apiRequestToRegion).mockRejectedValue( + new Error("Service unavailable"), + ); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered).toMatchObject({ + status: "ERROR", + run_id: "run-uuid", + patches: result.patches, + }); + expect(delivered.delivery_errors?.[0]).toContain("Service unavailable"); + expect(delivered.next_action).toContain("--run-id run-uuid"); + expect(apiRequestToRegion).toHaveBeenCalledOnce(); +}); + +test("applies a completed fix to a real checkout only when explicitly requested", async () => { + const cwd = await checkout(); + const delivered = await deliverSeerFix({ + ...options, + cwd, + result, + apply: true, + pr: false, + timeoutMs: 10_000, + }); + expect(delivered).toMatchObject({ + status: "COMPLETED", + applied_files: ["file.txt"], + patches: result.patches, + pr_urls: [], + }); + expect(await readFile(join(cwd, "file.txt"), "utf8")).toBe("after\n"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + expect(getSeerExplorerState).toHaveBeenCalledExactlyOnceWith( + "example-org", + "run-uuid", + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); +}); + +test("preserves local work and generated diffs when local application fails", async () => { + const cwd = await checkout(); + await writeFile(join(cwd, "file.txt"), "my work\n"); + const delivered = await deliverSeerFix({ + ...options, + cwd, + result, + apply: true, + timeoutMs: 10_000, + }); + expect(delivered).toMatchObject({ status: "ERROR", patches: result.patches }); + expect(delivered.applied_files).toBeUndefined(); + expect(delivered.delivery_errors?.[0]).toContain("local changes"); + expect(await readFile(join(cwd, "file.txt"), "utf8")).toBe("my work\n"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test.each(["apply", "pr"])( + "refuses %s of ambiguous raw incremental edits to one file", + async (action) => { + const cwd = await checkout(); + await writeFile( + join(cwd, "file.txt"), + "first\ncontext one\ncontext two\nlast\n", + ); + execFileSync("git", ["add", "file.txt"], { + cwd, + stdio: ["pipe", "pipe", "pipe"], + }); + execFileSync( + "git", + [ + "-c", + "user.name=Audit", + "-c", + "user.email=audit@example.invalid", + "-c", + "commit.gpgsign=false", + "-c", + "core.hooksPath=", + "commit", + "-m", + "two distant edit baseline", + ], + { cwd, stdio: ["pipe", "pipe", "pipe"] }, + ); + const first = { + ...patch, + diff: "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n-first\n+changed first\n context one\n context two\n", + }; + const latest = { + ...patch, + diff: "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -2,3 +2,3 @@\n context one\n context two\n-last\n+changed last\n", + }; + vi.mocked(getSeerExplorerState) + .mockResolvedValueOnce({ + status: "COMPLETED", + blocks: [ + { id: "first", file_patches: [first] }, + { id: "last", file_patches: [latest] }, + ], + }) + .mockResolvedValue(session("completed")); + const delivered = await deliverSeerFix({ + ...options, + cwd, + result: { ...result, patches: [latest] }, + apply: action === "apply", + pr: action === "pr", + timeoutMs: 10_000, + }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("merged"); + expect(delivered.patches).toEqual([latest]); + expect(await readFile(join(cwd, "file.txt"), "utf8")).toBe( + "first\ncontext one\ncontext two\nlast\n", + ); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + }, +); + +test.each(["pending", "processing", "changed diff", "changed metadata"])( + "refuses stale local delivery when fresh state has %s", + async (change) => { + const cwd = await checkout(); + const fresh = ready(); + if (change === "pending") { + fresh.pending_user_input = { + id: "approval", + input_type: "file_change_approval", + data: {}, + }; + } else if (change === "processing") { + fresh.status = "PROCESSING"; + } else { + fresh.blocks = [ + { + id: "changes", + merged_file_patches: [ + { + ...patch, + ...(change === "changed diff" + ? { diff: patch.diff.replace("+after", "+different") } + : { patch: { ...patch.patch, added: 2 } }), + }, + ], + }, + ]; + } + vi.mocked(getSeerExplorerState).mockResolvedValue(fresh); + const delivered = await deliverSeerFix({ + ...options, + cwd, + result, + apply: true, + pr: false, + timeoutMs: 10_000, + }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.patches).toEqual(result.patches); + expect(delivered.applied_files).toBeUndefined(); + expect(await readFile(join(cwd, "file.txt"), "utf8")).toBe("before\n"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + }, +); + +test.each(["PROCESSING", "changed patches"])( + "refuses stale PR delivery when fresh state has %s", + async (change) => { + vi.mocked(getSeerExplorerState).mockResolvedValue( + change === "PROCESSING" + ? { ...ready(), status: "PROCESSING" } + : ready([{ ...patch, diff: "different" }]), + ); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.patches).toEqual(result.patches); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + }, +); + +test("compares patch metadata structurally rather than relying on object key order", async () => { + const reordered = { + ...patch, + patch: { + type: patch.patch.type, + target_file: patch.patch.target_file, + source_file: patch.patch.source_file, + path: patch.patch.path, + hunks: patch.patch.hunks, + removed: patch.patch.removed, + added: patch.patch.added, + }, + }; + vi.mocked(getSeerExplorerState) + .mockResolvedValueOnce(ready([reordered])) + .mockResolvedValue(session("completed")); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.status).toBe("COMPLETED"); + expect(apiRequestToRegion).toHaveBeenCalledOnce(); +}); + +test("keeps successful local application when subsequent PR creation fails", async () => { + const cwd = await checkout(); + vi.mocked(getSeerExplorerState).mockResolvedValue(ready()); + vi.mocked(apiRequestToRegion).mockRejectedValue( + new Error("Service unavailable"), + ); + const delivered = await deliverSeerFix({ + ...options, + cwd, + result, + apply: true, + timeoutMs: 10_000, + }); + expect(delivered).toMatchObject({ + status: "ERROR", + applied_files: ["file.txt"], + patches: result.patches, + }); + expect(await readFile(join(cwd, "file.txt"), "utf8")).toBe("after\n"); + expect(apiRequestToRegion).toHaveBeenCalledOnce(); +}); + +test("preserves successful application when the deadline expires after the write", async () => { + const cwd = await checkout(); + vi.spyOn(Date, "now").mockImplementation(() => + readFileSync(join(cwd, "file.txt"), "utf8") === "after\n" ? 20_000 : 0, + ); + const delivered = await deliverSeerFix({ + ...options, + cwd, + result, + apply: true, + pr: false, + timeoutMs: 10_000, + }); + expect(delivered).toMatchObject({ + status: "ERROR", + applied_files: ["file.txt"], + patches: result.patches, + }); + expect(delivered.delivery_errors?.[0]).toContain("timed out"); + expect(await readFile(join(cwd, "file.txt"), "utf8")).toBe("after\n"); +}); + +test.each([ + { ...result, status: "PROCESSING" }, + { + ...result, + pending_user_input: { + id: "pending", + input_type: "file_change_approval", + data: {}, + }, + }, + { ...result, patches: [] }, +])( + "refuses delivery of incomplete or unapproved code changes %#", + async (ineligible) => { + const delivered = await deliverSeerFix({ + ...options, + result: ineligible, + apply: true, + }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("completed Seer fix"); + expect(delivered.patches).toEqual(ineligible.patches); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + expect(getSeerExplorerState).not.toHaveBeenCalled(); + }, +); + +test("returns the existing fix unchanged when delivery is skipped", async () => { + expect(await deliverSeerFix({ ...options, result, pr: false })).toEqual( + result, + ); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + expect(getSeerExplorerState).not.toHaveBeenCalled(); +}); + +test("reuses a completed PR without another creation request", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue(session("completed")); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.pr_urls).toEqual([ + "https://github.com/example/project/pull/1", + ]); + expect(delivered.status).toBe("COMPLETED"); + expect(getSeerExplorerState).toHaveBeenCalledOnce(); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test("awaits an already creating PR without another creation request", async () => { + vi.mocked(getSeerExplorerState) + .mockResolvedValueOnce(session("creating")) + .mockResolvedValue(session("completed")); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.status).toBe("COMPLETED"); + expect(delivered.pr_urls).toEqual([ + "https://github.com/example/project/pull/1", + ]); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + expect(getSeerExplorerState).toHaveBeenCalledTimes(2); +}); + +test("requests unique affected repositories once and preserves all PR URLs", async () => { + const multi: FixResult = { + ...result, + patches: [ + patch, + { ...patch, patch: { ...patch.patch, path: "second.txt" } }, + { ...patch, repo_name: "example/second" }, + ], + }; + vi.mocked(getSeerExplorerState) + .mockResolvedValueOnce(ready(multi.patches)) + .mockResolvedValueOnce(ready(multi.patches)) + .mockResolvedValue({ + ...ready(multi.patches), + repo_pr_states: { + "example/project": { + repo_name: "example/project", + pr_creation_status: "completed", + pr_url: "https://github.com/example/project/pull/1", + }, + "example/second": { + repo_name: "example/second", + pr_creation_status: "completed", + pr_url: "https://github.com/example/second/pull/2", + }, + }, + }); + const delivered = await deliverSeerFix({ ...options, result: multi }); + expect(delivered.status).toBe("COMPLETED"); + expect(delivered.pr_urls).toEqual([ + "https://github.com/example/project/pull/1", + "https://github.com/example/second/pull/2", + ]); + expect( + vi.mocked(apiRequestToRegion).mock.calls.map((call) => call[2]?.body), + ).toEqual([ + { payload: { type: "create_pr", repo_name: "example/project" } }, + { payload: { type: "create_pr", repo_name: "example/second" } }, + ]); + expect(getSeerExplorerState).toHaveBeenCalledTimes(3); +}); + +test("reports repository PR failure even though the conversation is completed", async () => { + const failed: ExplorerSession = { + ...ready(), + repo_pr_states: { + "example/project": { + repo_name: "example/project", + pr_creation_status: "error", + pr_creation_error: "permission denied", + }, + }, + }; + vi.mocked(getSeerExplorerState) + .mockResolvedValueOnce(ready()) + .mockResolvedValue(failed); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("permission denied"); + expect(delivered.patches).toEqual(result.patches); + expect(apiRequestToRegion).toHaveBeenCalledOnce(); +}); + +test("preserves a completed repository PR when another repository fails", async () => { + const multi = { + ...result, + patches: [patch, { ...patch, repo_name: "example/second" }], + }; + vi.mocked(getSeerExplorerState).mockResolvedValue({ + ...ready(multi.patches), + repo_pr_states: { + "example/project": { + repo_name: "example/project", + pr_creation_status: "completed", + pr_url: "https://github.com/example/project/pull/1", + }, + "example/second": { + repo_name: "example/second", + pr_creation_status: "error", + pr_creation_error: "Branch conflict", + }, + }, + }); + const delivered = await deliverSeerFix({ ...options, result: multi }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.pr_urls).toEqual([ + "https://github.com/example/project/pull/1", + ]); + expect(delivered.delivery_errors?.[0]).toContain("Branch conflict"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test("pauses delivery when fresh state introduces a pending user input", async () => { + const pending = { + id: "new-approval", + input_type: "file_change_approval", + data: {}, + }; + vi.mocked(getSeerExplorerState).mockResolvedValue({ + status: "WAITING_FOR_USER_RESPONSE", + pending_user_input: pending, + }); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered).toMatchObject({ + status: "ERROR", + pending_user_input: pending, + patches: result.patches, + }); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test("bounds polling without repeating an already creating PR request", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue(session("creating")); + const delivered = await deliverSeerFix({ ...options, result, timeoutMs: 10 }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("timed out"); + expect(delivered.patches).toEqual(result.patches); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test("bounds region resolution and prevents a POST after its timeout", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue(ready()); + vi.mocked(resolveOrgRegion).mockReturnValue(new Promise(() => {})); + const delivered = await deliverSeerFix({ ...options, result, timeoutMs: 10 }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("timed out"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test("does not submit a PR after the budget expires before its timeout callback runs", async () => { + const now = vi.spyOn(Date, "now").mockReturnValue(0); + vi.mocked(getSeerExplorerState) + .mockResolvedValueOnce(ready()) + .mockResolvedValue(session("completed")); + vi.mocked(resolveOrgRegion).mockImplementation(async () => { + now.mockReturnValue(100); + return "https://de.sentry.io"; + }); + const delivered = await deliverSeerFix({ ...options, result, timeoutMs: 50 }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("timed out"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test("honors caller cancellation before any delivery request", async () => { + const controller = new AbortController(); + controller.abort(); + const delivered = await deliverSeerFix({ + ...options, + result, + signal: controller.signal, + }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("cancelled"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); + expect(getSeerExplorerState).not.toHaveBeenCalled(); +}); + +test("bounds an uncertain creation POST and never repeats it", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue(ready()); + vi.mocked(apiRequestToRegion).mockReturnValue(new Promise(() => {})); + const delivered = await deliverSeerFix({ ...options, result, timeoutMs: 10 }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("timed out"); + expect(delivered.next_action).toContain("--run-id run-uuid"); + expect(apiRequestToRegion).toHaveBeenCalledOnce(); +}); + +test("redacts credentials in repository and network error diagnostics", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue(ready()); + vi.mocked(apiRequestToRegion).mockRejectedValue( + new Error("Authorization: Bearer secret-token sntryu_secret"), + ); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.error).not.toContain("secret-token"); + expect(delivered.error).not.toContain("sntryu_secret"); + expect(delivered.delivery_errors?.[0]).toContain("[REDACTED]"); +}); + +test("refuses missing conversation state without requesting creation", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue(null); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("Cannot find"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); + +test("refuses completed PR state missing its URL without requesting creation again", async () => { + vi.mocked(getSeerExplorerState).mockResolvedValue({ + ...ready(), + repo_pr_states: { + "example/project": { + repo_name: "example/project", + pr_creation_status: "completed", + }, + }, + }); + const delivered = await deliverSeerFix({ ...options, result }); + expect(delivered.status).toBe("ERROR"); + expect(delivered.delivery_errors?.[0]).toContain("without a PR URL"); + expect(apiRequestToRegion).not.toHaveBeenCalled(); +}); diff --git a/packages/cli/test/lib/seer-patch.test.ts b/packages/cli/test/lib/seer-patch.test.ts new file mode 100644 index 000000000..52ffda6a6 --- /dev/null +++ b/packages/cli/test/lib/seer-patch.test.ts @@ -0,0 +1,397 @@ +/** Real Git/filesystem coverage for local Seer patch application. */ +import { execFileSync } from "node:child_process"; +import { mkdir, readFile, symlink, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { applySeerPatches, type SeerPatch } from "../../src/lib/seer-patch.js"; +import { useTestConfigDir } from "../helpers.js"; + +const getDir = useTestConfigDir("seer-patch-"); +let repo: string; + +function git(args: string[]): string { + return execFileSync("git", args, { + cwd: repo, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + }); +} + +async function baseline(files: Record): Promise { + for (const [path, content] of Object.entries(files)) { + await writeFile(join(repo, path), content); + } + git(["add", "."]); + git([ + "-c", + "user.name=Audit", + "-c", + "user.email=audit@example.invalid", + "-c", + "commit.gpgsign=false", + "-c", + "core.hooksPath=", + "commit", + "-m", + "test baseline", + ]); +} + +function modify( + path = "file.txt", + before = "before", + after = "after", +): SeerPatch { + return { + repo_name: "example/project", + patch: { + path, + type: "M", + source_file: `a/${path}`, + target_file: `b/${path}`, + }, + diff: `diff --git a/${path} b/${path}\n--- a/${path}\n+++ b/${path}\n@@ -1 +1 @@\n-${before}\n+${after}\n`, + }; +} + +function addition(path: string): SeerPatch { + return { + repo_name: "example/project", + patch: { + path, + type: "A", + source_file: "/dev/null", + target_file: `b/${path}`, + }, + diff: `diff --git a/${path} b/${path}\nnew file mode 100644\n--- /dev/null\n+++ b/${path}\n@@ -0,0 +1 @@\n+new\n`, + }; +} + +beforeEach(async () => { + repo = join(getDir(), "repo"); + await mkdir(repo); + git(["init"]); + git(["config", "core.autocrlf", "false"]); + git(["remote", "add", "origin", "https://github.com/example/project.git"]); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("applySeerPatches", () => { + test.each([ + ["Foo.ts", "foo.ts"], + ["Dir/a.ts", "dir/b.ts"], + ["caf\u00e9.ts", "cafe\u0301.ts"], + ])( + "refuses portable filesystem aliases %s and %s before writing", + async (first, second) => { + await baseline({ "file.txt": "before\n" }); + await expect( + applySeerPatches([addition(first), addition(second)], repo), + ).rejects.toThrow(/alias/); + await expect(readFile(join(repo, first))).rejects.toMatchObject({ + code: "ENOENT", + }); + await expect(readFile(join(repo, second))).rejects.toMatchObject({ + code: "ENOENT", + }); + }, + ); + + test.each(["dirty", "untracked"])( + "preserves an existing %s file that aliases the patch path", + async (kind) => { + await baseline( + kind === "dirty" + ? { "file.txt": "before\n", "foo.ts": "base\n" } + : { "file.txt": "before\n" }, + ); + await writeFile(join(repo, "foo.ts"), "local work\n"); + await expect( + applySeerPatches([modify(), addition("Foo.ts")], repo), + ).rejects.toThrow(/alias/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + expect(await readFile(join(repo, "foo.ts"), "utf8")).toBe("local work\n"); + }, + ); + + test("does not write when its deadline expires during validation", async () => { + await baseline({ "file.txt": "before\n" }); + vi.spyOn(Date, "now").mockReturnValueOnce(0).mockReturnValue(100); + await expect( + applySeerPatches([modify()], repo, { deadline: 50 }), + ).rejects.toThrow(/timed out/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }); + + test("does not write when cancelled during filesystem validation", async () => { + await baseline({ "file.txt": "before\n" }); + const controller = new AbortController(); + queueMicrotask(() => controller.abort()); + await expect( + applySeerPatches([modify()], repo, { signal: controller.signal }), + ).rejects.toThrow(/cancelled/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }); + + test("applies a whole add/modify/delete batch from a nested cwd and preserves unrelated edits", async () => { + await baseline({ + "file with spaces.txt": "before\n", + "remove.txt": "remove\n", + "unrelated.txt": "base\n", + }); + await writeFile(join(repo, "unrelated.txt"), "local work\n"); + await mkdir(join(repo, "nested")); + const added: SeerPatch = { + repo_name: "example/project", + patch: { + path: "new.txt", + type: "A", + source_file: "/dev/null", + target_file: "b/new.txt", + }, + diff: "diff --git a/new.txt b/new.txt\nnew file mode 100644\n--- /dev/null\n+++ b/new.txt\n@@ -0,0 +1 @@\n+new\n", + }; + const deleted: SeerPatch = { + repo_name: "example/project", + patch: { + path: "remove.txt", + type: "D", + source_file: "a/remove.txt", + target_file: "/dev/null", + }, + diff: "diff --git a/remove.txt b/remove.txt\ndeleted file mode 100644\n--- a/remove.txt\n+++ /dev/null\n@@ -1 +0,0 @@\n-remove\n", + }; + expect( + await applySeerPatches( + [modify("file with spaces.txt"), added, deleted], + join(repo, "nested"), + ), + ).toEqual({ applied: ["file with spaces.txt", "new.txt", "remove.txt"] }); + expect(await readFile(join(repo, "file with spaces.txt"), "utf8")).toBe( + "after\n", + ); + expect(await readFile(join(repo, "new.txt"), "utf8")).toBe("new\n"); + await expect(readFile(join(repo, "remove.txt"))).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(await readFile(join(repo, "unrelated.txt"), "utf8")).toBe( + "local work\n", + ); + }); + + test("checks every patch before changing any files", async () => { + await baseline({ "file.txt": "before\n", "other.txt": "before\n" }); + await expect( + applySeerPatches([modify(), modify("other.txt", "wrong context")], repo), + ).rejects.toThrow(/cleanly/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + expect(await readFile(join(repo, "other.txt"), "utf8")).toBe("before\n"); + }); + + test.each(["unstaged", "staged", "untracked", "ignored"])( + "preserves %s files impacted by a patch", + async (kind) => { + await baseline({ "file.txt": "before\n", "other.txt": "before\n" }); + const path = + kind === "untracked" || kind === "ignored" ? "new.txt" : "other.txt"; + if (kind === "ignored") { + await writeFile(join(repo, ".gitignore"), "new.txt\n"); + } + await writeFile(join(repo, path), "local work\n"); + if (kind === "staged") { + git(["add", path]); + } + const conflicting = + path === "new.txt" + ? { + repo_name: "example/project", + patch: { path, type: "A" as const }, + diff: "diff --git a/new.txt b/new.txt\nnew file mode 100644\n--- /dev/null\n+++ b/new.txt\n@@ -0,0 +1 @@\n+new\n", + } + : modify(path, "local work"); + await expect( + applySeerPatches([modify(), conflicting], repo), + ).rejects.toThrow(/local changes|untracked/); + expect(await readFile(join(repo, path), "utf8")).toBe("local work\n"); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }, + ); + + test("protects dirty files marked assume-unchanged", async () => { + await baseline({ "file.txt": "before\n" }); + git(["update-index", "--assume-unchanged", "file.txt"]); + await writeFile(join(repo, "file.txt"), "local work\n"); + await expect( + applySeerPatches([modify("file.txt", "local work")], repo), + ).rejects.toThrow(); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("local work\n"); + }); + + test("refuses mismatched repositories before applying a matching patch", async () => { + await baseline({ "file.txt": "before\n", "other.txt": "before\n" }); + await expect( + applySeerPatches( + [modify(), { ...modify("other.txt"), repo_name: "other/project" }], + repo, + ), + ).rejects.toThrow(/repository|repositories/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }); + + test("accepts matching upstream when origin names a fork", async () => { + await baseline({ "file.txt": "before\n" }); + git(["remote", "set-url", "origin", "git@github.com:fork/project.git"]); + git(["remote", "add", "upstream", "git@github.com:example/project.git"]); + expect(await applySeerPatches([modify()], repo)).toEqual({ + applied: ["file.txt"], + }); + }); + + test("refuses a multi-repository batch even when both repositories match remotes", async () => { + await baseline({ "file.txt": "before\n", "other.txt": "before\n" }); + git(["remote", "add", "upstream", "git@github.com:upstream/project.git"]); + await expect( + applySeerPatches( + [modify(), { ...modify("other.txt"), repo_name: "upstream/project" }], + repo, + ), + ).rejects.toThrow(/one repository|multi-repository/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + expect(await readFile(join(repo, "other.txt"), "utf8")).toBe("before\n"); + }); + + test.each([ + "../outside.txt", + "/outside.txt", + "C:/outside.txt", + "C:outside.txt", + "folder\\outside.txt", + ".git/config", + "folder/.GiT/config", + ])("rejects unsafe metadata path %s before writing", async (path) => { + await baseline({ "file.txt": "before\n" }); + await expect( + applySeerPatches([modify(), modify(path)], repo), + ).rejects.toThrow(/Unsafe/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }); + + test("rejects diff headers that target a different file than metadata", async () => { + await baseline({ "file.txt": "before\n", "hidden.txt": "before\n" }); + const misleading = { ...modify(), diff: modify("hidden.txt").diff }; + await expect(applySeerPatches([misleading], repo)).rejects.toThrow(/match/); + expect(await readFile(join(repo, "hidden.txt"), "utf8")).toBe("before\n"); + }); + + test("rejects a second file hidden after the first file's hunks", async () => { + await baseline({ "file.txt": "before\n", "hidden.txt": "before\n" }); + const hidden = modify("hidden.txt").diff.replace(/^diff --git[^\n]+\n/, ""); + await expect( + applySeerPatches([{ ...modify(), diff: modify().diff + hidden }], repo), + ).rejects.toThrow(); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + expect(await readFile(join(repo, "hidden.txt"), "utf8")).toBe("before\n"); + }); + + test("rejects unsafe diff paths even with benign metadata", async () => { + await baseline({ "file.txt": "before\n" }); + await expect( + applySeerPatches( + [{ ...modify(), diff: modify(".git/config").diff }], + repo, + ), + ).rejects.toThrow(/Unsafe/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }); + + test("rejects symlink payloads", async () => { + await baseline({ "file.txt": "before\n" }); + const symlinkPatch: SeerPatch = { + repo_name: "example/project", + patch: { path: "link", type: "A" }, + diff: "diff --git a/link b/link\nnew file mode 120000\n--- /dev/null\n+++ b/link\n@@ -0,0 +1 @@\n+/outside\n", + }; + await expect( + applySeerPatches([modify(), symlinkPatch], repo), + ).rejects.toThrow(/symlinks/); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }); + + test("rejects paths through a symlink or Windows junction", async () => { + await baseline({ "file.txt": "before\n" }); + const outside = join(getDir(), "outside"); + await mkdir(outside); + await writeFile(join(outside, "file.txt"), "before\n"); + await symlink(outside, join(repo, "linked"), "junction"); + await expect( + applySeerPatches([modify(), modify("linked/file.txt")], repo), + ).rejects.toThrow(/symlink/); + expect(await readFile(join(outside, "file.txt"), "utf8")).toBe("before\n"); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("before\n"); + }); + + test("applies a pure rename with spaces and protects both paths", async () => { + await baseline({ "old name.txt": "before\n" }); + const renamed: SeerPatch = { + repo_name: "example/project", + patch: { + path: "new name.txt", + type: "M", + source_file: "a/old name.txt", + target_file: "b/new name.txt", + }, + diff: "diff --git a/old name.txt b/new name.txt\nsimilarity index 100%\nrename from old name.txt\nrename to new name.txt\n", + }; + expect(await applySeerPatches([renamed], repo)).toEqual({ + applied: ["old name.txt", "new name.txt"], + }); + expect(await readFile(join(repo, "new name.txt"), "utf8")).toBe("before\n"); + await expect(readFile(join(repo, "old name.txt"))).rejects.toMatchObject({ + code: "ENOENT", + }); + }); + + test("preserves CRLF content while applying a matching patch", async () => { + await baseline({ "file.txt": "before\r\n" }); + expect( + await applySeerPatches([modify("file.txt", "before\r", "after\r")], repo), + ).toEqual({ applied: ["file.txt"] }); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("after\r\n"); + }); + + test("respects Git's Windows CRLF checkout conversion", async () => { + git(["config", "core.autocrlf", "true"]); + await baseline({ "file.txt": "before\r\n" }); + expect(await applySeerPatches([modify()], repo)).toEqual({ + applied: ["file.txt"], + }); + expect(await readFile(join(repo, "file.txt"), "utf8")).toBe("after\r\n"); + expect(git(["show", ":file.txt"])).toBe("before\n"); + }); + + test.each(["source", "target"])( + "protects a dirty rename %s", + async (side) => { + await baseline({ "old.txt": "before\n" }); + const path = side === "source" ? "old.txt" : "new.txt"; + await writeFile(join(repo, path), "local work\n"); + const renamed: SeerPatch = { + repo_name: "example/project", + patch: { + path: "new.txt", + type: "M", + source_file: "a/old.txt", + target_file: "b/new.txt", + }, + diff: "diff --git a/old.txt b/new.txt\nsimilarity index 100%\nrename from old.txt\nrename to new.txt\n", + }; + await expect(applySeerPatches([renamed], repo)).rejects.toThrow( + /local changes|untracked/, + ); + expect(await readFile(join(repo, path), "utf8")).toBe("local work\n"); + }, + ); +});