From 9034a3df60c339e11665bd630390e115e7805306 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 18 May 2026 00:17:32 +0800 Subject: [PATCH 01/25] fix(review): validate provider evidence --- CHANGELOG.md | 2 + docs/code-review.md | 2 + src/app.ts | 8 ++- src/provider.ts | 23 +++++- src/review-validation.test.ts | 106 +++++++++++++++++++++++++++ src/review-validation.ts | 130 ++++++++++++++++++++++++++++++++++ 6 files changed, 268 insertions(+), 3 deletions(-) create mode 100644 src/review-validation.test.ts create mode 100644 src/review-validation.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eeffed..9e25c5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## 0.3.1 - Unreleased +- Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. + ## 0.3.0 - 2026-05-18 - Added a `pi` provider for routing review, fix, revalidate, and agent map through the [pi coding agent](https://pi.dev) in non-interactive print mode, thanks @danielmarbach. diff --git a/docs/code-review.md b/docs/code-review.md index c5c3c70..f2dba85 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -25,6 +25,8 @@ Current behavior: - builds bounded prompt context from owned files, context files, and tests - calls the configured provider - requires strict JSON output +- rejects findings whose evidence cites files outside the prompt context, stale + line ranges, or quotes that do not match current file contents - writes findings under `.clawpatch/findings/` - appends analysis history to the feature record - releases the feature lock diff --git a/src/app.ts b/src/app.ts index 95e7b1a..b4d10b5 100644 --- a/src/app.ts +++ b/src/app.ts @@ -32,6 +32,7 @@ import { renderFindingDetail, renderReport, } from "./reporting.js"; +import { validateReviewOutput } from "./review-validation.js"; import { filterFeaturesByChangedFiles, filterFeaturesByProject, @@ -641,7 +642,12 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId mode, customPrompt, ); - const output = await provider.review(loaded.root, prompt, providerOptions(config)); + const output = await validateReviewOutput( + loaded.root, + lockedFeature, + config, + await provider.review(loaded.root, prompt, providerOptions(config)), + ); const modeFindings = reviewFindingsForMode(output.findings, mode); const records = modeFindings .slice(0, config.review.maxFindingsPerFeature) diff --git a/src/provider.ts b/src/provider.ts index 1efacf1..e784252 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -361,6 +361,9 @@ const mockProvider: Provider = { if (!prompt.includes("TODO_BUG") && !prompt.includes("BUG:")) { return { findings: [], inspected: { files: [], symbols: [], notes: ["mock clean"] } }; } + const evidencePath = prompt.includes("BAD_EVIDENCE") + ? "src/not-included.ts" + : (firstPromptFileWith(prompt, "TODO_BUG") ?? "src/index.ts"); return { findings: [ { @@ -370,7 +373,7 @@ const mockProvider: Provider = { confidence: "high", evidence: [ { - path: "src/index.ts", + path: evidencePath, startLine: null, endLine: null, symbol: null, @@ -386,7 +389,7 @@ const mockProvider: Provider = { minimumFixScope: "Replace the marker in the owning feature file.", }, ], - inspected: { files: ["src/index.ts"], symbols: [], notes: ["mock finding"] }, + inspected: { files: [evidencePath], symbols: [], notes: ["mock finding"] }, }; }, async fix(): Promise { @@ -417,6 +420,22 @@ const mockProvider: Provider = { }, }; +function firstPromptFileWith(prompt: string, marker: string): string | null { + const blocks = prompt.split(/^--- /gmu).slice(1); + for (const block of blocks) { + const newline = block.indexOf("\n"); + if (newline === -1) { + continue; + } + const path = block.slice(0, newline).trim(); + const contents = block.slice(newline + 1); + if (path.length > 0 && contents.includes(marker)) { + return path; + } + } + return null; +} + const mockFailProvider: Provider = { name: "mock-fail", async check(): Promise { diff --git a/src/review-validation.test.ts b/src/review-validation.test.ts new file mode 100644 index 0000000..745762b --- /dev/null +++ b/src/review-validation.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it } from "vitest"; +import { defaultConfig } from "./config.js"; +import { validateReviewOutput } from "./review-validation.js"; +import { fixtureRoot, writeFixture } from "./test-helpers.js"; +import type { FeatureRecord, ReviewOutput } from "./types.js"; + +describe("validateReviewOutput", () => { + it("accepts evidence that points at included files, existing lines, and matching quotes", async () => { + const root = await fixtureRoot("clawpatch-review-validation-ok-"); + await writeFixture(root, "src/index.ts", "const value = 'TODO_BUG';\n"); + + await expect( + validateReviewOutput(root, feature("src/index.ts"), defaultConfig(), output("src/index.ts")), + ).resolves.toMatchObject({ findings: [{ title: "Bug" }] }); + }); + + it("rejects evidence for files that were not included in review context", async () => { + const root = await fixtureRoot("clawpatch-review-validation-path-"); + await writeFixture(root, "src/index.ts", "const value = 'TODO_BUG';\n"); + await writeFixture(root, "src/other.ts", "const value = 'TODO_BUG';\n"); + + await expect( + validateReviewOutput(root, feature("src/index.ts"), defaultConfig(), output("src/other.ts")), + ).rejects.toMatchObject({ code: "malformed-output" }); + }); + + it("rejects stale line ranges and quotes that do not match current file text", async () => { + const root = await fixtureRoot("clawpatch-review-validation-content-"); + await writeFixture(root, "src/index.ts", "const value = 'real';\n"); + + await expect( + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + output("src/index.ts", { startLine: 9, endLine: 9, quote: "real" }), + ), + ).rejects.toMatchObject({ code: "malformed-output" }); + + await expect( + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + output("src/index.ts", { startLine: 1, endLine: 1, quote: "missing" }), + ), + ).rejects.toMatchObject({ code: "malformed-output" }); + }); +}); + +function feature(path: string): FeatureRecord { + return { + schemaVersion: 1, + featureId: "feat_test", + title: "Test feature", + summary: "Test feature.", + kind: "library", + source: "test", + confidence: "high", + entrypoints: [{ path, symbol: null, route: null, command: null }], + ownedFiles: [{ path, reason: "test" }], + contextFiles: [], + tests: [], + tags: [], + trustBoundaries: [], + status: "pending", + lock: null, + findingIds: [], + patchAttemptIds: [], + analysisHistory: [], + createdAt: "2026-05-18T00:00:00.000Z", + updatedAt: "2026-05-18T00:00:00.000Z", + }; +} + +function output( + path: string, + evidence: { startLine?: number | null; endLine?: number | null; quote?: string | null } = {}, +): ReviewOutput { + return { + findings: [ + { + title: "Bug", + category: "bug", + severity: "medium", + confidence: "high", + evidence: [ + { + path, + startLine: evidence.startLine ?? 1, + endLine: evidence.endLine ?? 1, + symbol: null, + quote: evidence.quote ?? "TODO_BUG", + }, + ], + reasoning: "Reason.", + reproduction: null, + recommendation: "Fix it.", + whyTestsDoNotAlreadyCoverThis: "No test.", + suggestedRegressionTest: null, + minimumFixScope: "Small.", + }, + ], + inspected: { files: [path], symbols: [], notes: [] }, + }; +} diff --git a/src/review-validation.ts b/src/review-validation.ts new file mode 100644 index 0000000..e62f3ec --- /dev/null +++ b/src/review-validation.ts @@ -0,0 +1,130 @@ +import { readFile, realpath } from "node:fs/promises"; +import { isAbsolute, relative, resolve } from "node:path"; +import { ClawpatchError } from "./errors.js"; +import { ClawpatchConfig, FeatureRecord, ReviewOutput } from "./types.js"; + +export async function validateReviewOutput( + root: string, + feature: FeatureRecord, + config: ClawpatchConfig, + output: ReviewOutput, +): Promise { + const included = includedReviewPaths(feature, config); + const cache = new Map>(); + for (const file of output.inspected.files) { + assertIncludedPath(file, included, "inspected file"); + } + for (const finding of output.findings) { + if (finding.evidence.length === 0) { + throwMalformed(`finding "${finding.title}" has no evidence`); + } + for (const evidence of finding.evidence) { + assertIncludedPath(evidence.path, included, "evidence file"); + const contents = await fileContents(root, evidence.path, cache); + assertLineRange(contents, evidence); + assertQuote(contents, evidence); + } + } + return output; +} + +function includedReviewPaths(feature: FeatureRecord, config: ClawpatchConfig): Set { + return new Set( + [ + ...feature.ownedFiles.slice(0, config.review.maxOwnedFiles).map((ref) => ref.path), + ...feature.contextFiles.slice(0, config.review.maxContextFiles).map((ref) => ref.path), + ].map(normalizePath), + ); +} + +function assertIncludedPath(path: string, included: ReadonlySet, label: string): void { + const normalized = normalizePath(path); + if (normalized.startsWith("../") || isAbsolute(normalized)) { + throwMalformed(`${label} escapes repository root: ${path}`); + } + if (!included.has(normalized)) { + throwMalformed(`${label} was not included in review context: ${path}`); + } +} + +async function fileContents( + root: string, + path: string, + cache: Map>, +): Promise { + const normalized = normalizePath(path); + const existing = cache.get(normalized); + if (existing !== undefined) { + return existing; + } + const loaded = readIncludedFile(root, normalized); + cache.set(normalized, loaded); + return loaded; +} + +async function readIncludedFile(root: string, path: string): Promise { + const full = resolve(root, path); + const realRoot = await realpath(root).catch(() => root); + const realFull = await realpath(full).catch(() => null); + if (realFull === null || !isInside(realRoot, realFull)) { + throwMalformed(`evidence file is not readable inside repository: ${path}`); + } + return readFile(full, "utf8").catch(() => { + throwMalformed(`evidence file is not readable inside repository: ${path}`); + }); +} + +function assertLineRange( + contents: string, + evidence: ReviewOutput["findings"][number]["evidence"][number], +): void { + const { startLine, endLine } = evidence; + if (startLine === null && endLine === null) { + return; + } + if (startLine === null || endLine === null) { + throwMalformed(`evidence line range must include both startLine and endLine: ${evidence.path}`); + } + if (startLine > endLine) { + throwMalformed(`evidence line range is inverted: ${evidence.path}:${startLine}-${endLine}`); + } + const lineCount = contents.length === 0 ? 1 : contents.split("\n").length; + if (endLine > lineCount) { + throwMalformed( + `evidence line range exceeds file length: ${evidence.path}:${startLine}-${endLine}`, + ); + } +} + +function assertQuote( + contents: string, + evidence: ReviewOutput["findings"][number]["evidence"][number], +): void { + const quote = evidence.quote; + if (quote === null || quote.trim().length === 0) { + return; + } + if ( + !contents.includes(quote) && + !compactWhitespace(contents).includes(compactWhitespace(quote)) + ) { + throwMalformed(`evidence quote does not match file contents: ${evidence.path}`); + } +} + +function isInside(root: string, candidate: string): boolean { + const relativePath = relative(root, candidate); + return relativePath === "" || (!relativePath.startsWith("..") && !isAbsolute(relativePath)); +} + +function normalizePath(path: string): string { + return path.replace(/\\/gu, "/").replace(/^\.\/+/u, ""); +} + +function compactWhitespace(value: string): string { + return value.replace(/\s+/gu, " ").trim(); +} + +function throwMalformed(message: string): never { + throw new ClawpatchError(`malformed provider review output: ${message}`, 8, "malformed-output"); +} From 5424e406545fd200d5209d550bb3b6a7b2f7f274 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 18 May 2026 00:20:26 +0800 Subject: [PATCH 02/25] feat(ci): add review artifact command --- CHANGELOG.md | 1 + README.md | 2 + docs/code-review.md | 13 +++++ src/app.ts | 112 ++++++++++++++++++++++++++++++++++++++++++- src/cli.ts | 31 ++++++++++++ src/workflow.test.ts | 68 ++++++++++++++++++++++++++ 6 files changed, 226 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e25c5b..e0a1e2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.3.1 - Unreleased +- Added `clawpatch ci` to initialize, map, review, write a report, and append a GitHub Actions step summary in one CI-friendly command. - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. ## 0.3.0 - 2026-05-18 diff --git a/README.md b/README.md index 6da095a..baa8aee 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ clawpatch init clawpatch map clawpatch review --limit 3 --jobs 3 clawpatch review --mode deslopify --limit 3 +clawpatch ci --since origin/main --output clawpatch-report.md clawpatch report clawpatch next clawpatch show --finding @@ -122,6 +123,7 @@ Supported provider names today: - `clawpatch status`: show project, dirty state, feature/finding counts - `clawpatch review`: review pending or selected features - `clawpatch review --mode deslopify`: review only for locally provable slop cleanup +- `clawpatch ci`: initialize if needed, map, review, write a report, and append a GitHub step summary - `clawpatch report`: print or write a Markdown findings report - `clawpatch next`: print the next actionable finding - `clawpatch show --finding `: inspect one finding with evidence and suggested validation diff --git a/docs/code-review.md b/docs/code-review.md index f2dba85..a3046b5 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -49,6 +49,19 @@ If no features are touched by the diff, `review` exits cleanly with no findings. The same flag is available on `revalidate`; revalidation scopes open findings to features whose owned files changed. +### CI command + +Use `clawpatch ci` when a GitHub Actions job should run the whole read-only +review loop: + +```bash +clawpatch ci --since origin/main --limit 20 --jobs 4 --output clawpatch-report.md +``` + +The command initializes `.clawpatch/` if needed, maps features, reviews the +selected feature set, writes a Markdown report when `--output` is provided, and +appends a compact summary to `GITHUB_STEP_SUMMARY` when that file is available. + Progress uses stderr so `--json` stdout remains machine-readable. The worker pool is per-process, and lock files under `.clawpatch/locks/` prevent overlapping review processes from claiming the same feature. Interrupted runs diff --git a/src/app.ts b/src/app.ts index b4d10b5..0993a01 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,4 +1,4 @@ -import { readFile, writeFile } from "node:fs/promises"; +import { appendFile, readFile, writeFile } from "node:fs/promises"; import { join, resolve } from "node:path"; import { hostname } from "node:os"; import { @@ -233,6 +233,37 @@ export async function statusCommand(context: AppContext): Promise { }; } +export async function ciCommand( + context: AppContext, + flags: Record, +): Promise { + const initialized = await ensureInitialized(context); + const mapFlags = providerFlagSubset(flags); + const reviewFlags = reviewFlagSubset(flags); + const mapped = await mapCommand(context, mapFlags); + const reviewed = await reviewCommand(context, reviewFlags); + const reportFlags = reportFlagSubset(flags); + const report = (await reportCommand(context, reportFlags)) as { + findings?: number; + output?: string | null; + markdown?: string; + }; + const summary = renderCiSummary({ initialized, mapped, reviewed, report }); + const githubStepSummary = process.env["GITHUB_STEP_SUMMARY"]; + if (githubStepSummary !== undefined && githubStepSummary.length > 0) { + await appendFile(githubStepSummary, summary, "utf8"); + } + return { + initialized, + mapped: numberField(mapped, "features"), + reviewed: numberField(reviewed, "reviewed"), + findings: numberField(reviewed, "findings") ?? report.findings ?? 0, + report: report.output ?? null, + githubStepSummary: githubStepSummary ?? null, + next: stringField(reviewed, "next") ?? "clawpatch status", + }; +} + export async function reviewCommand( context: AppContext, flags: Record, @@ -1055,6 +1086,17 @@ async function loadProjectState(context: AppContext) { return { root: context.root, config, paths, project }; } +async function ensureInitialized(context: AppContext): Promise { + const config = await loadConfig(context.root, context.options); + const paths = statePaths(resolveStateDir(context.root, config)); + if ((await readProject(paths)) !== null) { + await ensureStateDirs(paths); + return false; + } + await initCommand(context, {}); + return true; +} + function applyProviderFlags( config: Awaited>, flags: Record, @@ -1074,6 +1116,74 @@ function applyProviderFlags( }; } +function providerFlagSubset(flags: Record): Record { + const subset: Record = {}; + for (const flag of ["provider", "model", "reasoningEffort"] as const) { + const value = stringFlag(flags, flag); + if (value !== undefined) { + subset[flag] = value; + } + } + return subset; +} + +function reviewFlagSubset(flags: Record): Record { + const subset = providerFlagSubset(flags); + for (const flag of ["since", "limit", "jobs"] as const) { + const value = stringFlag(flags, flag); + if (value !== undefined) { + subset[flag] = value; + } + } + return subset; +} + +function reportFlagSubset(flags: Record): Record { + const output = stringFlag(flags, "output"); + return output === undefined ? {} : { output }; +} + +function renderCiSummary(input: { + initialized: boolean; + mapped: unknown; + reviewed: unknown; + report: { findings?: number; output?: string | null }; +}): string { + const lines = [ + "## Clawpatch review", + "", + `- initialized: ${input.initialized ? "yes" : "no"}`, + `- mapped features: ${numberField(input.mapped, "features") ?? "unknown"}`, + `- reviewed features: ${numberField(input.reviewed, "reviewed") ?? 0}`, + `- findings: ${numberField(input.reviewed, "findings") ?? input.report.findings ?? 0}`, + ]; + if (input.report.output !== undefined && input.report.output !== null) { + lines.push(`- report: ${input.report.output}`); + } + const next = stringField(input.reviewed, "next"); + if (next !== undefined) { + lines.push(`- next: \`${next}\``); + } + lines.push(""); + return `${lines.join("\n")}\n`; +} + +function numberField(value: unknown, field: string): number | null { + if (typeof value !== "object" || value === null) { + return null; + } + const candidate = (value as Record)[field]; + return typeof candidate === "number" ? candidate : null; +} + +function stringField(value: unknown, field: string): string | undefined { + if (typeof value !== "object" || value === null) { + return undefined; + } + const candidate = (value as Record)[field]; + return typeof candidate === "string" ? candidate : undefined; +} + function providerOptions(config: ReturnType) { return { model: config.provider.model, diff --git a/src/cli.ts b/src/cli.ts index 894918d..9f6b721 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -4,6 +4,7 @@ import { createRequire } from "node:module"; import { pathToFileURL } from "node:url"; import { cleanLocksCommand, + ciCommand, doctorCommand, fixCommand, initCommand, @@ -51,6 +52,8 @@ async function dispatch( return statusCommand(context); case "review": return reviewCommand(context, flags); + case "ci": + return ciCommand(context, flags); case "report": return reportCommand(context, flags); case "show": @@ -163,6 +166,15 @@ const commandFlags = { "promptFile", "exportTribunalLedger", ]), + ci: new Set([ + "limit", + "since", + "jobs", + "provider", + "model", + "reasoningEffort", + "output", + ]), report: new Set(["status", "severity", "feature", "project", "category", "triage", "output"]), show: new Set(["finding"]), next: new Set(["status", "project"]), @@ -420,6 +432,24 @@ Flags: --triage --output --json +`); + return; + } + if (command === "ci") { + process.stdout.write(`clawpatch ci + +Usage: + clawpatch ci [flags] + +Flags: + --since + --limit + --jobs default: 10 + --provider + --model + --reasoning-effort + --output + --json `); return; } @@ -579,6 +609,7 @@ Commands: map status review + ci report show next diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 0ac0482..584af9b 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -15,6 +15,7 @@ import { delimiter, join } from "node:path"; import { fixCommand, cleanLocksCommand, + ciCommand, doctorCommand, initCommand, makeContext, @@ -266,6 +267,24 @@ describe("workflow", () => { expect(() => parseArgs(["review", "--mode", "slop"])).toThrow( "invalid --mode; expected default or deslopify", ); + expect( + parseArgs([ + "ci", + "--since", + "origin/main", + "--limit", + "2", + "--jobs", + "1", + "--output", + "report.md", + ]).flags, + ).toMatchObject({ + since: "origin/main", + limit: "2", + jobs: "1", + output: "report.md", + }); expect(parseArgs(["revalidate", "--since", "origin/main"]).flags).toMatchObject({ since: "origin/main", }); @@ -395,6 +414,55 @@ describe("workflow", () => { delete process.env["CLAWPATCH_PROVIDER"]; }); + it("runs CI review flow and appends a GitHub step summary", async () => { + const root = await fixtureRoot("clawpatch-ci-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ + name: "ci-flow", + bin: { app: "src/index.ts" }, + scripts: { test: "vitest run" }, + }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + const summaryPath = join(root, "summary.md"); + const reportPath = join(root, "review.md"); + const previousProvider = process.env["CLAWPATCH_PROVIDER"]; + const previousSummary = process.env["GITHUB_STEP_SUMMARY"]; + process.env["CLAWPATCH_PROVIDER"] = "mock"; + process.env["GITHUB_STEP_SUMMARY"] = summaryPath; + try { + const context = await makeContext(testOptions(root)); + const result = await ciCommand(context, { limit: "1", jobs: "1", output: reportPath }); + const summary = await readFile(summaryPath, "utf8"); + const report = await readFile(reportPath, "utf8"); + + expect(result).toMatchObject({ + initialized: true, + mapped: expect.any(Number), + reviewed: 1, + findings: 1, + report: reportPath, + githubStepSummary: summaryPath, + }); + expect(summary).toContain("## Clawpatch review"); + expect(summary).toContain("- findings: 1"); + expect(report).toContain("# clawpatch report"); + } finally { + if (previousProvider === undefined) { + delete process.env["CLAWPATCH_PROVIDER"]; + } else { + process.env["CLAWPATCH_PROVIDER"] = previousProvider; + } + if (previousSummary === undefined) { + delete process.env["GITHUB_STEP_SUMMARY"]; + } else { + process.env["GITHUB_STEP_SUMMARY"] = previousSummary; + } + } + }); + it.runIf(process.platform !== "win32")( "reviews end-to-end when codex writes fenced JSON with trailing prose", async () => { From 56469984d7c0c0988fcdf7b733b035056c12d8ea Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 18 May 2026 00:23:53 +0800 Subject: [PATCH 03/25] feat(pr): open patch attempts as pull requests --- CHANGELOG.md | 1 + README.md | 2 + docs/patching.md | 15 ++- docs/spec.md | 6 +- src/app.ts | 312 ++++++++++++++++++++++++++++++++++++++++++- src/cli.ts | 29 ++++ src/workflow.test.ts | 110 ++++++++++++++- 7 files changed, 467 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0a1e2c..e9c1d88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 0.3.1 - Unreleased - Added `clawpatch ci` to initialize, map, review, write a report, and append a GitHub Actions step summary in one CI-friendly command. +- Added `clawpatch open-pr --patch ` to turn an applied patch attempt into an explicit GitHub pull request. - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. ## 0.3.0 - 2026-05-18 diff --git a/README.md b/README.md index baa8aee..8653aa7 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ clawpatch next clawpatch show --finding clawpatch triage --finding --status false-positive --note "covered by tests" clawpatch fix --finding +clawpatch open-pr --patch --draft clawpatch revalidate --finding clawpatch revalidate --all --status open ``` @@ -129,6 +130,7 @@ Supported provider names today: - `clawpatch show --finding `: inspect one finding with evidence and suggested validation - `clawpatch triage --finding --status `: mark a finding with optional history note - `clawpatch fix --finding `: run the explicit patch loop for one finding +- `clawpatch open-pr --patch `: commit an applied patch attempt and open a GitHub PR - `clawpatch revalidate --finding `: re-check one finding - `clawpatch revalidate --all`: re-check open findings with report-style filters - `clawpatch doctor`: check provider availability diff --git a/docs/patching.md b/docs/patching.md index c3ebd2b..ea19abc 100644 --- a/docs/patching.md +++ b/docs/patching.md @@ -34,10 +34,23 @@ Status updates: The CLI does not currently mark a finding `fixed` from the patch pass alone. Use `clawpatch revalidate --finding ` for a second pass. +## Opening a PR + +After reviewing the applied worktree changes, create a GitHub PR explicitly: + +```bash +clawpatch open-pr --patch --draft +``` + +`open-pr` requires an applied or validated patch attempt with recorded changed +files. It refuses failed validation unless `--force` is passed, commits only the +recorded patch files, pushes the branch, and calls the GitHub CLI. Use +`--dry-run` to preview the branch, title, body, and commands without touching +git. + Not implemented yet: - fixing by severity or category - batching multiple findings - auto-commit -- PR creation - rollback snapshots diff --git a/docs/spec.md b/docs/spec.md index e139ee1..85d14ff 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -330,10 +330,8 @@ Behavior: - Requires existing patch attempt with changed files. - Requires clean validation state unless `--force`. - Creates branch/commit/PR only after explicit command. -- Uses repo-native commit helper if configured. -- PR body includes findings, tests, revalidation, and links to state report. - -Post-v0. +- Uses the GitHub CLI to create the PR after committing the recorded patch files. +- PR body includes linked findings, changed files, validation output, and the patch plan. ### `clawpatch land` diff --git a/src/app.ts b/src/app.ts index 0993a01..60a412a 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,5 +1,5 @@ import { appendFile, readFile, writeFile } from "node:fs/promises"; -import { join, resolve } from "node:path"; +import { join, relative, resolve } from "node:path"; import { hostname } from "node:os"; import { changedPathsBetweenSnapshots, @@ -9,7 +9,7 @@ import { import { loadConfig, resolveStateDir, GlobalOptions } from "./config.js"; import { detectProject } from "./detect.js"; import { ClawpatchError, assertDefined } from "./errors.js"; -import { runCommand } from "./exec.js"; +import { runCommand, runCommandArgs } from "./exec.js"; import { appendFindingHistory, findingFromOutput, @@ -1021,6 +1021,98 @@ export async function fixCommand( }; } +export async function openPrCommand( + context: AppContext, + flags: Record, +): Promise { + const loaded = await loadProjectState(context); + const patchId = assertDefined(stringFlag(flags, "patch"), "missing --patch"); + const patches = await readPatchAttempts(loaded.paths); + const patch = assertDefined( + patches.find((candidate) => candidate.patchAttemptId === patchId), + `patch attempt not found: ${patchId}`, + ); + const force = flags["force"] === true; + validatePrPatch(patch, force); + const git = await discoverGit(loaded.root); + if (git.root === null) { + throw new ClawpatchError("open-pr requires a git repository", 2, "not-git-repository"); + } + const base = stringFlag(flags, "base") ?? git.defaultBranch ?? "main"; + const branch = prBranchName(patch, stringFlag(flags, "branch"), git.currentBranch, base); + const findings = await readFindings(loaded.paths); + const linkedFindings = findings.filter((finding) => patch.findingIds.includes(finding.findingId)); + const title = prTitle(stringFlag(flags, "title"), linkedFindings, patch); + const body = renderPatchPrBody(patch, linkedFindings); + const gitFiles = gitRelativePatchFiles(git.root, loaded.root, patch.filesChanged); + const commands = plannedPrCommands(patch, branch, base, title); + if (flags["dryRun"] === true) { + return { + dryRun: true, + patchAttempt: patch.patchAttemptId, + branch, + base, + title, + body, + commands, + }; + } + + await assertPatchWorktree(patch, git.root, loaded.paths.stateDir, gitFiles, force); + let commitSha = patch.git.commitSha; + if (commitSha === null) { + if (git.currentBranch !== branch) { + await checkedRun("git switch", runCommandArgs("git", ["switch", "-c", branch], git.root)); + } + await checkedRun("git add", runCommandArgs("git", ["add", "--", ...gitFiles], git.root)); + await checkedRun( + "git commit", + runCommandArgs("git", ["commit", "-m", title], git.root), + ); + const commit = await checkedRun( + "git rev-parse", + runCommandArgs("git", ["rev-parse", "HEAD"], git.root), + ); + commitSha = commit.stdout.trim(); + } + await checkedRun("git push", runCommandArgs("git", ["push", "-u", "origin", branch], git.root)); + const ghArgs = [ + "pr", + "create", + "--base", + base, + "--head", + branch, + "--title", + title, + "--body-file", + "-", + ]; + if (flags["draft"] === true) { + ghArgs.push("--draft"); + } + const gh = await checkedRun("gh pr create", runCommandArgs(githubCli(), ghArgs, git.root, body)); + const prUrl = firstUrl(gh.stdout) ?? gh.stdout.trim(); + await writePatchAttempt(loaded.paths, { + ...patch, + git: { + ...patch.git, + commitSha, + branchName: branch, + prUrl, + }, + updatedAt: nowIso(), + }); + return { + patchAttempt: patch.patchAttemptId, + branch, + base, + commit: commitSha, + pr: prUrl, + next: prUrl.length > 0 ? prUrl : "inspect GitHub CLI output", + }; +} + export async function doctorCommand( context: AppContext, flags: Record = {}, @@ -1184,6 +1276,222 @@ function stringField(value: unknown, field: string): string | undefined { return typeof candidate === "string" ? candidate : undefined; } +function validatePrPatch(patch: PatchAttempt, force: boolean): void { + if (patch.filesChanged.length === 0) { + throw new ClawpatchError(`patch has no changed files: ${patch.patchAttemptId}`, 2, "invalid-input"); + } + if (!["applied", "validated"].includes(patch.status) && !force) { + throw new ClawpatchError( + `patch is not ready for PR: ${patch.patchAttemptId} (${patch.status})`, + 2, + "invalid-input", + ); + } + const failed = patch.testResults.filter((result) => result.exitCode !== 0); + if (failed.length > 0 && !force) { + throw new ClawpatchError( + `patch validation failed; use --force to open a PR anyway: ${failed[0]?.command ?? "unknown"}`, + 6, + "validation-failed", + ); + } +} + +function prBranchName( + patch: PatchAttempt, + explicit: string | undefined, + currentBranch: string | null, + base: string, +): string { + if (explicit !== undefined) { + return explicit; + } + if ( + patch.git.branchName !== null && + patch.git.branchName !== base && + patch.git.branchName !== "main" && + patch.git.branchName !== "master" + ) { + return patch.git.branchName; + } + if ( + currentBranch !== null && + currentBranch !== base && + currentBranch !== "main" && + currentBranch !== "master" + ) { + return currentBranch; + } + return `clawpatch/${patch.patchAttemptId}`; +} + +function prTitle( + explicit: string | undefined, + findings: FindingRecord[], + patch: PatchAttempt, +): string { + if (explicit !== undefined) { + return explicit; + } + const title = findings[0]?.title ?? patch.plan.split("\n")[0] ?? patch.patchAttemptId; + return `fix: ${title}`.slice(0, 120); +} + +function renderPatchPrBody(patch: PatchAttempt, findings: FindingRecord[]): string { + const lines = [ + "## Summary", + "", + `- patch attempt: \`${patch.patchAttemptId}\``, + `- status: \`${patch.status}\``, + `- files changed: ${patch.filesChanged.length}`, + "", + "## Findings", + "", + ]; + if (findings.length === 0) { + lines.push("- none linked"); + } else { + for (const finding of findings) { + lines.push(`- \`${finding.findingId}\`: ${finding.title} (${finding.severity})`); + } + } + lines.push("", "## Changed Files", ""); + for (const file of patch.filesChanged) { + lines.push(`- \`${file}\``); + } + lines.push("", "## Validation", ""); + const validation = patch.testResults.length > 0 ? patch.testResults : patch.commandsRun; + if (validation.length === 0) { + lines.push("- none recorded"); + } else { + for (const result of validation) { + lines.push(`- \`${result.command}\` => ${result.exitCode ?? "unknown"}`); + } + } + lines.push("", "## Plan", "", patch.plan, ""); + return `${lines.join("\n")}\n`; +} + +function gitRelativePatchFiles(gitRoot: string, projectRoot: string, files: string[]): string[] { + const projectPrefix = normalizePath(relative(gitRoot, projectRoot)); + const scopedPrefix = + projectPrefix.length > 0 && + projectPrefix !== "." && + !projectPrefix.startsWith("../") && + projectPrefix !== ".." + ? projectPrefix + : ""; + return files.map((file) => { + const relativeFile = normalizePath(file); + if ( + relativeFile.startsWith("../") || + relativeFile === ".." || + relativeFile.split("/").includes("..") || + resolve(relativeFile) === relativeFile || + relativeFile.length === 0 + ) { + throw new ClawpatchError(`patch file escapes git repository: ${file}`, 2, "invalid-input"); + } + return scopedPrefix.length === 0 ? relativeFile : `${scopedPrefix}/${relativeFile}`; + }); +} + +function plannedPrCommands( + patch: PatchAttempt, + branch: string, + base: string, + title: string, +): string[] { + const commands: string[] = []; + if (patch.git.commitSha === null) { + commands.push(`git switch -c ${branch}`); + commands.push(`git add -- ${patch.filesChanged.join(" ")}`); + commands.push(`git commit -m ${title}`); + } + commands.push(`git push -u origin ${branch}`); + commands.push(`gh pr create --base ${base} --head ${branch} --title ${title} --body-file -`); + return commands; +} + +async function assertPatchWorktree( + patch: PatchAttempt, + gitRoot: string, + stateDir: string, + gitFiles: string[], + force: boolean, +): Promise { + if (patch.git.commitSha !== null) { + return; + } + const status = await checkedRun( + "git status", + runCommandArgs("git", ["status", "--porcelain", "--untracked-files=all"], gitRoot, undefined, { + trimOutput: false, + }), + ); + const dirty = gitStatusPaths(status.stdout); + const statePrefix = normalizePath(relative(gitRoot, stateDir)); + const sourceDirty = dirty.filter((file) => !isStatePath(file, statePrefix)); + if (sourceDirty.length === 0) { + throw new ClawpatchError("no uncommitted patch changes to commit", 2, "invalid-input"); + } + const expected = new Set(gitFiles); + const extra = sourceDirty.filter((file) => !expected.has(file)); + if (extra.length > 0 && !force) { + throw new ClawpatchError( + `dirty worktree has files outside patch attempt: ${extra.join(", ")}`, + 3, + "dirty-worktree", + ); + } + const missing = gitFiles.filter((file) => !sourceDirty.includes(file)); + if (missing.length > 0 && !force) { + throw new ClawpatchError( + `patch files are not dirty in the worktree: ${missing.join(", ")}`, + 2, + "invalid-input", + ); + } +} + +function gitStatusPaths(output: string): string[] { + return output + .split("\n") + .map((line) => line.trimEnd()) + .filter((line) => line.length > 3) + .map((line) => line.slice(3)) + .map((path) => path.split(" -> ").at(-1) ?? path) + .map(normalizePath); +} + +function isStatePath(file: string, statePrefix: string): boolean { + return statePrefix.length > 0 && (file === statePrefix || file.startsWith(`${statePrefix}/`)); +} + +async function checkedRun(label: string, resultPromise: Promise): Promise { + const result = await resultPromise; + if (result.exitCode !== 0) { + throw new ClawpatchError( + `${label} failed: ${result.stderr || result.stdout}`, + label.startsWith("gh") ? 7 : 1, + label.startsWith("gh") ? "github-failure" : "git-failure", + ); + } + return result; +} + +function githubCli(): string { + return process.env["CLAWPATCH_GH"] ?? "gh"; +} + +function firstUrl(output: string): string | null { + return /https?:\/\/\S+/u.exec(output)?.[0] ?? null; +} + +function normalizePath(path: string): string { + return path.replace(/\\/gu, "/"); +} + function providerOptions(config: ReturnType) { return { model: config.provider.model, diff --git a/src/cli.ts b/src/cli.ts index 9f6b721..28e8b16 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -14,6 +14,7 @@ import { revalidateCommand, reviewCommand, nextCommand, + openPrCommand, showCommand, statusCommand, triageCommand, @@ -64,6 +65,8 @@ async function dispatch( return triageCommand(context, flags); case "fix": return fixCommand(context, flags); + case "open-pr": + return openPrCommand(context, flags); case "revalidate": return revalidateCommand(context, flags); case "doctor": @@ -180,6 +183,7 @@ const commandFlags = { next: new Set(["status", "project"]), triage: new Set(["finding", "status", "note"]), fix: new Set(["finding", "provider", "model", "reasoningEffort", "skipGitRepoCheck", "dryRun"]), + "open-pr": new Set(["patch", "base", "branch", "title", "draft", "dryRun", "force"]), revalidate: new Set([ "finding", "all", @@ -203,6 +207,7 @@ const requiredCommandFlags: Partial> show: ["finding"], triage: ["finding", "status"], fix: ["finding"], + "open-pr": ["patch"], }; const valueFlagNames = new Set([ @@ -228,6 +233,10 @@ const valueFlagNames = new Set([ "triage", "project", "note", + "patch", + "base", + "branch", + "title", ]); const booleanFlagNames = new Set([ @@ -242,6 +251,7 @@ const booleanFlagNames = new Set([ "skip-git-repo-check", "force", "all", + "draft", ]); const shortFlagNames = new Set(["-h", "-q", "-v", "-o"]); @@ -506,6 +516,24 @@ Flags: --skip-git-repo-check --dry-run --json +`); + return; + } + if (command === "open-pr") { + process.stdout.write(`clawpatch open-pr + +Usage: + clawpatch open-pr --patch [flags] + +Flags: + --patch + --base + --branch + --title + --draft + --dry-run + --force + --json `); return; } @@ -615,6 +643,7 @@ Commands: next triage fix + open-pr revalidate doctor clean-locks diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 584af9b..d29cbac 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -21,6 +21,7 @@ import { makeContext, mapCommand, nextCommand, + openPrCommand, reportCommand, revalidateCommand, reviewCommand, @@ -46,12 +47,13 @@ import { statePaths, writeFeature, writeFinding, + writePatchAttempt, } from "./state.js"; import { buildFixPrompt, buildReviewPrompt } from "./prompt.js"; import type { Provider } from "./provider.js"; import { fixtureRoot, testOptions, writeFixture } from "./test-helpers.js"; import { findingRecordSchema } from "./types.js"; -import type { FeatureRecord } from "./types.js"; +import type { FeatureRecord, PatchAttempt } from "./types.js"; async function sinceFixture(prefix: string): Promise<string> { const root = await fixtureRoot(prefix); @@ -245,6 +247,25 @@ describe("workflow", () => { dryRun: true, finding: "f", }); + expect( + parseArgs([ + "open-pr", + "--patch", + "pat_123", + "--base", + "main", + "--branch", + "clawpatch/pat_123", + "--draft", + "--dry-run", + ]).flags, + ).toMatchObject({ + patch: "pat_123", + base: "main", + branch: "clawpatch/pat_123", + draft: true, + dryRun: true, + }); }); it("parses review jobs and report filters", () => { @@ -3082,6 +3103,93 @@ describe("workflow", () => { expect(symlinkPrompt).not.toContain("do-not-read"); }); + it("previews a PR for an applied patch attempt", async () => { + const root = await fixtureRoot("clawpatch-open-pr-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "open-pr", bin: { open: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git add package.json src/index.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const previousProvider = process.env["CLAWPATCH_PROVIDER"]; + process.env["CLAWPATCH_PROVIDER"] = "mock"; + try { + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await mapCommand(context); + await reviewCommand(context, { limit: "1" }); + const finding = (await readFindings(paths))[0]; + expect(finding).toBeDefined(); + await writeFixture(root, "src/index.ts", "export const value = 'fixed';\n"); + const baseSha = (await runCommand("git rev-parse HEAD", root)).stdout.trim(); + const now = new Date().toISOString(); + const patch: PatchAttempt = { + schemaVersion: 1, + patchAttemptId: "pat_open_pr", + findingIds: [finding!.findingId], + featureIds: [finding!.featureId], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha, + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }; + await writePatchAttempt(paths, patch); + + const preview = await openPrCommand(context, { + patch: patch.patchAttemptId, + base: "main", + branch: "clawpatch/pat_open_pr", + dryRun: true, + }); + const stored = (await readPatchAttempts(paths)).find( + (candidate) => candidate.patchAttemptId === patch.patchAttemptId, + ); + + expect(preview).toMatchObject({ + dryRun: true, + patchAttempt: patch.patchAttemptId, + branch: "clawpatch/pat_open_pr", + base: "main", + }); + expect(preview).toMatchObject({ + body: expect.stringContaining("pat_open_pr"), + commands: expect.arrayContaining([ + expect.stringContaining("gh pr create --base main --head clawpatch/pat_open_pr"), + ]), + }); + expect(stored?.git.prUrl).toBeNull(); + } finally { + if (previousProvider === undefined) { + delete process.env["CLAWPATCH_PROVIDER"]; + } else { + process.env["CLAWPATCH_PROVIDER"] = previousProvider; + } + } + }); + it("persists failed patch attempts when provider fix throws", async () => { const root = await fixtureRoot("clawpatch-fix-fail-"); await runCommand( From 43df8aab17a63867e8917b0cbce31c9808aa54af Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 00:25:15 +0800 Subject: [PATCH 04/25] feat(review): record prompt provenance --- CHANGELOG.md | 1 + docs/code-review.md | 3 + src/app.ts | 20 ++++-- src/prompt.test.ts | 96 +++++++++++++++++++++++++++ src/prompt.ts | 150 +++++++++++++++++++++++++++++++++++++++++-- src/workflow.test.ts | 4 ++ 6 files changed, 262 insertions(+), 12 deletions(-) create mode 100644 src/prompt.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e9c1d88..623c763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Added `clawpatch ci` to initialize, map, review, write a report, and append a GitHub Actions step summary in one CI-friendly command. - Added `clawpatch open-pr --patch <id>` to turn an applied patch attempt into an explicit GitHub pull request. +- Added review prompt provenance and budget accounting for included files, omitted files, prompt bytes, and approximate tokens. - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. ## 0.3.0 - 2026-05-18 diff --git a/docs/code-review.md b/docs/code-review.md index a3046b5..e1ce7f0 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -23,12 +23,15 @@ Current behavior: - reviews with a bounded worker pool; default `--jobs` is `10` - emits progress to stderr unless `--quiet` is set - builds bounded prompt context from owned files, context files, and tests +- includes a prompt context manifest with included files, omitted files, byte + counts, and truncation status - calls the configured provider - requires strict JSON output - rejects findings whose evidence cites files outside the prompt context, stale line ranges, or quotes that do not match current file contents - writes findings under `.clawpatch/findings/` - appends analysis history to the feature record +- records prompt byte and approximate token counts in feature analysis history - releases the feature lock ## Flags diff --git a/src/app.ts b/src/app.ts index 60a412a..e01d17e 100644 --- a/src/app.ts +++ b/src/app.ts @@ -23,8 +23,8 @@ import { mapWithSource } from "./agent-mapper.js"; import { mapFeatures } from "./mapper.js"; import { emitProgress } from "./progress.js"; import { providerByName } from "./provider.js"; -import { buildFixPrompt, buildReviewPrompt, buildRevalidatePrompt } from "./prompt.js"; -import type { ReviewMode } from "./prompt.js"; +import { buildFixPrompt, buildReviewPromptBundle, buildRevalidatePrompt } from "./prompt.js"; +import type { ReviewMode, ReviewPromptManifest } from "./prompt.js"; import { evidenceLabel, findingSummaries, @@ -665,7 +665,7 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId }, ); locked = lockedFeature; - const prompt = await buildReviewPrompt( + const reviewPrompt = await buildReviewPromptBundle( loaded.root, loaded.project, lockedFeature, @@ -677,7 +677,7 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId loaded.root, lockedFeature, config, - await provider.review(loaded.root, prompt, providerOptions(config)), + await provider.review(loaded.root, reviewPrompt.prompt, providerOptions(config)), ); const modeFindings = reviewFindingsForMode(output.findings, mode); const records = modeFindings @@ -702,7 +702,7 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId { runId: currentRunId, kind: "review", - summary: `${records.length} finding(s)`, + summary: reviewAnalysisSummary(records.length, reviewPrompt.manifest), provider: provider.name, model: config.provider.model, reasoningEffort: config.provider.reasoningEffort, @@ -1276,6 +1276,16 @@ function stringField(value: unknown, field: string): string | undefined { return typeof candidate === "string" ? candidate : undefined; } +function reviewAnalysisSummary(findings: number, manifest: ReviewPromptManifest): string { + return [ + `${findings} finding(s)`, + `prompt=${manifest.promptBytes} bytes`, + `approxTokens=${manifest.approximateTokens}`, + `includedFiles=${manifest.includedFiles.length}`, + `omittedFiles=${manifest.omittedFiles.length}`, + ].join("; "); +} + function validatePrPatch(patch: PatchAttempt, force: boolean): void { if (patch.filesChanged.length === 0) { throw new ClawpatchError(`patch has no changed files: ${patch.patchAttemptId}`, 2, "invalid-input"); diff --git a/src/prompt.test.ts b/src/prompt.test.ts new file mode 100644 index 0000000..06013dc --- /dev/null +++ b/src/prompt.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from "vitest"; +import { buildReviewPromptBundle } from "./prompt.js"; +import { defaultConfig } from "./config.js"; +import { fixtureRoot, writeFixture } from "./test-helpers.js"; +import type { FeatureRecord, ProjectRecord } from "./types.js"; + +describe("review prompt provenance", () => { + it("records included, omitted, and truncated review prompt context", async () => { + const root = await fixtureRoot("clawpatch-prompt-provenance-"); + await writeFixture(root, "src/index.ts", "export const value = 1;\n"); + await writeFixture(root, "src/extra.ts", "export const extra = 1;\n"); + await writeFixture(root, "tests/index.test.ts", "expect(1).toBe(1);\n"); + await writeFixture(root, "docs/large.md", `${"x".repeat(24_100)}\n`); + const bundle = await buildReviewPromptBundle(root, project(root), feature(), { + ...defaultConfig(), + review: { + ...defaultConfig().review, + maxOwnedFiles: 1, + maxContextFiles: 2, + }, + }); + + expect(bundle.prompt).toContain("Prompt context:"); + expect(bundle.prompt).toContain("--- src/index.ts"); + expect(bundle.prompt).toContain("--- tests/index.test.ts"); + expect(bundle.prompt).not.toContain("--- src/extra.ts"); + expect(bundle.manifest.includedFiles).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: "src/index.ts", role: "owned", truncated: false }), + expect.objectContaining({ path: "docs/large.md", role: "context", truncated: true }), + ]), + ); + expect(bundle.manifest.omittedFiles).toEqual([ + { path: "src/extra.ts", role: "owned", reason: "maxOwnedFiles" }, + { path: "docs/omitted.md", role: "context", reason: "maxContextFiles" }, + ]); + expect(bundle.manifest.promptBytes).toBeGreaterThan(0); + expect(bundle.manifest.approximateTokens).toBeGreaterThan(0); + }); +}); + +function project(root: string): ProjectRecord { + return { + schemaVersion: 1, + projectId: "proj_prompt", + name: "prompt", + rootPath: root, + git: { + remoteUrl: null, + defaultBranch: null, + currentBranch: null, + headSha: null, + }, + detected: { + languages: ["typescript"], + frameworks: [], + packageManagers: ["npm"], + commands: defaultConfig().commands, + }, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; +} + +function feature(): FeatureRecord { + const now = new Date().toISOString(); + return { + schemaVersion: 1, + featureId: "feat_prompt", + title: "Prompt feature", + summary: "Prompt provenance feature", + kind: "library", + source: "test", + confidence: "high", + entrypoints: [], + ownedFiles: [ + { path: "src/index.ts", reason: "primary" }, + { path: "src/extra.ts", reason: "overflow" }, + ], + contextFiles: [ + { path: "tests/index.test.ts", reason: "test" }, + { path: "docs/large.md", reason: "large doc" }, + { path: "docs/omitted.md", reason: "overflow" }, + ], + tests: [], + tags: [], + trustBoundaries: [], + status: "pending", + lock: null, + findingIds: [], + patchAttemptIds: [], + analysisHistory: [], + createdAt: now, + updatedAt: now, + }; +} diff --git a/src/prompt.ts b/src/prompt.ts index 6831791..dbf15f2 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -4,6 +4,32 @@ import { ClawpatchConfig, FeatureRecord, FindingRecord, ProjectRecord } from "./ export type ReviewMode = "default" | "deslopify"; +export type ReviewPromptFileRole = "owned" | "context"; + +export type ReviewPromptFileManifest = { + path: string; + role: ReviewPromptFileRole; + bytes: number; + includedBytes: number; + truncated: boolean; + readable: boolean; + skippedReason: string | null; +}; + +export type ReviewPromptManifest = { + maxOwnedFiles: number; + maxContextFiles: number; + includedFiles: ReviewPromptFileManifest[]; + omittedFiles: Array<{ path: string; role: ReviewPromptFileRole; reason: string }>; + promptBytes: number; + approximateTokens: number; +}; + +export type ReviewPromptBundle = { + prompt: string; + manifest: ReviewPromptManifest; +}; + export function buildAgentMapPrompt(project: ProjectRecord, inventory: unknown): string { return `You are mapping a repository into semantic clawpatch review slices. @@ -62,11 +88,42 @@ export async function buildReviewPrompt( mode: ReviewMode = "default", customPrompt: string | null = null, ): Promise<string> { + return (await buildReviewPromptBundle(root, project, feature, config, mode, customPrompt)).prompt; +} + +export async function buildReviewPromptBundle( + root: string, + project: ProjectRecord, + feature: FeatureRecord, + config: ClawpatchConfig, + mode: ReviewMode = "default", + customPrompt: string | null = null, +): Promise<ReviewPromptBundle> { const owned = feature.ownedFiles.slice(0, config.review.maxOwnedFiles); const context = feature.contextFiles.slice(0, config.review.maxContextFiles); + const omittedFiles = [ + ...feature.ownedFiles.slice(config.review.maxOwnedFiles).map((ref) => ({ + path: ref.path, + role: "owned" as const, + reason: "maxOwnedFiles", + })), + ...feature.contextFiles.slice(config.review.maxContextFiles).map((ref) => ({ + path: ref.path, + role: "context" as const, + reason: "maxContextFiles", + })), + ]; const fileBlocks: string[] = []; - for (const ref of [...owned, ...context]) { - fileBlocks.push(await fileBlock(root, ref.path)); + const includedFiles: ReviewPromptFileManifest[] = []; + for (const ref of owned) { + const file = await fileBlockWithManifest(root, ref.path, "owned"); + fileBlocks.push(file.block); + includedFiles.push(file.manifest); + } + for (const ref of context) { + const file = await fileBlockWithManifest(root, ref.path, "context"); + fileBlocks.push(file.block); + includedFiles.push(file.manifest); } const customBlock = customPrompt !== null && customPrompt.trim() !== "" @@ -76,7 +133,19 @@ ${customPrompt.trim()} ` : ""; - return `You are reviewing one semantic feature for clawpatch. + const promptContext = { + maxOwnedFiles: config.review.maxOwnedFiles, + maxContextFiles: config.review.maxContextFiles, + includedFiles: includedFiles.map(({ path, role, bytes, includedBytes, truncated }) => ({ + path, + role, + bytes, + includedBytes, + truncated, + })), + omittedFiles, + }; + const prompt = `You are reviewing one semantic feature for clawpatch. Return strict JSON only. No markdown fences. @@ -110,6 +179,9 @@ with multiple evidence refs instead of separate one-off findings. Avoid speculative low-evidence findings. Evidence must point at included files. +Prompt context: +${JSON.stringify(promptContext, null, 2)} + JSON shape: { "findings": [ @@ -132,6 +204,17 @@ JSON shape: Files: ${fileBlocks.join("\n\n")}`; + const promptBytes = Buffer.byteLength(prompt, "utf8"); + return { + prompt, + manifest: { + ...promptContext, + includedFiles, + omittedFiles, + promptBytes, + approximateTokens: Math.ceil(prompt.length / 4), + }, + }; } function reviewModeInstructions(mode: ReviewMode): string { @@ -245,19 +328,72 @@ function fixPromptPaths( } async function fileBlock(root: string, path: string): Promise<string> { + return (await fileBlockWithManifest(root, path, "context")).block; +} + +async function fileBlockWithManifest( + root: string, + path: string, + role: ReviewPromptFileRole, +): Promise<{ block: string; manifest: ReviewPromptFileManifest }> { const full = resolve(root, path); if (!isInside(root, full)) { - return `--- ${path}\n[skipped: path escapes repository root]`; + return skippedFileBlock(path, role, "path escapes repository root"); } const realRoot = await realpath(root).catch(() => root); const realFull = await realpath(full).catch(() => full); if (!isInside(realRoot, realFull)) { - return `--- ${path}\n[skipped: path escapes repository root]`; + return skippedFileBlock(path, role, "path escapes repository root"); } - const contents = await readFile(full, "utf8").catch(() => "[unreadable]"); + const contents = await readFile(full, "utf8").catch(() => null); + if (contents === null) { + return { + block: `--- ${path}\n[unreadable]`, + manifest: { + path, + role, + bytes: 0, + includedBytes: 0, + truncated: false, + readable: false, + skippedReason: "unreadable", + }, + }; + } + const bytes = Buffer.byteLength(contents, "utf8"); const trimmed = contents.length > 24_000 ? `${contents.slice(0, 24_000)}\n...[truncated]` : contents; - return `--- ${path}\n${trimmed}`; + return { + block: `--- ${path}\n${trimmed}`, + manifest: { + path, + role, + bytes, + includedBytes: Buffer.byteLength(trimmed, "utf8"), + truncated: trimmed.length !== contents.length, + readable: true, + skippedReason: null, + }, + }; +} + +function skippedFileBlock( + path: string, + role: ReviewPromptFileRole, + reason: string, +): { block: string; manifest: ReviewPromptFileManifest } { + return { + block: `--- ${path}\n[skipped: ${reason}]`, + manifest: { + path, + role, + bytes: 0, + includedBytes: 0, + truncated: false, + readable: false, + skippedReason: reason, + }, + }; } function isInside(root: string, candidate: string): boolean { diff --git a/src/workflow.test.ts b/src/workflow.test.ts index d29cbac..d087da4 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -396,6 +396,9 @@ describe("workflow", () => { const reviewed = await reviewCommand(context, { limit: "1" }); const paths = statePaths(join(root, ".clawpatch")); const finding = (await readFindings(paths))[0]; + const reviewedFeature = (await readFeatures(paths)).find( + (feature) => feature.featureId === finding?.featureId, + ); expect(finding).toBeDefined(); await writeFinding(paths, { ...finding!, @@ -432,6 +435,7 @@ describe("workflow", () => { }, ], }); + expect(reviewedFeature?.analysisHistory.at(-1)?.summary).toContain("prompt="); delete process.env["CLAWPATCH_PROVIDER"]; }); From 5aea8abb049046276385f4b6e8caae21861bf895 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 00:32:17 +0800 Subject: [PATCH 05/25] fix(review): harden review retry validation --- src/app.ts | 59 ++++++++++++++++++---- src/review-validation.test.ts | 18 +++++++ src/review-validation.ts | 11 +++- src/workflow.test.ts | 94 +++++++++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 13 deletions(-) diff --git a/src/app.ts b/src/app.ts index e01d17e..fe79eec 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1074,6 +1074,11 @@ export async function openPrCommand( runCommandArgs("git", ["rev-parse", "HEAD"], git.root), ); commitSha = commit.stdout.trim(); + await writePatchPrGitState(loaded.paths, patch, { + commitSha, + branchName: branch, + prUrl: patch.git.prUrl, + }); } await checkedRun("git push", runCommandArgs("git", ["push", "-u", "origin", branch], git.root)); const ghArgs = [ @@ -1093,16 +1098,7 @@ export async function openPrCommand( } const gh = await checkedRun("gh pr create", runCommandArgs(githubCli(), ghArgs, git.root, body)); const prUrl = firstUrl(gh.stdout) ?? gh.stdout.trim(); - await writePatchAttempt(loaded.paths, { - ...patch, - git: { - ...patch.git, - commitSha, - branchName: branch, - prUrl, - }, - updatedAt: nowIso(), - }); + await writePatchPrGitState(loaded.paths, patch, { commitSha, branchName: branch, prUrl }); return { patchAttempt: patch.patchAttemptId, branch, @@ -1113,6 +1109,23 @@ export async function openPrCommand( }; } +async function writePatchPrGitState( + paths: ReturnType<typeof statePaths>, + patch: PatchAttempt, + git: { commitSha: string; branchName: string; prUrl: string | null }, +): Promise<void> { + await writePatchAttempt(paths, { + ...patch, + git: { + ...patch.git, + commitSha: git.commitSha, + branchName: git.branchName, + prUrl: git.prUrl, + }, + updatedAt: nowIso(), + }); +} + export async function doctorCommand( context: AppContext, flags: Record<string, string | boolean> = {}, @@ -1440,7 +1453,7 @@ async function assertPatchWorktree( }), ); const dirty = gitStatusPaths(status.stdout); - const statePrefix = normalizePath(relative(gitRoot, stateDir)); + const statePrefix = gitRelativePathPrefix(gitRoot, stateDir); const sourceDirty = dirty.filter((file) => !isStatePath(file, statePrefix)); if (sourceDirty.length === 0) { throw new ClawpatchError("no uncommitted patch changes to commit", 2, "invalid-input"); @@ -1478,6 +1491,26 @@ function isStatePath(file: string, statePrefix: string): boolean { return statePrefix.length > 0 && (file === statePrefix || file.startsWith(`${statePrefix}/`)); } +function gitRelativePathPrefix(gitRoot: string, path: string): string { + const direct = normalizePath(relative(gitRoot, path)); + if (isUsableRelativePrefix(direct)) { + return direct; + } + const normalizedGitRoot = normalizeDarwinPrivateVar(gitRoot); + const normalizedPath = normalizeDarwinPrivateVar(path); + if (normalizedPath === normalizedGitRoot) { + return ""; + } + if (normalizedPath.startsWith(`${normalizedGitRoot}/`)) { + return normalizedPath.slice(normalizedGitRoot.length + 1); + } + return direct; +} + +function isUsableRelativePrefix(path: string): boolean { + return path.length > 0 && path !== "." && path !== ".." && !path.startsWith("../"); +} + async function checkedRun(label: string, resultPromise: Promise<CommandResult>): Promise<CommandResult> { const result = await resultPromise; if (result.exitCode !== 0) { @@ -1502,6 +1535,10 @@ function normalizePath(path: string): string { return path.replace(/\\/gu, "/"); } +function normalizeDarwinPrivateVar(path: string): string { + return normalizePath(path).replace(/^\/private\/var\//u, "/var/"); +} + function providerOptions(config: ReturnType<typeof applyProviderFlags>) { return { model: config.provider.model, diff --git a/src/review-validation.test.ts b/src/review-validation.test.ts index 745762b..848b3ea 100644 --- a/src/review-validation.test.ts +++ b/src/review-validation.test.ts @@ -46,6 +46,24 @@ describe("validateReviewOutput", () => { ), ).rejects.toMatchObject({ code: "malformed-output" }); }); + + it("rejects quotes that only match outside the cited line range", async () => { + const root = await fixtureRoot("clawpatch-review-validation-line-quote-"); + await writeFixture( + root, + "src/index.ts", + "const first = 'TODO_BUG';\nconst second = 'safe';\n", + ); + + await expect( + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + output("src/index.ts", { startLine: 2, endLine: 2, quote: "TODO_BUG" }), + ), + ).rejects.toMatchObject({ code: "malformed-output" }); + }); }); function feature(path: string): FeatureRecord { diff --git a/src/review-validation.ts b/src/review-validation.ts index e62f3ec..46bd7df 100644 --- a/src/review-validation.ts +++ b/src/review-validation.ts @@ -104,9 +104,16 @@ function assertQuote( if (quote === null || quote.trim().length === 0) { return; } + const target = + evidence.startLine !== null && evidence.endLine !== null + ? contents + .split("\n") + .slice(evidence.startLine - 1, evidence.endLine) + .join("\n") + : contents; if ( - !contents.includes(quote) && - !compactWhitespace(contents).includes(compactWhitespace(quote)) + !target.includes(quote) && + !compactWhitespace(target).includes(compactWhitespace(quote)) ) { throwMalformed(`evidence quote does not match file contents: ${evidence.path}`); } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index d087da4..9794b31 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3194,6 +3194,100 @@ describe("workflow", () => { } }); + it("persists the patch commit before failing external PR creation", async () => { + const root = await fixtureRoot("clawpatch-open-pr-retry-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "open-pr-retry", bin: { open: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git add package.json src/index.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const origin = await fixtureRoot("clawpatch-open-pr-retry-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await writeFixture(root, "src/index.ts", "export const value = 'fixed';\n"); + const now = new Date().toISOString(); + const patch: PatchAttempt = { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_retry", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }; + await writePatchAttempt(paths, patch); + const ghScripts = await fixtureRoot("clawpatch-open-pr-gh-"); + const failingGh = join(ghScripts, "fail-gh.sh"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture(ghScripts, "fail-gh.sh", "#!/bin/sh\nexit 42\n"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/999\n", + ); + await chmod(failingGh, 0o755); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = failingGh; + await expect( + openPrCommand(context, { + patch: patch.patchAttemptId, + base: "main", + branch: "clawpatch/pat_open_pr_retry", + }), + ).rejects.toMatchObject({ code: "github-failure" }); + const afterFailure = (await readPatchAttempts(paths)).find( + (candidate) => candidate.patchAttemptId === patch.patchAttemptId, + ); + expect(afterFailure?.git.commitSha).toMatch(/^[a-f0-9]{40}$/u); + expect(afterFailure?.git.branchName).toBe("clawpatch/pat_open_pr_retry"); + + process.env["CLAWPATCH_GH"] = successGh; + await expect( + openPrCommand(context, { + patch: patch.patchAttemptId, + base: "main", + }), + ).resolves.toMatchObject({ + pr: "https://github.com/openclaw/clawpatch/pull/999", + }); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("persists failed patch attempts when provider fix throws", async () => { const root = await fixtureRoot("clawpatch-fix-fail-"); await runCommand( From 92703d1498f771aba989c2318903cdf1891a2bdc Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 09:29:05 +0800 Subject: [PATCH 06/25] fix(pr): isolate open-pr git state --- src/app.ts | 34 +++++++++++++------ src/workflow.test.ts | 80 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 11 deletions(-) diff --git a/src/app.ts b/src/app.ts index fe79eec..9356082 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1067,7 +1067,7 @@ export async function openPrCommand( await checkedRun("git add", runCommandArgs("git", ["add", "--", ...gitFiles], git.root)); await checkedRun( "git commit", - runCommandArgs("git", ["commit", "-m", title], git.root), + runCommandArgs("git", ["commit", "-m", title, "--", ...gitFiles], git.root), ); const commit = await checkedRun( "git rev-parse", @@ -1448,9 +1448,15 @@ async function assertPatchWorktree( } const status = await checkedRun( "git status", - runCommandArgs("git", ["status", "--porcelain", "--untracked-files=all"], gitRoot, undefined, { - trimOutput: false, - }), + runCommandArgs( + "git", + ["status", "--porcelain=v1", "-z", "--untracked-files=all"], + gitRoot, + undefined, + { + trimOutput: false, + }, + ), ); const dirty = gitStatusPaths(status.stdout); const statePrefix = gitRelativePathPrefix(gitRoot, stateDir); @@ -1478,13 +1484,19 @@ async function assertPatchWorktree( } function gitStatusPaths(output: string): string[] { - return output - .split("\n") - .map((line) => line.trimEnd()) - .filter((line) => line.length > 3) - .map((line) => line.slice(3)) - .map((path) => path.split(" -> ").at(-1) ?? path) - .map(normalizePath); + const fields = output.split("\0").filter((field) => field.length > 0); + const paths: string[] = []; + for (let index = 0; index < fields.length; index += 1) { + const field = fields[index] ?? ""; + if (field.length < 4) { + continue; + } + paths.push(field.slice(3)); + if (/[RC]/u.test(field.slice(0, 2))) { + index += 1; + } + } + return paths.map(normalizePath); } function isStatePath(file: string, statePrefix: string): boolean { diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 9794b31..09cd8d5 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3288,6 +3288,86 @@ describe("workflow", () => { } }); + it("opens PRs for quoted paths without committing pre-staged state", async () => { + const root = await fixtureRoot("clawpatch-open-pr-pathspec-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "open-pr-pathspec", bin: { open: "docs/foo bar.md" } }), + ); + await writeFixture(root, "docs/foo bar.md", "TODO_BUG\n"); + await initGit(root); + await checkCommand(root, "git add package.json docs"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const origin = await fixtureRoot("clawpatch-open-pr-pathspec-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await checkCommand(root, "git add .clawpatch/config.json"); + await writeFixture(root, "docs/foo bar.md", "fixed\n"); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_pathspec", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["docs/foo bar.md"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-pathspec-gh-"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1000\n", + ); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = successGh; + const opened = (await openPrCommand(context, { + patch: "pat_open_pr_pathspec", + base: "main", + branch: "clawpatch/pat_open_pr_pathspec", + })) as { commit: string; pr: string }; + const committed = await runCommand(`git show --name-only --format= ${opened.commit}`, root); + const cached = await runCommand("git diff --cached --name-only", root); + + expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1000"); + expect(committed.stdout.trim().split("\n")).toEqual(["docs/foo bar.md"]); + expect(cached.stdout.trim().split("\n")).toContain(".clawpatch/config.json"); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("persists failed patch attempts when provider fix throws", async () => { const root = await fixtureRoot("clawpatch-fix-fail-"); await runCommand( From ffe7204f6fea576f04b56f18558df936c9a14ce2 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 09:34:51 +0800 Subject: [PATCH 07/25] fix(review): cap findings before validation --- src/review-validation.test.ts | 31 +++++++++++++++++++++++++++++++ src/review-validation.ts | 18 ++++++++++++------ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/review-validation.test.ts b/src/review-validation.test.ts index 848b3ea..4ff4e36 100644 --- a/src/review-validation.test.ts +++ b/src/review-validation.test.ts @@ -64,6 +64,37 @@ describe("validateReviewOutput", () => { ), ).rejects.toMatchObject({ code: "malformed-output" }); }); + + it("validates only findings kept by the feature finding cap", async () => { + const root = await fixtureRoot("clawpatch-review-validation-cap-"); + await writeFixture(root, "src/index.ts", "const value = 'TODO_BUG';\n"); + await writeFixture(root, "src/other.ts", "const value = 'hidden';\n"); + const config = defaultConfig(); + config.review.maxFindingsPerFeature = 1; + const providerOutput = output("src/index.ts"); + const keptFinding = providerOutput.findings[0]; + if (keptFinding === undefined) { + throw new Error("fixture output did not include a finding"); + } + providerOutput.findings.push({ + ...keptFinding, + title: "Discarded", + evidence: [ + { + path: "src/other.ts", + startLine: 1, + endLine: 1, + symbol: null, + quote: "hidden", + }, + ], + }); + providerOutput.inspected.files.push("src/other.ts"); + + await expect( + validateReviewOutput(root, feature("src/index.ts"), config, providerOutput), + ).resolves.toMatchObject({ findings: [{ title: "Bug" }] }); + }); }); function feature(path: string): FeatureRecord { diff --git a/src/review-validation.ts b/src/review-validation.ts index 46bd7df..957fa04 100644 --- a/src/review-validation.ts +++ b/src/review-validation.ts @@ -12,9 +12,10 @@ export async function validateReviewOutput( const included = includedReviewPaths(feature, config); const cache = new Map<string, Promise<string>>(); for (const file of output.inspected.files) { - assertIncludedPath(file, included, "inspected file"); + assertSafePath(file, "inspected file"); } - for (const finding of output.findings) { + const findings = output.findings.slice(0, config.review.maxFindingsPerFeature); + for (const finding of findings) { if (finding.evidence.length === 0) { throwMalformed(`finding "${finding.title}" has no evidence`); } @@ -25,7 +26,7 @@ export async function validateReviewOutput( assertQuote(contents, evidence); } } - return output; + return { ...output, findings }; } function includedReviewPaths(feature: FeatureRecord, config: ClawpatchConfig): Set<string> { @@ -39,14 +40,19 @@ function includedReviewPaths(feature: FeatureRecord, config: ClawpatchConfig): S function assertIncludedPath(path: string, included: ReadonlySet<string>, label: string): void { const normalized = normalizePath(path); - if (normalized.startsWith("../") || isAbsolute(normalized)) { - throwMalformed(`${label} escapes repository root: ${path}`); - } + assertSafePath(path, label); if (!included.has(normalized)) { throwMalformed(`${label} was not included in review context: ${path}`); } } +function assertSafePath(path: string, label: string): void { + const normalized = normalizePath(path); + if (normalized.startsWith("../") || isAbsolute(normalized)) { + throwMalformed(`${label} escapes repository root: ${path}`); + } +} + async function fileContents( root: string, path: string, From 8dfe99a765d8c06167f9541ecf27911a53f9205e Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 09:49:48 +0800 Subject: [PATCH 08/25] test(exec): stabilize timeout descendant check --- src/exec.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/exec.test.ts b/src/exec.test.ts index 539981c..f104a9e 100644 --- a/src/exec.test.ts +++ b/src/exec.test.ts @@ -84,7 +84,7 @@ describe("runCommandArgs", () => { "import { writeFileSync } from 'node:fs';", "process.on('SIGTERM', () => {});", "process.send?.('ready');", - `setTimeout(() => writeFileSync(${JSON.stringify(marker)}, 'alive'), 1000);`, + `setTimeout(() => writeFileSync(${JSON.stringify(marker)}, 'alive'), 2500);`, "setInterval(() => {}, 1000);", ].join("\n"), "utf8", @@ -103,9 +103,9 @@ describe("runCommandArgs", () => { ); const result = await runCommandArgs(process.execPath, [parentScript], dir, undefined, { - timeoutMs: 300, + timeoutMs: 1000, }); - await new Promise((resolve) => setTimeout(resolve, 500)); + await new Promise((resolve) => setTimeout(resolve, 1200)); expect(result.exitCode).toBe(124); await expect(access(ready)).resolves.toBeUndefined(); From 1745e208bed9e0bc3644a3a228249f2911445161 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 09:56:53 +0800 Subject: [PATCH 09/25] fix(review): validate capped prompt evidence --- src/app.ts | 18 +++++--- src/prompt.ts | 6 ++- src/provider.ts | 51 +++++++++++++++++++++ src/review-validation.test.ts | 83 +++++++++++++++++++++++------------ src/review-validation.ts | 24 +++++++--- src/workflow.test.ts | 42 ++++++++++++++++++ 6 files changed, 182 insertions(+), 42 deletions(-) diff --git a/src/app.ts b/src/app.ts index 9356082..35f196e 100644 --- a/src/app.ts +++ b/src/app.ts @@ -673,16 +673,24 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId mode, customPrompt, ); + const providerOutput = await provider.review(loaded.root, reviewPrompt.prompt, providerOptions(config)); + const reviewOutput = { + ...providerOutput, + findings: reviewFindingsForMode(providerOutput.findings, mode).slice( + 0, + config.review.maxFindingsPerFeature, + ), + }; const output = await validateReviewOutput( loaded.root, lockedFeature, config, - await provider.review(loaded.root, reviewPrompt.prompt, providerOptions(config)), + reviewPrompt.manifest, + reviewOutput, + ); + const records = output.findings.map((finding) => + findingFromOutput(finding, lockedFeature.featureId, currentRunId), ); - const modeFindings = reviewFindingsForMode(output.findings, mode); - const records = modeFindings - .slice(0, config.review.maxFindingsPerFeature) - .map((finding) => findingFromOutput(finding, lockedFeature.featureId, currentRunId)); const findingIds: string[] = []; for (const finding of records) { const existingFinding = await readFinding(loaded.paths, finding.findingId); diff --git a/src/prompt.ts b/src/prompt.ts index dbf15f2..d555a1d 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -4,6 +4,8 @@ import { ClawpatchConfig, FeatureRecord, FindingRecord, ProjectRecord } from "./ export type ReviewMode = "default" | "deslopify"; +export const REVIEW_PROMPT_FILE_CHAR_LIMIT = 24_000; + export type ReviewPromptFileRole = "owned" | "context"; export type ReviewPromptFileManifest = { @@ -362,7 +364,9 @@ async function fileBlockWithManifest( } const bytes = Buffer.byteLength(contents, "utf8"); const trimmed = - contents.length > 24_000 ? `${contents.slice(0, 24_000)}\n...[truncated]` : contents; + contents.length > REVIEW_PROMPT_FILE_CHAR_LIMIT + ? `${contents.slice(0, REVIEW_PROMPT_FILE_CHAR_LIMIT)}\n...[truncated]` + : contents; return { block: `--- ${path}\n${trimmed}`, manifest: { diff --git a/src/provider.ts b/src/provider.ts index e784252..da6a764 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -364,6 +364,57 @@ const mockProvider: Provider = { const evidencePath = prompt.includes("BAD_EVIDENCE") ? "src/not-included.ts" : (firstPromptFileWith(prompt, "TODO_BUG") ?? "src/index.ts"); + if (prompt.includes("DESLOPIFY_LATE")) { + return { + findings: [ + { + title: "General bug first", + category: "bug", + severity: "medium", + confidence: "high", + evidence: [ + { + path: evidencePath, + startLine: null, + endLine: null, + symbol: null, + quote: "TODO_BUG", + }, + ], + reasoning: "Mock provider found an explicit bug marker.", + reproduction: null, + recommendation: "Replace marker with real handling.", + whyTestsDoNotAlreadyCoverThis: + "Mock fixtures do not encode this marker as intended behavior.", + suggestedRegressionTest: "Add a focused test that fails when TODO_BUG is present.", + minimumFixScope: "Replace the marker in the owning feature file.", + }, + { + title: "Late simplification finding", + category: "maintainability", + severity: "low", + confidence: "high", + evidence: [ + { + path: evidencePath, + startLine: null, + endLine: null, + symbol: null, + quote: "DESLOPIFY_LATE", + }, + ], + reasoning: "Mock provider returned a simplification finding after a general finding.", + reproduction: null, + recommendation: "Keep the deslopify finding after mode filtering.", + whyTestsDoNotAlreadyCoverThis: + "Mock fixtures need to prove filtering occurs before the finding cap.", + suggestedRegressionTest: null, + minimumFixScope: "Filter before capping.", + }, + ], + inspected: { files: [evidencePath], symbols: [], notes: ["mock mixed findings"] }, + }; + } return { findings: [ { diff --git a/src/review-validation.test.ts b/src/review-validation.test.ts index 4ff4e36..05418cc 100644 --- a/src/review-validation.test.ts +++ b/src/review-validation.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { defaultConfig } from "./config.js"; +import type { ReviewPromptManifest } from "./prompt.js"; import { validateReviewOutput } from "./review-validation.js"; import { fixtureRoot, writeFixture } from "./test-helpers.js"; import type { FeatureRecord, ReviewOutput } from "./types.js"; @@ -10,7 +11,13 @@ describe("validateReviewOutput", () => { await writeFixture(root, "src/index.ts", "const value = 'TODO_BUG';\n"); await expect( - validateReviewOutput(root, feature("src/index.ts"), defaultConfig(), output("src/index.ts")), + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + manifest("src/index.ts"), + output("src/index.ts"), + ), ).resolves.toMatchObject({ findings: [{ title: "Bug" }] }); }); @@ -20,7 +27,13 @@ describe("validateReviewOutput", () => { await writeFixture(root, "src/other.ts", "const value = 'TODO_BUG';\n"); await expect( - validateReviewOutput(root, feature("src/index.ts"), defaultConfig(), output("src/other.ts")), + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + manifest("src/index.ts"), + output("src/other.ts"), + ), ).rejects.toMatchObject({ code: "malformed-output" }); }); @@ -33,6 +46,7 @@ describe("validateReviewOutput", () => { root, feature("src/index.ts"), defaultConfig(), + manifest("src/index.ts"), output("src/index.ts", { startLine: 9, endLine: 9, quote: "real" }), ), ).rejects.toMatchObject({ code: "malformed-output" }); @@ -42,6 +56,7 @@ describe("validateReviewOutput", () => { root, feature("src/index.ts"), defaultConfig(), + manifest("src/index.ts"), output("src/index.ts", { startLine: 1, endLine: 1, quote: "missing" }), ), ).rejects.toMatchObject({ code: "malformed-output" }); @@ -60,40 +75,25 @@ describe("validateReviewOutput", () => { root, feature("src/index.ts"), defaultConfig(), + manifest("src/index.ts"), output("src/index.ts", { startLine: 2, endLine: 2, quote: "TODO_BUG" }), ), ).rejects.toMatchObject({ code: "malformed-output" }); }); - it("validates only findings kept by the feature finding cap", async () => { - const root = await fixtureRoot("clawpatch-review-validation-cap-"); - await writeFixture(root, "src/index.ts", "const value = 'TODO_BUG';\n"); - await writeFixture(root, "src/other.ts", "const value = 'hidden';\n"); - const config = defaultConfig(); - config.review.maxFindingsPerFeature = 1; - const providerOutput = output("src/index.ts"); - const keptFinding = providerOutput.findings[0]; - if (keptFinding === undefined) { - throw new Error("fixture output did not include a finding"); - } - providerOutput.findings.push({ - ...keptFinding, - title: "Discarded", - evidence: [ - { - path: "src/other.ts", - startLine: 1, - endLine: 1, - symbol: null, - quote: "hidden", - }, - ], - }); - providerOutput.inspected.files.push("src/other.ts"); + it("rejects evidence that only exists beyond the truncated prompt text", async () => { + const root = await fixtureRoot("clawpatch-review-validation-truncated-"); + await writeFixture(root, "src/index.ts", `${"a".repeat(24_000)}\nconst value = 'TODO_TAIL';\n`); await expect( - validateReviewOutput(root, feature("src/index.ts"), config, providerOutput), - ).resolves.toMatchObject({ findings: [{ title: "Bug" }] }); + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + manifest("src/index.ts", { truncated: true }), + output("src/index.ts", { startLine: null, endLine: null, quote: "TODO_TAIL" }), + ), + ).rejects.toMatchObject({ code: "malformed-output" }); }); }); @@ -153,3 +153,28 @@ function output( inspected: { files: [path], symbols: [], notes: [] }, }; } + +function manifest( + path: string, + options: { truncated?: boolean; readable?: boolean } = {}, +): ReviewPromptManifest { + const readable = options.readable ?? true; + return { + maxOwnedFiles: defaultConfig().review.maxOwnedFiles, + maxContextFiles: defaultConfig().review.maxContextFiles, + includedFiles: [ + { + path, + role: "owned", + bytes: readable ? 1 : 0, + includedBytes: readable ? 1 : 0, + truncated: options.truncated ?? false, + readable, + skippedReason: readable ? null : "unreadable", + }, + ], + omittedFiles: [], + promptBytes: 1, + approximateTokens: 1, + }; +} diff --git a/src/review-validation.ts b/src/review-validation.ts index 957fa04..1dce866 100644 --- a/src/review-validation.ts +++ b/src/review-validation.ts @@ -1,27 +1,34 @@ import { readFile, realpath } from "node:fs/promises"; import { isAbsolute, relative, resolve } from "node:path"; import { ClawpatchError } from "./errors.js"; +import { REVIEW_PROMPT_FILE_CHAR_LIMIT, type ReviewPromptManifest } from "./prompt.js"; import { ClawpatchConfig, FeatureRecord, ReviewOutput } from "./types.js"; export async function validateReviewOutput( root: string, feature: FeatureRecord, config: ClawpatchConfig, + manifest: ReviewPromptManifest, output: ReviewOutput, ): Promise<ReviewOutput> { const included = includedReviewPaths(feature, config); + const promptFiles = new Map(manifest.includedFiles.map((file) => [normalizePath(file.path), file])); const cache = new Map<string, Promise<string>>(); for (const file of output.inspected.files) { assertSafePath(file, "inspected file"); } - const findings = output.findings.slice(0, config.review.maxFindingsPerFeature); + const findings = output.findings; for (const finding of findings) { if (finding.evidence.length === 0) { throwMalformed(`finding "${finding.title}" has no evidence`); } for (const evidence of finding.evidence) { assertIncludedPath(evidence.path, included, "evidence file"); - const contents = await fileContents(root, evidence.path, cache); + const promptFile = promptFiles.get(normalizePath(evidence.path)); + if (promptFile === undefined || !promptFile.readable) { + throwMalformed(`evidence file was not readable in review context: ${evidence.path}`); + } + const contents = await fileContents(root, evidence.path, promptFile.truncated, cache); assertLineRange(contents, evidence); assertQuote(contents, evidence); } @@ -56,28 +63,31 @@ function assertSafePath(path: string, label: string): void { async function fileContents( root: string, path: string, + truncated: boolean, cache: Map<string, Promise<string>>, ): Promise<string> { const normalized = normalizePath(path); - const existing = cache.get(normalized); + const key = `${normalized}\0${truncated ? "truncated" : "full"}`; + const existing = cache.get(key); if (existing !== undefined) { return existing; } - const loaded = readIncludedFile(root, normalized); - cache.set(normalized, loaded); + const loaded = readIncludedFile(root, normalized, truncated); + cache.set(key, loaded); return loaded; } -async function readIncludedFile(root: string, path: string): Promise<string> { +async function readIncludedFile(root: string, path: string, truncated: boolean): Promise<string> { const full = resolve(root, path); const realRoot = await realpath(root).catch(() => root); const realFull = await realpath(full).catch(() => null); if (realFull === null || !isInside(realRoot, realFull)) { throwMalformed(`evidence file is not readable inside repository: ${path}`); } - return readFile(full, "utf8").catch(() => { + const contents = await readFile(full, "utf8").catch(() => { throwMalformed(`evidence file is not readable inside repository: ${path}`); }); + return truncated ? contents.slice(0, REVIEW_PROMPT_FILE_CHAR_LIMIT) : contents; } function assertLineRange( diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 09cd8d5..d047d6e 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3032,6 +3032,48 @@ describe("workflow", () => { delete process.env["CLAWPATCH_PROVIDER"]; }); + it("applies the finding cap after deslopify mode filtering", async () => { + const root = await fixtureRoot("clawpatch-deslopify-cap-"); + await writeFixture(root, "package.json", JSON.stringify({ name: "deslopify-cap" })); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG DESLOPIFY_LATE';\n"); + const previousProvider = process.env["CLAWPATCH_PROVIDER"]; + process.env["CLAWPATCH_PROVIDER"] = "mock"; + try { + const context = await makeContext(testOptions(root)); + const config = defaultConfig(); + config.review.maxFindingsPerFeature = 1; + + await initCommand(context, {}); + await writeFixture(root, ".clawpatch/config.json", JSON.stringify(config, null, 2)); + await mapCommand(context); + const paths = statePaths(join(root, ".clawpatch")); + const sourceFeature = (await readFeatures(paths)).find((feature) => + feature.ownedFiles.some((file) => file.path === "src/index.ts"), + ); + if (sourceFeature === undefined) { + throw new Error("missing source feature"); + } + const reviewed = await reviewCommand(context, { + feature: sourceFeature.featureId, + mode: "deslopify", + }); + const findings = await readFindings(paths); + + expect(reviewed).toMatchObject({ findings: 1 }); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + title: "Late simplification finding", + category: "maintainability", + }); + } finally { + if (previousProvider === undefined) { + delete process.env["CLAWPATCH_PROVIDER"]; + } else { + process.env["CLAWPATCH_PROVIDER"] = previousProvider; + } + } + }); + it("does not include escaped feature paths in prompts", async () => { const root = await fixtureRoot("clawpatch-path-escape-"); const siblingSecret = join(root, "..", "secret.txt"); From e139515a0f57e5c0dfa8941bfb069e9ce05800da Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 10:03:23 +0800 Subject: [PATCH 10/25] fix(ci): separate review and report findings --- src/app.ts | 12 +++++++--- src/review-validation.test.ts | 17 +++++++++++++ src/review-validation.ts | 3 --- src/workflow.test.ts | 45 +++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/app.ts b/src/app.ts index 35f196e..9523176 100644 --- a/src/app.ts +++ b/src/app.ts @@ -248,7 +248,8 @@ export async function ciCommand( output?: string | null; markdown?: string; }; - const summary = renderCiSummary({ initialized, mapped, reviewed, report }); + const reviewFindings = numberField(reviewed, "findings") ?? 0; + const summary = renderCiSummary({ initialized, mapped, reviewed, reviewFindings, report }); const githubStepSummary = process.env["GITHUB_STEP_SUMMARY"]; if (githubStepSummary !== undefined && githubStepSummary.length > 0) { await appendFile(githubStepSummary, summary, "utf8"); @@ -257,7 +258,8 @@ export async function ciCommand( initialized, mapped: numberField(mapped, "features"), reviewed: numberField(reviewed, "reviewed"), - findings: numberField(reviewed, "findings") ?? report.findings ?? 0, + findings: reviewFindings, + reportFindings: report.findings ?? 0, report: report.output ?? null, githubStepSummary: githubStepSummary ?? null, next: stringField(reviewed, "next") ?? "clawpatch status", @@ -1260,6 +1262,7 @@ function renderCiSummary(input: { initialized: boolean; mapped: unknown; reviewed: unknown; + reviewFindings: number; report: { findings?: number; output?: string | null }; }): string { const lines = [ @@ -1268,8 +1271,11 @@ function renderCiSummary(input: { `- initialized: ${input.initialized ? "yes" : "no"}`, `- mapped features: ${numberField(input.mapped, "features") ?? "unknown"}`, `- reviewed features: ${numberField(input.reviewed, "reviewed") ?? 0}`, - `- findings: ${numberField(input.reviewed, "findings") ?? input.report.findings ?? 0}`, + `- findings: ${input.reviewFindings}`, ]; + if (input.report.findings !== undefined && input.report.findings !== input.reviewFindings) { + lines.push(`- report findings: ${input.report.findings}`); + } if (input.report.output !== undefined && input.report.output !== null) { lines.push(`- report: ${input.report.output}`); } diff --git a/src/review-validation.test.ts b/src/review-validation.test.ts index 05418cc..beebb7b 100644 --- a/src/review-validation.test.ts +++ b/src/review-validation.test.ts @@ -21,6 +21,23 @@ describe("validateReviewOutput", () => { ).resolves.toMatchObject({ findings: [{ title: "Bug" }] }); }); + it("does not reject absolute inspected file metadata", async () => { + const root = await fixtureRoot("clawpatch-review-validation-inspected-"); + await writeFixture(root, "src/index.ts", "const value = 'TODO_BUG';\n"); + const providerOutput = output("src/index.ts"); + providerOutput.inspected.files = [`${root}/src/index.ts`]; + + await expect( + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + manifest("src/index.ts"), + providerOutput, + ), + ).resolves.toMatchObject({ findings: [{ title: "Bug" }] }); + }); + it("rejects evidence for files that were not included in review context", async () => { const root = await fixtureRoot("clawpatch-review-validation-path-"); await writeFixture(root, "src/index.ts", "const value = 'TODO_BUG';\n"); diff --git a/src/review-validation.ts b/src/review-validation.ts index 1dce866..6d4c2d5 100644 --- a/src/review-validation.ts +++ b/src/review-validation.ts @@ -14,9 +14,6 @@ export async function validateReviewOutput( const included = includedReviewPaths(feature, config); const promptFiles = new Map(manifest.includedFiles.map((file) => [normalizePath(file.path), file])); const cache = new Map<string, Promise<string>>(); - for (const file of output.inspected.files) { - assertSafePath(file, "inspected file"); - } const findings = output.findings; for (const finding of findings) { if (finding.evidence.length === 0) { diff --git a/src/workflow.test.ts b/src/workflow.test.ts index d047d6e..0953fed 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -488,6 +488,51 @@ describe("workflow", () => { } }); + it("does not count stale report findings as CI review findings", async () => { + const root = await fixtureRoot("clawpatch-ci-stale-findings-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "ci-stale", bin: { app: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git add package.json src/index.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "initial"'); + const summaryPath = join(root, "summary.md"); + const previousProvider = process.env["CLAWPATCH_PROVIDER"]; + const previousSummary = process.env["GITHUB_STEP_SUMMARY"]; + process.env["CLAWPATCH_PROVIDER"] = "mock"; + process.env["GITHUB_STEP_SUMMARY"] = summaryPath; + try { + const context = await makeContext(testOptions(root)); + await initCommand(context, {}); + await mapCommand(context); + await reviewCommand(context, { limit: "1" }); + + const result = await ciCommand(context, { since: "HEAD", limit: "10" }); + const summary = await readFile(summaryPath, "utf8"); + + expect(result).toMatchObject({ + findings: 0, + reportFindings: 1, + }); + expect(summary).toContain("- findings: 0"); + expect(summary).toContain("- report findings: 1"); + } finally { + if (previousProvider === undefined) { + delete process.env["CLAWPATCH_PROVIDER"]; + } else { + process.env["CLAWPATCH_PROVIDER"] = previousProvider; + } + if (previousSummary === undefined) { + delete process.env["GITHUB_STEP_SUMMARY"]; + } else { + process.env["GITHUB_STEP_SUMMARY"] = previousSummary; + } + } + }); + it.runIf(process.platform !== "win32")( "reviews end-to-end when codex writes fenced JSON with trailing prose", async () => { From 87ef57fdefb9de701a513e57b00888eb2951f6ce Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 10:11:36 +0800 Subject: [PATCH 11/25] fix(review): mark truncated prompt files explicitly --- src/prompt.test.ts | 19 ++++++++++++++++++- src/prompt.ts | 5 +++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/prompt.test.ts b/src/prompt.test.ts index 06013dc..daba65c 100644 --- a/src/prompt.test.ts +++ b/src/prompt.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { buildReviewPromptBundle } from "./prompt.js"; +import { REVIEW_PROMPT_FILE_CHAR_LIMIT, buildReviewPromptBundle } from "./prompt.js"; import { defaultConfig } from "./config.js"; import { fixtureRoot, writeFixture } from "./test-helpers.js"; import type { FeatureRecord, ProjectRecord } from "./types.js"; @@ -37,6 +37,23 @@ describe("review prompt provenance", () => { expect(bundle.manifest.promptBytes).toBeGreaterThan(0); expect(bundle.manifest.approximateTokens).toBeGreaterThan(0); }); + + it("marks exact marker-length replacements as truncated", async () => { + const root = await fixtureRoot("clawpatch-prompt-truncated-edge-"); + await writeFixture(root, "src/index.ts", "export const value = 1;\n"); + await writeFixture( + root, + "docs/large.md", + `${"x".repeat(REVIEW_PROMPT_FILE_CHAR_LIMIT)}TAIL_ONLY_TOKEN`, + ); + const bundle = await buildReviewPromptBundle(root, project(root), feature(), defaultConfig()); + + expect(bundle.manifest.includedFiles).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: "docs/large.md", truncated: true }), + ]), + ); + }); }); function project(root: string): ProjectRecord { diff --git a/src/prompt.ts b/src/prompt.ts index d555a1d..83ae0af 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -363,8 +363,9 @@ async function fileBlockWithManifest( }; } const bytes = Buffer.byteLength(contents, "utf8"); + const truncated = contents.length > REVIEW_PROMPT_FILE_CHAR_LIMIT; const trimmed = - contents.length > REVIEW_PROMPT_FILE_CHAR_LIMIT + truncated ? `${contents.slice(0, REVIEW_PROMPT_FILE_CHAR_LIMIT)}\n...[truncated]` : contents; return { @@ -374,7 +375,7 @@ async function fileBlockWithManifest( role, bytes, includedBytes: Buffer.byteLength(trimmed, "utf8"), - truncated: trimmed.length !== contents.length, + truncated, readable: true, skippedReason: null, }, From 7efaad59b9fa4c3aa4e2c73760c2b10c4d2dff6a Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 10:18:02 +0800 Subject: [PATCH 12/25] fix(pr): preserve staged rename paths --- src/app.ts | 86 +++++++++++++++++++++++++++++++++++++------- src/workflow.test.ts | 73 +++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 13 deletions(-) diff --git a/src/app.ts b/src/app.ts index 9523176..bfef8fe 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,4 +1,4 @@ -import { appendFile, readFile, writeFile } from "node:fs/promises"; +import { appendFile, readFile, stat, writeFile } from "node:fs/promises"; import { join, relative, resolve } from "node:path"; import { hostname } from "node:os"; import { @@ -1068,16 +1068,35 @@ export async function openPrCommand( }; } - await assertPatchWorktree(patch, git.root, loaded.paths.stateDir, gitFiles, force); + const patchWorktree = await assertPatchWorktree( + patch, + git.root, + loaded.paths.stateDir, + gitFiles, + force, + ); + const { commitFiles } = patchWorktree; let commitSha = patch.git.commitSha; if (commitSha === null) { if (git.currentBranch !== branch) { await checkedRun("git switch", runCommandArgs("git", ["switch", "-c", branch], git.root)); } - await checkedRun("git add", runCommandArgs("git", ["add", "--", ...gitFiles], git.root)); + const stagedOnlyFiles = new Set(patchWorktree.stagedOnlyFiles); + const stageableFiles = commitFiles.filter((file) => !stagedOnlyFiles.has(file)); + const addFiles = await existingGitFiles(git.root, stageableFiles); + const updateFiles = stageableFiles.filter((file) => !addFiles.includes(file)); + if (addFiles.length > 0) { + await checkedRun("git add", runCommandArgs("git", ["add", "--", ...addFiles], git.root)); + } + if (updateFiles.length > 0) { + await checkedRun( + "git add -u", + runCommandArgs("git", ["add", "-u", "--", ...updateFiles], git.root), + ); + } await checkedRun( "git commit", - runCommandArgs("git", ["commit", "-m", title, "--", ...gitFiles], git.root), + runCommandArgs("git", ["commit", "-m", title, "--", ...commitFiles], git.root), ); const commit = await checkedRun( "git rev-parse", @@ -1456,9 +1475,9 @@ async function assertPatchWorktree( stateDir: string, gitFiles: string[], force: boolean, -): Promise<void> { +): Promise<{ commitFiles: string[]; stagedOnlyFiles: string[] }> { if (patch.git.commitSha !== null) { - return; + return { commitFiles: gitFiles, stagedOnlyFiles: [] }; } const status = await checkedRun( "git status", @@ -1472,14 +1491,27 @@ async function assertPatchWorktree( }, ), ); - const dirty = gitStatusPaths(status.stdout); + const statusChanges = gitStatusChanges(status.stdout); + const dirty = uniqueStrings(statusChanges.flatMap((change) => change.paths)); const statePrefix = gitRelativePathPrefix(gitRoot, stateDir); const sourceDirty = dirty.filter((file) => !isStatePath(file, statePrefix)); if (sourceDirty.length === 0) { throw new ClawpatchError("no uncommitted patch changes to commit", 2, "invalid-input"); } const expected = new Set(gitFiles); - const extra = sourceDirty.filter((file) => !expected.has(file)); + const commitFiles = new Set(gitFiles); + const stagedOnlyFiles = new Set<string>(); + for (const change of statusChanges) { + if (change.secondaryPath === undefined) { + continue; + } + if (expected.has(change.primaryPath) || expected.has(change.secondaryPath)) { + commitFiles.add(change.primaryPath); + commitFiles.add(change.secondaryPath); + stagedOnlyFiles.add(change.secondaryPath); + } + } + const extra = sourceDirty.filter((file) => !commitFiles.has(file)); if (extra.length > 0 && !force) { throw new ClawpatchError( `dirty worktree has files outside patch attempt: ${extra.join(", ")}`, @@ -1495,22 +1527,37 @@ async function assertPatchWorktree( "invalid-input", ); } + return { commitFiles: [...commitFiles], stagedOnlyFiles: [...stagedOnlyFiles] }; } -function gitStatusPaths(output: string): string[] { +type GitStatusChange = { + paths: string[]; + primaryPath: string; + secondaryPath: string | undefined; +}; + +function gitStatusChanges(output: string): GitStatusChange[] { const fields = output.split("\0").filter((field) => field.length > 0); - const paths: string[] = []; + const changes: GitStatusChange[] = []; for (let index = 0; index < fields.length; index += 1) { const field = fields[index] ?? ""; if (field.length < 4) { continue; } - paths.push(field.slice(3)); - if (/[RC]/u.test(field.slice(0, 2))) { + const status = field.slice(0, 2); + const primaryPath = normalizePath(field.slice(3)); + const paths = [primaryPath]; + let secondaryPath: string | undefined; + if (/[RC]/u.test(status)) { + secondaryPath = normalizePath(fields[index + 1] ?? ""); + if (secondaryPath.length > 0) { + paths.push(secondaryPath); + } index += 1; } + changes.push({ paths, primaryPath, secondaryPath }); } - return paths.map(normalizePath); + return changes; } function isStatePath(file: string, statePrefix: string): boolean { @@ -1561,6 +1608,19 @@ function normalizePath(path: string): string { return path.replace(/\\/gu, "/"); } +function uniqueStrings(values: string[]): string[] { + return [...new Set(values)]; +} + +async function existingGitFiles(root: string, files: string[]): Promise<string[]> { + const existing = await Promise.all( + files.map(async (file) => + (await stat(resolve(root, file)).catch(() => null)) === null ? null : file, + ), + ); + return existing.filter((file): file is string => file !== null); +} + function normalizeDarwinPrivateVar(path: string): string { return normalizePath(path).replace(/^\/private\/var\//u, "/var/"); } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 0953fed..5d0572a 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3455,6 +3455,79 @@ describe("workflow", () => { } }); + it("opens PRs for staged renames when patch records only the destination", async () => { + const root = await fixtureRoot("clawpatch-open-pr-rename-"); + await writeFixture(root, "package.json", JSON.stringify({ name: "open-pr-rename" })); + await writeFixture(root, "docs/old.md", "TODO_BUG\n"); + await initGit(root); + await checkCommand(root, "git add package.json docs"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const origin = await fixtureRoot("clawpatch-open-pr-rename-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await checkCommand(root, "git mv docs/old.md docs/new.md"); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_rename", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Rename the reviewed file.", + filesChanged: ["docs/new.md"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-rename-gh-"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1001\n", + ); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = successGh; + const opened = (await openPrCommand(context, { + patch: "pat_open_pr_rename", + base: "main", + branch: "clawpatch/pat_open_pr_rename", + })) as { commit: string; pr: string }; + const committed = await runCommand(`git show --name-status --format= ${opened.commit}`, root); + + expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1001"); + expect(committed.stdout.trim()).toBe("R100\tdocs/old.md\tdocs/new.md"); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("persists failed patch attempts when provider fix throws", async () => { const root = await fixtureRoot("clawpatch-fix-fail-"); await runCommand( From 82ad1701472674aeca35ae08a5fa903d0be61242 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 10:23:24 +0800 Subject: [PATCH 13/25] fix(review): support PR resume edge cases --- src/app.ts | 14 ++++++- src/review-validation.test.ts | 10 +++++ src/review-validation.ts | 10 ++++- src/workflow.test.ts | 78 +++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 2 deletions(-) diff --git a/src/app.ts b/src/app.ts index bfef8fe..5449f14 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1079,7 +1079,10 @@ export async function openPrCommand( let commitSha = patch.git.commitSha; if (commitSha === null) { if (git.currentBranch !== branch) { - await checkedRun("git switch", runCommandArgs("git", ["switch", "-c", branch], git.root)); + const switchArgs = (await localBranchExists(git.root, branch)) + ? ["switch", branch] + : ["switch", "-c", branch]; + await checkedRun("git switch", runCommandArgs("git", switchArgs, git.root)); } const stagedOnlyFiles = new Set(patchWorktree.stagedOnlyFiles); const stageableFiles = commitFiles.filter((file) => !stagedOnlyFiles.has(file)); @@ -1600,6 +1603,15 @@ function githubCli(): string { return process.env["CLAWPATCH_GH"] ?? "gh"; } +async function localBranchExists(gitRoot: string, branch: string): Promise<boolean> { + const result = await runCommandArgs( + "git", + ["show-ref", "--verify", "--quiet", `refs/heads/${branch}`], + gitRoot, + ); + return result.exitCode === 0; +} + function firstUrl(output: string): string | null { return /https?:\/\/\S+/u.exec(output)?.[0] ?? null; } diff --git a/src/review-validation.test.ts b/src/review-validation.test.ts index beebb7b..7b631b5 100644 --- a/src/review-validation.test.ts +++ b/src/review-validation.test.ts @@ -77,6 +77,16 @@ describe("validateReviewOutput", () => { output("src/index.ts", { startLine: 1, endLine: 1, quote: "missing" }), ), ).rejects.toMatchObject({ code: "malformed-output" }); + + await expect( + validateReviewOutput( + root, + feature("src/index.ts"), + defaultConfig(), + manifest("src/index.ts"), + output("src/index.ts", { startLine: 2, endLine: 2, quote: null }), + ), + ).rejects.toMatchObject({ code: "malformed-output" }); }); it("rejects quotes that only match outside the cited line range", async () => { diff --git a/src/review-validation.ts b/src/review-validation.ts index 6d4c2d5..ff45416 100644 --- a/src/review-validation.ts +++ b/src/review-validation.ts @@ -101,7 +101,7 @@ function assertLineRange( if (startLine > endLine) { throwMalformed(`evidence line range is inverted: ${evidence.path}:${startLine}-${endLine}`); } - const lineCount = contents.length === 0 ? 1 : contents.split("\n").length; + const lineCount = reviewLineCount(contents); if (endLine > lineCount) { throwMalformed( `evidence line range exceeds file length: ${evidence.path}:${startLine}-${endLine}`, @@ -109,6 +109,14 @@ function assertLineRange( } } +function reviewLineCount(contents: string): number { + if (contents.length === 0) { + return 1; + } + const lines = contents.split("\n").length; + return contents.endsWith("\n") ? lines - 1 : lines; +} + function assertQuote( contents: string, evidence: ReviewOutput["findings"][number]["evidence"][number], diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 5d0572a..0518032 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3375,6 +3375,84 @@ describe("workflow", () => { } }); + it("switches to an existing patch branch when opening a PR", async () => { + const root = await fixtureRoot("clawpatch-open-pr-existing-branch-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "open-pr-existing-branch", bin: { open: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git add package.json src/index.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const origin = await fixtureRoot("clawpatch-open-pr-existing-branch-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await checkCommand(root, "git branch clawpatch/pat_open_pr_existing_branch"); + await writeFixture(root, "src/index.ts", "export const value = 'fixed';\n"); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_existing_branch", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: "clawpatch/pat_open_pr_existing_branch", + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-existing-branch-gh-"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1002\n", + ); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = successGh; + const opened = (await openPrCommand(context, { + patch: "pat_open_pr_existing_branch", + base: "main", + })) as { branch: string; pr: string }; + const currentBranch = (await runCommand("git branch --show-current", root)).stdout.trim(); + + expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1002"); + expect(opened.branch).toBe("clawpatch/pat_open_pr_existing_branch"); + expect(currentBranch).toBe("clawpatch/pat_open_pr_existing_branch"); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("opens PRs for quoted paths without committing pre-staged state", async () => { const root = await fixtureRoot("clawpatch-open-pr-pathspec-"); await writeFixture( From 4e7fa222e7ecf841197f4efd7aedead230836d98 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 10:34:01 +0800 Subject: [PATCH 14/25] fix(pr): avoid unsafe default PR bases --- CHANGELOG.md | 1 + src/app.ts | 15 ++++++---- src/workflow.test.ts | 66 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 623c763..b9d2639 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Added `clawpatch open-pr --patch <id>` to turn an applied patch attempt into an explicit GitHub pull request. - Added review prompt provenance and budget accounting for included files, omitted files, prompt bytes, and approximate tokens. - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. +- Fixed `clawpatch open-pr` so repositories without default-branch metadata use a dedicated patch branch and let GitHub choose the PR base. ## 0.3.0 - 2026-05-18 diff --git a/src/app.ts b/src/app.ts index 5449f14..ea7ad60 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1048,7 +1048,7 @@ export async function openPrCommand( if (git.root === null) { throw new ClawpatchError("open-pr requires a git repository", 2, "not-git-repository"); } - const base = stringFlag(flags, "base") ?? git.defaultBranch ?? "main"; + const base = stringFlag(flags, "base") ?? git.defaultBranch; const branch = prBranchName(patch, stringFlag(flags, "branch"), git.currentBranch, base); const findings = await readFindings(loaded.paths); const linkedFindings = findings.filter((finding) => patch.findingIds.includes(finding.findingId)); @@ -1116,8 +1116,6 @@ export async function openPrCommand( const ghArgs = [ "pr", "create", - "--base", - base, "--head", branch, "--title", @@ -1125,6 +1123,9 @@ export async function openPrCommand( "--body-file", "-", ]; + if (base !== null) { + ghArgs.splice(2, 0, "--base", base); + } if (flags["draft"] === true) { ghArgs.push("--draft"); } @@ -1360,7 +1361,7 @@ function prBranchName( patch: PatchAttempt, explicit: string | undefined, currentBranch: string | null, - base: string, + base: string | null, ): string { if (explicit !== undefined) { return explicit; @@ -1374,6 +1375,7 @@ function prBranchName( return patch.git.branchName; } if ( + base !== null && currentBranch !== null && currentBranch !== base && currentBranch !== "main" && @@ -1458,7 +1460,7 @@ function gitRelativePatchFiles(gitRoot: string, projectRoot: string, files: stri function plannedPrCommands( patch: PatchAttempt, branch: string, - base: string, + base: string | null, title: string, ): string[] { const commands: string[] = []; @@ -1468,7 +1470,8 @@ function plannedPrCommands( commands.push(`git commit -m ${title}`); } commands.push(`git push -u origin ${branch}`); - commands.push(`gh pr create --base ${base} --head ${branch} --title ${title} --body-file -`); + const baseArg = base === null ? "" : `--base ${base} `; + commands.push(`gh pr create ${baseArg}--head ${branch} --title ${title} --body-file -`); return commands; } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 0518032..1ce63bf 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3281,6 +3281,72 @@ describe("workflow", () => { } }); + it("uses a patch branch when the PR base is unknown", async () => { + const root = await fixtureRoot("clawpatch-open-pr-unknown-base-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "open-pr-unknown-base", bin: { open: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git branch -m develop"); + await checkCommand(root, "git add package.json src/index.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await writeFixture(root, "src/index.ts", "export const value = 'fixed';\n"); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_unknown_base", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + + const preview = await openPrCommand(context, { + patch: "pat_open_pr_unknown_base", + dryRun: true, + }); + + expect(preview).toMatchObject({ + branch: "clawpatch/pat_open_pr_unknown_base", + base: null, + }); + expect(preview).toMatchObject({ + commands: expect.arrayContaining([ + expect.stringContaining("gh pr create --head clawpatch/pat_open_pr_unknown_base"), + ]), + }); + expect(preview).toMatchObject({ + commands: expect.not.arrayContaining([expect.stringContaining("--base main")]), + }); + }); + it("persists the patch commit before failing external PR creation", async () => { const root = await fixtureRoot("clawpatch-open-pr-retry-"); await writeFixture( From c7a7f41f7420c482c7f6d6cb199d3dd030bd5126 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 10:40:45 +0800 Subject: [PATCH 15/25] fix(pr): align dry-run command previews --- src/app.ts | 63 +++++++++++++++++++++--------------------- src/workflow.test.ts | 66 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 31 deletions(-) diff --git a/src/app.ts b/src/app.ts index ea7ad60..c707b4d 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1055,7 +1055,8 @@ export async function openPrCommand( const title = prTitle(stringFlag(flags, "title"), linkedFindings, patch); const body = renderPatchPrBody(patch, linkedFindings); const gitFiles = gitRelativePatchFiles(git.root, loaded.root, patch.filesChanged); - const commands = plannedPrCommands(patch, branch, base, title); + const draft = flags["draft"] === true; + const commands = plannedPrCommands(patch, branch, base, title, gitFiles, draft); if (flags["dryRun"] === true) { return { dryRun: true, @@ -1113,22 +1114,7 @@ export async function openPrCommand( }); } await checkedRun("git push", runCommandArgs("git", ["push", "-u", "origin", branch], git.root)); - const ghArgs = [ - "pr", - "create", - "--head", - branch, - "--title", - title, - "--body-file", - "-", - ]; - if (base !== null) { - ghArgs.splice(2, 0, "--base", base); - } - if (flags["draft"] === true) { - ghArgs.push("--draft"); - } + const ghArgs = prCreateArgs(base, branch, title, draft); const gh = await checkedRun("gh pr create", runCommandArgs(githubCli(), ghArgs, git.root, body)); const prUrl = firstUrl(gh.stdout) ?? gh.stdout.trim(); await writePatchPrGitState(loaded.paths, patch, { commitSha, branchName: branch, prUrl }); @@ -1434,14 +1420,8 @@ function renderPatchPrBody(patch: PatchAttempt, findings: FindingRecord[]): stri } function gitRelativePatchFiles(gitRoot: string, projectRoot: string, files: string[]): string[] { - const projectPrefix = normalizePath(relative(gitRoot, projectRoot)); - const scopedPrefix = - projectPrefix.length > 0 && - projectPrefix !== "." && - !projectPrefix.startsWith("../") && - projectPrefix !== ".." - ? projectPrefix - : ""; + const projectPrefix = gitRelativePathPrefix(gitRoot, projectRoot); + const scopedPrefix = isUsableRelativePrefix(projectPrefix) ? projectPrefix : ""; return files.map((file) => { const relativeFile = normalizePath(file); if ( @@ -1462,19 +1442,36 @@ function plannedPrCommands( branch: string, base: string | null, title: string, + gitFiles: string[], + draft: boolean, ): string[] { const commands: string[] = []; if (patch.git.commitSha === null) { - commands.push(`git switch -c ${branch}`); - commands.push(`git add -- ${patch.filesChanged.join(" ")}`); - commands.push(`git commit -m ${title}`); + commands.push(`git switch -c ${shellArg(branch)}`); + commands.push(`git add -- ${gitFiles.map(shellArg).join(" ")}`); + commands.push(`git commit -m ${shellArg(title)} -- ${gitFiles.map(shellArg).join(" ")}`); } - commands.push(`git push -u origin ${branch}`); - const baseArg = base === null ? "" : `--base ${base} `; - commands.push(`gh pr create ${baseArg}--head ${branch} --title ${title} --body-file -`); + commands.push(`git push -u origin ${shellArg(branch)}`); + commands.push(`gh ${prCreateArgs(base, branch, title, draft).map(shellArg).join(" ")}`); return commands; } +function prCreateArgs( + base: string | null, + branch: string, + title: string, + draft: boolean, +): string[] { + const args = ["pr", "create", "--head", branch, "--title", title, "--body-file", "-"]; + if (base !== null) { + args.splice(2, 0, "--base", base); + } + if (draft) { + args.push("--draft"); + } + return args; +} + async function assertPatchWorktree( patch: PatchAttempt, gitRoot: string, @@ -1619,6 +1616,10 @@ function firstUrl(output: string): string | null { return /https?:\/\/\S+/u.exec(output)?.[0] ?? null; } +function shellArg(value: string): string { + return /^[A-Za-z0-9_./:@%+=,-]+$/u.test(value) ? value : `'${value.replace(/'/gu, "'\\''")}'`; +} + function normalizePath(path: string): string { return path.replace(/\\/gu, "/"); } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 1ce63bf..742bf61 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3347,6 +3347,72 @@ describe("workflow", () => { }); }); + it("previews PR commands with execution paths and draft flags", async () => { + const root = await fixtureRoot("clawpatch-open-pr-subdir-"); + const projectRoot = join(root, "packages/app"); + await writeFixture( + root, + "packages/app/package.json", + JSON.stringify({ name: "open-pr-subdir", bin: { open: "src/index.ts" } }), + ); + await writeFixture(root, "packages/app/src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git add packages"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const context = await makeContext(testOptions(projectRoot)); + const paths = statePaths(join(projectRoot, ".clawpatch")); + await initCommand(context, {}); + await writeFixture(root, "packages/app/src/index.ts", "export const value = 'fixed';\n"); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_subdir", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: projectRoot, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + + const preview = await openPrCommand(context, { + patch: "pat_open_pr_subdir", + base: "develop", + branch: "clawpatch/pat_open_pr_subdir", + draft: true, + dryRun: true, + }); + + expect(preview).toMatchObject({ + commands: expect.arrayContaining([ + expect.stringContaining("git add -- packages/app/src/index.ts"), + expect.stringContaining( + "gh pr create --base develop --head clawpatch/pat_open_pr_subdir", + ), + expect.stringContaining("--draft"), + ]), + }); + }); + it("persists the patch commit before failing external PR creation", async () => { const root = await fixtureRoot("clawpatch-open-pr-retry-"); await writeFixture( From e0d863aa16e023f2bb8aab1367d17c6bdac1e195 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 10:49:22 +0800 Subject: [PATCH 16/25] fix(pr): handle symlink and rerun edges --- src/app.ts | 19 ++++++- src/workflow.test.ts | 130 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 2 deletions(-) diff --git a/src/app.ts b/src/app.ts index c707b4d..e804f34 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,4 +1,4 @@ -import { appendFile, readFile, stat, writeFile } from "node:fs/promises"; +import { appendFile, lstat, readFile, writeFile } from "node:fs/promises"; import { join, relative, resolve } from "node:path"; import { hostname } from "node:os"; import { @@ -1050,6 +1050,21 @@ export async function openPrCommand( } const base = stringFlag(flags, "base") ?? git.defaultBranch; const branch = prBranchName(patch, stringFlag(flags, "branch"), git.currentBranch, base); + if ( + flags["dryRun"] !== true && + patch.git.prUrl !== null && + patch.git.commitSha !== null && + patch.git.branchName !== null + ) { + return { + patchAttempt: patch.patchAttemptId, + branch: patch.git.branchName, + base, + commit: patch.git.commitSha, + pr: patch.git.prUrl, + next: patch.git.prUrl, + }; + } const findings = await readFindings(loaded.paths); const linkedFindings = findings.filter((finding) => patch.findingIds.includes(finding.findingId)); const title = prTitle(stringFlag(flags, "title"), linkedFindings, patch); @@ -1631,7 +1646,7 @@ function uniqueStrings(values: string[]): string[] { async function existingGitFiles(root: string, files: string[]): Promise<string[]> { const existing = await Promise.all( files.map(async (file) => - (await stat(resolve(root, file)).catch(() => null)) === null ? null : file, + (await lstat(resolve(root, file)).catch(() => null)) === null ? null : file, ), ); return existing.filter((file): file is string => file !== null); diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 742bf61..392226c 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3413,6 +3413,136 @@ describe("workflow", () => { }); }); + it("opens PRs for newly created dangling symlinks", async () => { + const root = await fixtureRoot("clawpatch-open-pr-symlink-"); + await writeFixture(root, "package.json", JSON.stringify({ name: "open-pr-symlink" })); + await initGit(root); + await checkCommand(root, "git add package.json"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const origin = await fixtureRoot("clawpatch-open-pr-symlink-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await symlink("missing-target", join(root, "link")); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_symlink", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Add the symlink.", + filesChanged: ["link"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-symlink-gh-"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1003\n", + ); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = successGh; + const opened = (await openPrCommand(context, { + patch: "pat_open_pr_symlink", + base: "main", + branch: "clawpatch/pat_open_pr_symlink", + })) as { commit: string; pr: string }; + const committed = await runCommand(`git show --name-status --format= ${opened.commit}`, root); + + expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1003"); + expect(committed.stdout.trim()).toBe("A\tlink"); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + + it("returns an existing PR URL without recreating it", async () => { + const root = await fixtureRoot("clawpatch-open-pr-existing-url-"); + await writeFixture(root, "package.json", JSON.stringify({ name: "open-pr-existing-url" })); + await writeFixture(root, "src/index.ts", "export const value = 'fixed';\n"); + await initGit(root); + await checkCommand(root, "git add package.json src/index.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const commitSha = (await runCommand("git rev-parse HEAD", root)).stdout.trim(); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_existing_url", + findingIds: [], + featureIds: [], + status: "validated", + plan: "Already opened.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [], + provider: null, + git: { + baseSha: commitSha, + commitSha, + branchName: "clawpatch/pat_open_pr_existing_url", + prUrl: "https://github.com/openclaw/clawpatch/pull/1004", + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-existing-url-gh-"); + const failingGh = join(ghScripts, "fail-gh.sh"); + await writeFixture(ghScripts, "fail-gh.sh", "#!/bin/sh\nexit 42\n"); + await chmod(failingGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = failingGh; + await expect( + openPrCommand(context, { + patch: "pat_open_pr_existing_url", + base: "main", + }), + ).resolves.toMatchObject({ + pr: "https://github.com/openclaw/clawpatch/pull/1004", + branch: "clawpatch/pat_open_pr_existing_url", + commit: commitSha, + }); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("persists the patch commit before failing external PR creation", async () => { const root = await fixtureRoot("clawpatch-open-pr-retry-"); await writeFixture( From 2d846e85bb333cd0ebba2537350acb484d322bd2 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 11:01:44 +0800 Subject: [PATCH 17/25] fix(ci): forward git repo check override --- README.md | 3 ++- src/app.ts | 9 ++++++--- src/cli.ts | 2 ++ src/workflow.test.ts | 3 +++ 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8653aa7..5ee40b4 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,8 @@ to features so runs can resume and be audited. - Review does not edit files. - Fix is explicit and selected by finding ID. - Fix refuses a dirty source worktree by default. -- Clawpatch never commits, pushes, opens PRs, or lands changes today. +- Clawpatch commits, pushes, and opens PRs only from explicit patch commands such as `open-pr`. +- Clawpatch does not land changes today. - Provider output is parsed through strict schemas. - Symlinked directories and generated build output are skipped during mapping. diff --git a/src/app.ts b/src/app.ts index e804f34..262b320 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1255,18 +1255,21 @@ function applyProviderFlags( }; } -function providerFlagSubset(flags: Record<string, string | boolean>): Record<string, string> { - const subset: Record<string, string> = {}; +function providerFlagSubset(flags: Record<string, string | boolean>): Record<string, string | boolean> { + const subset: Record<string, string | boolean> = {}; for (const flag of ["provider", "model", "reasoningEffort"] as const) { const value = stringFlag(flags, flag); if (value !== undefined) { subset[flag] = value; } } + if (flags["skipGitRepoCheck"] === true) { + subset["skipGitRepoCheck"] = true; + } return subset; } -function reviewFlagSubset(flags: Record<string, string | boolean>): Record<string, string> { +function reviewFlagSubset(flags: Record<string, string | boolean>): Record<string, string | boolean> { const subset = providerFlagSubset(flags); for (const flag of ["since", "limit", "jobs"] as const) { const value = stringFlag(flags, flag); diff --git a/src/cli.ts b/src/cli.ts index 28e8b16..e6f858c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -176,6 +176,7 @@ const commandFlags = { "provider", "model", "reasoningEffort", + "skipGitRepoCheck", "output", ]), report: new Set(["status", "severity", "feature", "project", "category", "triage", "output"]), @@ -458,6 +459,7 @@ Flags: --provider <name> --model <name> --reasoning-effort <none|minimal|low|medium|high|xhigh> + --skip-git-repo-check --output <path> --json `); diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 392226c..7c9314b 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -243,6 +243,9 @@ describe("workflow", () => { expect(parseArgs(["review", "--skip-git-repo-check"]).flags).toMatchObject({ skipGitRepoCheck: true, }); + expect(parseArgs(["ci", "--skip-git-repo-check"]).flags).toMatchObject({ + skipGitRepoCheck: true, + }); expect(parseArgs(["fix", "--finding", "f", "--dry-run"]).flags).toMatchObject({ dryRun: true, finding: "f", From cd12ab08a7099f92ef7e83128e89d003d569bef3 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 11:07:12 +0800 Subject: [PATCH 18/25] fix(pr): use patch branches for unknown bases --- src/app.ts | 14 ++++++++++++-- src/workflow.test.ts | 13 ++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/app.ts b/src/app.ts index 262b320..6113061 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1071,7 +1071,11 @@ export async function openPrCommand( const body = renderPatchPrBody(patch, linkedFindings); const gitFiles = gitRelativePatchFiles(git.root, loaded.root, patch.filesChanged); const draft = flags["draft"] === true; - const commands = plannedPrCommands(patch, branch, base, title, gitFiles, draft); + const branchExists = + flags["dryRun"] === true && patch.git.commitSha === null + ? await localBranchExists(git.root, branch) + : false; + const commands = plannedPrCommands(patch, branch, base, title, gitFiles, draft, branchExists); if (flags["dryRun"] === true) { return { dryRun: true, @@ -1370,6 +1374,11 @@ function prBranchName( if (explicit !== undefined) { return explicit; } + if (base === null) { + return patch.git.branchName?.startsWith("clawpatch/") === true + ? patch.git.branchName + : `clawpatch/${patch.patchAttemptId}`; + } if ( patch.git.branchName !== null && patch.git.branchName !== base && @@ -1462,10 +1471,11 @@ function plannedPrCommands( title: string, gitFiles: string[], draft: boolean, + branchExists: boolean, ): string[] { const commands: string[] = []; if (patch.git.commitSha === null) { - commands.push(`git switch -c ${shellArg(branch)}`); + commands.push(branchExists ? `git switch ${shellArg(branch)}` : `git switch -c ${shellArg(branch)}`); commands.push(`git add -- ${gitFiles.map(shellArg).join(" ")}`); commands.push(`git commit -m ${shellArg(title)} -- ${gitFiles.map(shellArg).join(" ")}`); } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 7c9314b..ed458bd 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3324,7 +3324,7 @@ describe("workflow", () => { git: { baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), commitSha: null, - branchName: null, + branchName: "develop", prUrl: null, }, createdAt: now, @@ -3700,12 +3700,23 @@ describe("workflow", () => { const previousGh = process.env["CLAWPATCH_GH"]; try { process.env["CLAWPATCH_GH"] = successGh; + const preview = (await openPrCommand(context, { + patch: "pat_open_pr_existing_branch", + base: "main", + dryRun: true, + })) as { commands: string[] }; const opened = (await openPrCommand(context, { patch: "pat_open_pr_existing_branch", base: "main", })) as { branch: string; pr: string }; const currentBranch = (await runCommand("git branch --show-current", root)).stdout.trim(); + expect(preview.commands).toEqual( + expect.arrayContaining(["git switch clawpatch/pat_open_pr_existing_branch"]), + ); + expect(preview.commands).toEqual( + expect.not.arrayContaining(["git switch -c clawpatch/pat_open_pr_existing_branch"]), + ); expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1002"); expect(opened.branch).toBe("clawpatch/pat_open_pr_existing_branch"); expect(currentBranch).toBe("clawpatch/pat_open_pr_existing_branch"); From 744c541a4ea0f155289c2c3458966cda3c45053a Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 11:14:28 +0800 Subject: [PATCH 19/25] fix(pr): show dry-run command preview --- src/app.ts | 1 + src/workflow.test.ts | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/app.ts b/src/app.ts index 6113061..0fe6600 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1085,6 +1085,7 @@ export async function openPrCommand( title, body, commands, + commandsPreview: commands.join("\n"), }; } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index ed458bd..3d21561 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3261,6 +3261,18 @@ describe("workflow", () => { const stored = (await readPatchAttempts(paths)).find( (candidate) => candidate.patchAttemptId === patch.patchAttemptId, ); + const cliPreview = await runCli([ + "--root", + root, + "open-pr", + "--patch", + patch.patchAttemptId, + "--base", + "main", + "--branch", + "clawpatch/pat_open_pr", + "--dry-run", + ]); expect(preview).toMatchObject({ dryRun: true, @@ -3274,6 +3286,8 @@ describe("workflow", () => { expect.stringContaining("gh pr create --base main --head clawpatch/pat_open_pr"), ]), }); + expect(cliPreview.stdout).toContain("commandsPreview: git switch"); + expect(cliPreview.stdout).toContain("gh pr create --base main --head clawpatch/pat_open_pr"); expect(stored?.git.prUrl).toBeNull(); } finally { if (previousProvider === undefined) { From bf35e80464da1b8141b3c0ad17c7d8df0a1ded09 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 14:22:09 +0800 Subject: [PATCH 20/25] fix(pr): correct dry-run patch staging preview --- src/app.ts | 67 ++++++++++++++++++++++++++++++++++--------- src/workflow.test.ts | 68 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 13 deletions(-) diff --git a/src/app.ts b/src/app.ts index 0fe6600..6fc04fe 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1071,11 +1071,27 @@ export async function openPrCommand( const body = renderPatchPrBody(patch, linkedFindings); const gitFiles = gitRelativePatchFiles(git.root, loaded.root, patch.filesChanged); const draft = flags["draft"] === true; + const dryRunStagePlan = + flags["dryRun"] === true && patch.git.commitSha === null + ? await patchStagePlan( + git.root, + await assertPatchWorktree(patch, git.root, loaded.paths.stateDir, gitFiles, force), + ) + : null; const branchExists = flags["dryRun"] === true && patch.git.commitSha === null ? await localBranchExists(git.root, branch) : false; - const commands = plannedPrCommands(patch, branch, base, title, gitFiles, draft, branchExists); + const commands = plannedPrCommands( + patch, + branch, + base, + title, + gitFiles, + draft, + branchExists, + dryRunStagePlan, + ); if (flags["dryRun"] === true) { return { dryRun: true, @@ -1096,7 +1112,6 @@ export async function openPrCommand( gitFiles, force, ); - const { commitFiles } = patchWorktree; let commitSha = patch.git.commitSha; if (commitSha === null) { if (git.currentBranch !== branch) { @@ -1105,22 +1120,22 @@ export async function openPrCommand( : ["switch", "-c", branch]; await checkedRun("git switch", runCommandArgs("git", switchArgs, git.root)); } - const stagedOnlyFiles = new Set(patchWorktree.stagedOnlyFiles); - const stageableFiles = commitFiles.filter((file) => !stagedOnlyFiles.has(file)); - const addFiles = await existingGitFiles(git.root, stageableFiles); - const updateFiles = stageableFiles.filter((file) => !addFiles.includes(file)); - if (addFiles.length > 0) { - await checkedRun("git add", runCommandArgs("git", ["add", "--", ...addFiles], git.root)); + const stagePlan = await patchStagePlan(git.root, patchWorktree); + if (stagePlan.addFiles.length > 0) { + await checkedRun( + "git add", + runCommandArgs("git", ["add", "--", ...stagePlan.addFiles], git.root), + ); } - if (updateFiles.length > 0) { + if (stagePlan.updateFiles.length > 0) { await checkedRun( "git add -u", - runCommandArgs("git", ["add", "-u", "--", ...updateFiles], git.root), + runCommandArgs("git", ["add", "-u", "--", ...stagePlan.updateFiles], git.root), ); } await checkedRun( "git commit", - runCommandArgs("git", ["commit", "-m", title, "--", ...commitFiles], git.root), + runCommandArgs("git", ["commit", "-m", title, "--", ...stagePlan.commitFiles], git.root), ); const commit = await checkedRun( "git rev-parse", @@ -1473,12 +1488,21 @@ function plannedPrCommands( gitFiles: string[], draft: boolean, branchExists: boolean, + stagePlan: PatchStagePlan | null, ): string[] { const commands: string[] = []; if (patch.git.commitSha === null) { + const commitFiles = stagePlan?.commitFiles ?? gitFiles; + const addFiles = stagePlan?.addFiles ?? gitFiles; + const updateFiles = stagePlan?.updateFiles ?? []; commands.push(branchExists ? `git switch ${shellArg(branch)}` : `git switch -c ${shellArg(branch)}`); - commands.push(`git add -- ${gitFiles.map(shellArg).join(" ")}`); - commands.push(`git commit -m ${shellArg(title)} -- ${gitFiles.map(shellArg).join(" ")}`); + if (addFiles.length > 0) { + commands.push(`git add -- ${addFiles.map(shellArg).join(" ")}`); + } + if (updateFiles.length > 0) { + commands.push(`git add -u -- ${updateFiles.map(shellArg).join(" ")}`); + } + commands.push(`git commit -m ${shellArg(title)} -- ${commitFiles.map(shellArg).join(" ")}`); } commands.push(`git push -u origin ${shellArg(branch)}`); commands.push(`gh ${prCreateArgs(base, branch, title, draft).map(shellArg).join(" ")}`); @@ -1562,6 +1586,23 @@ async function assertPatchWorktree( return { commitFiles: [...commitFiles], stagedOnlyFiles: [...stagedOnlyFiles] }; } +type PatchStagePlan = { + commitFiles: string[]; + addFiles: string[]; + updateFiles: string[]; +}; + +async function patchStagePlan( + root: string, + patchWorktree: { commitFiles: string[]; stagedOnlyFiles: string[] }, +): Promise<PatchStagePlan> { + const stagedOnlyFiles = new Set(patchWorktree.stagedOnlyFiles); + const stageableFiles = patchWorktree.commitFiles.filter((file) => !stagedOnlyFiles.has(file)); + const addFiles = await existingGitFiles(root, stageableFiles); + const updateFiles = stageableFiles.filter((file) => !addFiles.includes(file)); + return { commitFiles: patchWorktree.commitFiles, addFiles, updateFiles }; +} + type GitStatusChange = { paths: string[]; primaryPath: string; diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 3d21561..e180e46 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3878,6 +3878,12 @@ describe("workflow", () => { const previousGh = process.env["CLAWPATCH_GH"]; try { process.env["CLAWPATCH_GH"] = successGh; + const preview = (await openPrCommand(context, { + patch: "pat_open_pr_rename", + base: "main", + branch: "clawpatch/pat_open_pr_rename", + dryRun: true, + })) as { commands: string[] }; const opened = (await openPrCommand(context, { patch: "pat_open_pr_rename", base: "main", @@ -3885,6 +3891,12 @@ describe("workflow", () => { })) as { commit: string; pr: string }; const committed = await runCommand(`git show --name-status --format= ${opened.commit}`, root); + expect(preview.commands).toContain("git add -- docs/new.md"); + expect(preview.commands).toEqual( + expect.arrayContaining([ + expect.stringMatching(/git commit .*docs\/new\.md.*docs\/old\.md/u), + ]), + ); expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1001"); expect(committed.stdout.trim()).toBe("R100\tdocs/old.md\tdocs/new.md"); } finally { @@ -3896,6 +3908,62 @@ describe("workflow", () => { } }); + it("previews deletion patch PRs with update staging", async () => { + const root = await fixtureRoot("clawpatch-open-pr-delete-"); + await writeFixture(root, "package.json", JSON.stringify({ name: "open-pr-delete" })); + await writeFixture(root, "docs/old.md", "TODO_BUG\n"); + await initGit(root); + await checkCommand(root, "git add package.json docs"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await rm(join(root, "docs/old.md")); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_delete", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Delete the reviewed file.", + filesChanged: ["docs/old.md"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + + const preview = (await openPrCommand(context, { + patch: "pat_open_pr_delete", + base: "main", + branch: "clawpatch/pat_open_pr_delete", + dryRun: true, + })) as { commands: string[] }; + + expect(preview.commands).toContain("git add -u -- docs/old.md"); + expect(preview.commands).toEqual( + expect.arrayContaining([expect.stringMatching(/git commit .*docs\/old\.md/u)]), + ); + expect(preview.commands).not.toContain("git add -- docs/old.md"); + }); + it("persists failed patch attempts when provider fix throws", async () => { const root = await fixtureRoot("clawpatch-fix-fail-"); await runCommand( From 926e834b68e3c8f77991f073715fe8502c467101 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 14:33:52 +0800 Subject: [PATCH 21/25] fix(pr): use literal git pathspecs --- src/app.ts | 32 ++++++++++++--- src/workflow.test.ts | 92 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/app.ts b/src/app.ts index 6fc04fe..f655449 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1124,18 +1124,30 @@ export async function openPrCommand( if (stagePlan.addFiles.length > 0) { await checkedRun( "git add", - runCommandArgs("git", ["add", "--", ...stagePlan.addFiles], git.root), + runCommandArgs( + "git", + ["add", "--", ...stagePlan.addFiles.map(gitPathspec)], + git.root, + ), ); } if (stagePlan.updateFiles.length > 0) { await checkedRun( "git add -u", - runCommandArgs("git", ["add", "-u", "--", ...stagePlan.updateFiles], git.root), + runCommandArgs( + "git", + ["add", "-u", "--", ...stagePlan.updateFiles.map(gitPathspec)], + git.root, + ), ); } await checkedRun( "git commit", - runCommandArgs("git", ["commit", "-m", title, "--", ...stagePlan.commitFiles], git.root), + runCommandArgs( + "git", + ["commit", "-m", title, "--", ...stagePlan.commitFiles.map(gitPathspec)], + git.root, + ), ); const commit = await checkedRun( "git rev-parse", @@ -1497,12 +1509,12 @@ function plannedPrCommands( const updateFiles = stagePlan?.updateFiles ?? []; commands.push(branchExists ? `git switch ${shellArg(branch)}` : `git switch -c ${shellArg(branch)}`); if (addFiles.length > 0) { - commands.push(`git add -- ${addFiles.map(shellArg).join(" ")}`); + commands.push(`git add -- ${shellPathspecArgs(addFiles)}`); } if (updateFiles.length > 0) { - commands.push(`git add -u -- ${updateFiles.map(shellArg).join(" ")}`); + commands.push(`git add -u -- ${shellPathspecArgs(updateFiles)}`); } - commands.push(`git commit -m ${shellArg(title)} -- ${commitFiles.map(shellArg).join(" ")}`); + commands.push(`git commit -m ${shellArg(title)} -- ${shellPathspecArgs(commitFiles)}`); } commands.push(`git push -u origin ${shellArg(branch)}`); commands.push(`gh ${prCreateArgs(base, branch, title, draft).map(shellArg).join(" ")}`); @@ -1686,6 +1698,14 @@ function firstUrl(output: string): string | null { return /https?:\/\/\S+/u.exec(output)?.[0] ?? null; } +function gitPathspec(path: string): string { + return `:(literal)${path}`; +} + +function shellPathspecArgs(files: string[]): string { + return files.map((file) => shellArg(gitPathspec(file))).join(" "); +} + function shellArg(value: string): string { return /^[A-Za-z0-9_./:@%+=,-]+$/u.test(value) ? value : `'${value.replace(/'/gu, "'\\''")}'`; } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index e180e46..6bb1860 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3421,7 +3421,7 @@ describe("workflow", () => { expect(preview).toMatchObject({ commands: expect.arrayContaining([ - expect.stringContaining("git add -- packages/app/src/index.ts"), + expect.stringContaining("git add -- ':(literal)packages/app/src/index.ts'"), expect.stringContaining( "gh pr create --base develop --head clawpatch/pat_open_pr_subdir", ), @@ -3823,6 +3823,92 @@ describe("workflow", () => { } }); + it("opens PRs for literal names that look like git pathspec magic", async () => { + const root = await fixtureRoot("clawpatch-open-pr-literal-pathspec-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "open-pr-literal-pathspec" }), + ); + await writeFixture(root, "README.md", "base\n"); + await initGit(root); + await checkCommand(root, "git add package.json README.md"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const origin = await fixtureRoot("clawpatch-open-pr-literal-pathspec-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await writeFixture(root, ":(top)README.md", "literal\n"); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_literal_pathspec", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Add the reviewed literal pathspec-looking file.", + filesChanged: [":(top)README.md"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-literal-pathspec-gh-"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1002\n", + ); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = successGh; + const preview = (await openPrCommand(context, { + patch: "pat_open_pr_literal_pathspec", + base: "main", + branch: "clawpatch/pat_open_pr_literal_pathspec", + dryRun: true, + })) as { commands: string[] }; + const opened = (await openPrCommand(context, { + patch: "pat_open_pr_literal_pathspec", + base: "main", + branch: "clawpatch/pat_open_pr_literal_pathspec", + })) as { commit: string; pr: string }; + const committed = await runCommand(`git show --name-status --format= ${opened.commit}`, root); + const readme = await readFile(join(root, "README.md"), "utf8"); + + expect(preview.commands).toContain("git add -- ':(literal):(top)README.md'"); + expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1002"); + expect(committed.stdout.trim()).toBe("A\t:(top)README.md"); + expect(readme).toBe("base\n"); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("opens PRs for staged renames when patch records only the destination", async () => { const root = await fixtureRoot("clawpatch-open-pr-rename-"); await writeFixture(root, "package.json", JSON.stringify({ name: "open-pr-rename" })); @@ -3891,7 +3977,7 @@ describe("workflow", () => { })) as { commit: string; pr: string }; const committed = await runCommand(`git show --name-status --format= ${opened.commit}`, root); - expect(preview.commands).toContain("git add -- docs/new.md"); + expect(preview.commands).toContain("git add -- ':(literal)docs/new.md'"); expect(preview.commands).toEqual( expect.arrayContaining([ expect.stringMatching(/git commit .*docs\/new\.md.*docs\/old\.md/u), @@ -3957,7 +4043,7 @@ describe("workflow", () => { dryRun: true, })) as { commands: string[] }; - expect(preview.commands).toContain("git add -u -- docs/old.md"); + expect(preview.commands).toContain("git add -u -- ':(literal)docs/old.md'"); expect(preview.commands).toEqual( expect.arrayContaining([expect.stringMatching(/git commit .*docs\/old\.md/u)]), ); From 64810a3f39a74dd3ca179b039911f547aa62cded Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 14:40:55 +0800 Subject: [PATCH 22/25] fix(pr): resolve symlinked project roots --- src/app.ts | 35 ++++++++++++++---- src/workflow.test.ts | 88 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/app.ts b/src/app.ts index f655449..53a0e5e 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,4 +1,4 @@ -import { appendFile, lstat, readFile, writeFile } from "node:fs/promises"; +import { appendFile, lstat, readFile, realpath, writeFile } from "node:fs/promises"; import { join, relative, resolve } from "node:path"; import { hostname } from "node:os"; import { @@ -1069,7 +1069,7 @@ export async function openPrCommand( const linkedFindings = findings.filter((finding) => patch.findingIds.includes(finding.findingId)); const title = prTitle(stringFlag(flags, "title"), linkedFindings, patch); const body = renderPatchPrBody(patch, linkedFindings); - const gitFiles = gitRelativePatchFiles(git.root, loaded.root, patch.filesChanged); + const gitFiles = await gitRelativePatchFiles(git.root, loaded.root, patch.filesChanged); const draft = flags["draft"] === true; const dryRunStagePlan = flags["dryRun"] === true && patch.git.commitSha === null @@ -1474,8 +1474,19 @@ function renderPatchPrBody(patch: PatchAttempt, findings: FindingRecord[]): stri return `${lines.join("\n")}\n`; } -function gitRelativePatchFiles(gitRoot: string, projectRoot: string, files: string[]): string[] { - const projectPrefix = gitRelativePathPrefix(gitRoot, projectRoot); +async function gitRelativePatchFiles( + gitRoot: string, + projectRoot: string, + files: string[], +): Promise<string[]> { + const projectPrefix = await gitRelativePathPrefix(gitRoot, projectRoot); + if (projectPrefix === ".." || projectPrefix.startsWith("../")) { + throw new ClawpatchError( + `project root is outside git repository: ${projectRoot}`, + 2, + "invalid-root", + ); + } const scopedPrefix = isUsableRelativePrefix(projectPrefix) ? projectPrefix : ""; return files.map((file) => { const relativeFile = normalizePath(file); @@ -1561,7 +1572,7 @@ async function assertPatchWorktree( ); const statusChanges = gitStatusChanges(status.stdout); const dirty = uniqueStrings(statusChanges.flatMap((change) => change.paths)); - const statePrefix = gitRelativePathPrefix(gitRoot, stateDir); + const statePrefix = await gitRelativePathPrefix(gitRoot, stateDir); const sourceDirty = dirty.filter((file) => !isStatePath(file, statePrefix)); if (sourceDirty.length === 0) { throw new ClawpatchError("no uncommitted patch changes to commit", 2, "invalid-input"); @@ -1649,13 +1660,21 @@ function isStatePath(file: string, statePrefix: string): boolean { return statePrefix.length > 0 && (file === statePrefix || file.startsWith(`${statePrefix}/`)); } -function gitRelativePathPrefix(gitRoot: string, path: string): string { +async function gitRelativePathPrefix(gitRoot: string, path: string): Promise<string> { const direct = normalizePath(relative(gitRoot, path)); if (isUsableRelativePrefix(direct)) { return direct; } - const normalizedGitRoot = normalizeDarwinPrivateVar(gitRoot); - const normalizedPath = normalizeDarwinPrivateVar(path); + const [realGitRoot, realPath] = await Promise.all([ + realpath(gitRoot).catch(() => gitRoot), + realpath(path).catch(() => path), + ]); + const resolved = normalizePath(relative(realGitRoot, realPath)); + if (resolved === "" || isUsableRelativePrefix(resolved)) { + return resolved; + } + const normalizedGitRoot = normalizeDarwinPrivateVar(realGitRoot); + const normalizedPath = normalizeDarwinPrivateVar(realPath); if (normalizedPath === normalizedGitRoot) { return ""; } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 6bb1860..a89755e 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3430,6 +3430,94 @@ describe("workflow", () => { }); }); + it("opens PRs from symlinked project roots with repo-relative patch paths", async () => { + const root = await fixtureRoot("clawpatch-open-pr-symlink-root-"); + const projectRoot = join(root, "packages/app"); + await writeFixture( + root, + "packages/app/package.json", + JSON.stringify({ name: "open-pr-symlink-root" }), + ); + await writeFixture(root, "packages/app/src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git add packages"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const origin = await fixtureRoot("clawpatch-open-pr-symlink-root-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const linkParent = await fixtureRoot("clawpatch-open-pr-symlink-root-link-"); + const linkedProjectRoot = join(linkParent, "app"); + await symlink(projectRoot, linkedProjectRoot); + const context = await makeContext(testOptions(linkedProjectRoot)); + const paths = statePaths(join(linkedProjectRoot, ".clawpatch")); + await initCommand(context, {}); + await writeFixture(root, "packages/app/src/index.ts", "export const value = 'fixed';\n"); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_symlink_root", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: linkedProjectRoot, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha: (await runCommand("git rev-parse HEAD", root)).stdout.trim(), + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-symlink-root-gh-"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1004\n", + ); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = successGh; + const preview = (await openPrCommand(context, { + patch: "pat_open_pr_symlink_root", + base: "main", + branch: "clawpatch/pat_open_pr_symlink_root", + dryRun: true, + })) as { commands: string[] }; + const opened = (await openPrCommand(context, { + patch: "pat_open_pr_symlink_root", + base: "main", + branch: "clawpatch/pat_open_pr_symlink_root", + })) as { commit: string; pr: string }; + const committed = await runCommand(`git show --name-only --format= ${opened.commit}`, root); + + expect(preview.commands).toContain("git add -- ':(literal)packages/app/src/index.ts'"); + expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1004"); + expect(committed.stdout.trim()).toBe("packages/app/src/index.ts"); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("opens PRs for newly created dangling symlinks", async () => { const root = await fixtureRoot("clawpatch-open-pr-symlink-"); await writeFixture(root, "package.json", JSON.stringify({ name: "open-pr-symlink" })); From 5350752961f3ff693fcb51ed7143a13c409c60ad Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 15:14:09 +0800 Subject: [PATCH 23/25] chore: align changed files with linux oxfmt --- src/app.ts | 35 ++++++++++++++++++++++++----------- src/prompt.test.ts | 4 +--- src/prompt.ts | 7 +++---- src/review-validation.test.ts | 6 +----- src/review-validation.ts | 9 ++++----- src/workflow.test.ts | 10 ++-------- 6 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/app.ts b/src/app.ts index 53a0e5e..dbcdaef 100644 --- a/src/app.ts +++ b/src/app.ts @@ -675,7 +675,11 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId mode, customPrompt, ); - const providerOutput = await provider.review(loaded.root, reviewPrompt.prompt, providerOptions(config)); + const providerOutput = await provider.review( + loaded.root, + reviewPrompt.prompt, + providerOptions(config), + ); const reviewOutput = { ...providerOutput, findings: reviewFindingsForMode(providerOutput.findings, mode).slice( @@ -1124,11 +1128,7 @@ export async function openPrCommand( if (stagePlan.addFiles.length > 0) { await checkedRun( "git add", - runCommandArgs( - "git", - ["add", "--", ...stagePlan.addFiles.map(gitPathspec)], - git.root, - ), + runCommandArgs("git", ["add", "--", ...stagePlan.addFiles.map(gitPathspec)], git.root), ); } if (stagePlan.updateFiles.length > 0) { @@ -1287,7 +1287,9 @@ function applyProviderFlags( }; } -function providerFlagSubset(flags: Record<string, string | boolean>): Record<string, string | boolean> { +function providerFlagSubset( + flags: Record<string, string | boolean>, +): Record<string, string | boolean> { const subset: Record<string, string | boolean> = {}; for (const flag of ["provider", "model", "reasoningEffort"] as const) { const value = stringFlag(flags, flag); @@ -1301,7 +1303,9 @@ function providerFlagSubset(flags: Record<string, string | boolean>): Record<str return subset; } -function reviewFlagSubset(flags: Record<string, string | boolean>): Record<string, string | boolean> { +function reviewFlagSubset( + flags: Record<string, string | boolean>, +): Record<string, string | boolean> { const subset = providerFlagSubset(flags); for (const flag of ["since", "limit", "jobs"] as const) { const value = stringFlag(flags, flag); @@ -1374,7 +1378,11 @@ function reviewAnalysisSummary(findings: number, manifest: ReviewPromptManifest) function validatePrPatch(patch: PatchAttempt, force: boolean): void { if (patch.filesChanged.length === 0) { - throw new ClawpatchError(`patch has no changed files: ${patch.patchAttemptId}`, 2, "invalid-input"); + throw new ClawpatchError( + `patch has no changed files: ${patch.patchAttemptId}`, + 2, + "invalid-input", + ); } if (!["applied", "validated"].includes(patch.status) && !force) { throw new ClawpatchError( @@ -1518,7 +1526,9 @@ function plannedPrCommands( const commitFiles = stagePlan?.commitFiles ?? gitFiles; const addFiles = stagePlan?.addFiles ?? gitFiles; const updateFiles = stagePlan?.updateFiles ?? []; - commands.push(branchExists ? `git switch ${shellArg(branch)}` : `git switch -c ${shellArg(branch)}`); + commands.push( + branchExists ? `git switch ${shellArg(branch)}` : `git switch -c ${shellArg(branch)}`, + ); if (addFiles.length > 0) { commands.push(`git add -- ${shellPathspecArgs(addFiles)}`); } @@ -1688,7 +1698,10 @@ function isUsableRelativePrefix(path: string): boolean { return path.length > 0 && path !== "." && path !== ".." && !path.startsWith("../"); } -async function checkedRun(label: string, resultPromise: Promise<CommandResult>): Promise<CommandResult> { +async function checkedRun( + label: string, + resultPromise: Promise<CommandResult>, +): Promise<CommandResult> { const result = await resultPromise; if (result.exitCode !== 0) { throw new ClawpatchError( diff --git a/src/prompt.test.ts b/src/prompt.test.ts index daba65c..c078de4 100644 --- a/src/prompt.test.ts +++ b/src/prompt.test.ts @@ -49,9 +49,7 @@ describe("review prompt provenance", () => { const bundle = await buildReviewPromptBundle(root, project(root), feature(), defaultConfig()); expect(bundle.manifest.includedFiles).toEqual( - expect.arrayContaining([ - expect.objectContaining({ path: "docs/large.md", truncated: true }), - ]), + expect.arrayContaining([expect.objectContaining({ path: "docs/large.md", truncated: true })]), ); }); }); diff --git a/src/prompt.ts b/src/prompt.ts index 83ae0af..e5078a1 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -364,10 +364,9 @@ async function fileBlockWithManifest( } const bytes = Buffer.byteLength(contents, "utf8"); const truncated = contents.length > REVIEW_PROMPT_FILE_CHAR_LIMIT; - const trimmed = - truncated - ? `${contents.slice(0, REVIEW_PROMPT_FILE_CHAR_LIMIT)}\n...[truncated]` - : contents; + const trimmed = truncated + ? `${contents.slice(0, REVIEW_PROMPT_FILE_CHAR_LIMIT)}\n...[truncated]` + : contents; return { block: `--- ${path}\n${trimmed}`, manifest: { diff --git a/src/review-validation.test.ts b/src/review-validation.test.ts index 7b631b5..27db552 100644 --- a/src/review-validation.test.ts +++ b/src/review-validation.test.ts @@ -91,11 +91,7 @@ describe("validateReviewOutput", () => { it("rejects quotes that only match outside the cited line range", async () => { const root = await fixtureRoot("clawpatch-review-validation-line-quote-"); - await writeFixture( - root, - "src/index.ts", - "const first = 'TODO_BUG';\nconst second = 'safe';\n", - ); + await writeFixture(root, "src/index.ts", "const first = 'TODO_BUG';\nconst second = 'safe';\n"); await expect( validateReviewOutput( diff --git a/src/review-validation.ts b/src/review-validation.ts index ff45416..7566a70 100644 --- a/src/review-validation.ts +++ b/src/review-validation.ts @@ -12,7 +12,9 @@ export async function validateReviewOutput( output: ReviewOutput, ): Promise<ReviewOutput> { const included = includedReviewPaths(feature, config); - const promptFiles = new Map(manifest.includedFiles.map((file) => [normalizePath(file.path), file])); + const promptFiles = new Map( + manifest.includedFiles.map((file) => [normalizePath(file.path), file]), + ); const cache = new Map<string, Promise<string>>(); const findings = output.findings; for (const finding of findings) { @@ -132,10 +134,7 @@ function assertQuote( .slice(evidence.startLine - 1, evidence.endLine) .join("\n") : contents; - if ( - !target.includes(quote) && - !compactWhitespace(target).includes(compactWhitespace(quote)) - ) { + if (!target.includes(quote) && !compactWhitespace(target).includes(compactWhitespace(quote))) { throwMalformed(`evidence quote does not match file contents: ${evidence.path}`); } } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index a89755e..0f9d55d 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3422,9 +3422,7 @@ describe("workflow", () => { expect(preview).toMatchObject({ commands: expect.arrayContaining([ expect.stringContaining("git add -- ':(literal)packages/app/src/index.ts'"), - expect.stringContaining( - "gh pr create --base develop --head clawpatch/pat_open_pr_subdir", - ), + expect.stringContaining("gh pr create --base develop --head clawpatch/pat_open_pr_subdir"), expect.stringContaining("--draft"), ]), }); @@ -3913,11 +3911,7 @@ describe("workflow", () => { it("opens PRs for literal names that look like git pathspec magic", async () => { const root = await fixtureRoot("clawpatch-open-pr-literal-pathspec-"); - await writeFixture( - root, - "package.json", - JSON.stringify({ name: "open-pr-literal-pathspec" }), - ); + await writeFixture(root, "package.json", JSON.stringify({ name: "open-pr-literal-pathspec" })); await writeFixture(root, "README.md", "base\n"); await initGit(root); await checkCommand(root, "git add package.json README.md"); From 3a66c1c6af06d03c39249dacfa5a02a85eb1b218 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 16:01:52 +0800 Subject: [PATCH 24/25] fix(open-pr): keep retry pushes on recorded commits --- CHANGELOG.md | 2 ++ src/app.ts | 15 ++++++++++++--- src/workflow.test.ts | 17 +++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85cc377..af19da7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - Added review prompt provenance and budget accounting for included files, omitted files, prompt bytes, and approximate tokens. - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. - Fixed `clawpatch open-pr` so repositories without default-branch metadata use a dedicated patch branch and let GitHub choose the PR base. +- Fixed `clawpatch open-pr` retries to push the recorded patch commit instead of any later local branch tip. +- Fixed `clawpatch ci --since` empty-review output so it reports `reviewed: 0`. - Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi. - Fixed Bun package-manager detection to recognize the text `bun.lock` lockfile, thanks @austinm911. diff --git a/src/app.ts b/src/app.ts index dbcdaef..de66328 100644 --- a/src/app.ts +++ b/src/app.ts @@ -257,7 +257,7 @@ export async function ciCommand( return { initialized, mapped: numberField(mapped, "features"), - reviewed: numberField(reviewed, "reviewed"), + reviewed: numberField(reviewed, "reviewed") ?? 0, findings: reviewFindings, reportFindings: report.findings ?? 0, report: report.output ?? null, @@ -1117,6 +1117,7 @@ export async function openPrCommand( force, ); let commitSha = patch.git.commitSha; + const hadRecordedCommit = commitSha !== null; if (commitSha === null) { if (git.currentBranch !== branch) { const switchArgs = (await localBranchExists(git.root, branch)) @@ -1160,7 +1161,11 @@ export async function openPrCommand( prUrl: patch.git.prUrl, }); } - await checkedRun("git push", runCommandArgs("git", ["push", "-u", "origin", branch], git.root)); + commitSha = assertDefined(commitSha, "missing patch commit"); + const pushArgs = hadRecordedCommit + ? ["push", "origin", `${commitSha}:refs/heads/${branch}`] + : ["push", "-u", "origin", branch]; + await checkedRun("git push", runCommandArgs("git", pushArgs, git.root)); const ghArgs = prCreateArgs(base, branch, title, draft); const gh = await checkedRun("gh pr create", runCommandArgs(githubCli(), ghArgs, git.root, body)); const prUrl = firstUrl(gh.stdout) ?? gh.stdout.trim(); @@ -1537,7 +1542,11 @@ function plannedPrCommands( } commands.push(`git commit -m ${shellArg(title)} -- ${shellPathspecArgs(commitFiles)}`); } - commands.push(`git push -u origin ${shellArg(branch)}`); + commands.push( + patch.git.commitSha === null + ? `git push -u origin ${shellArg(branch)}` + : `git push origin ${shellArg(`${patch.git.commitSha}:refs/heads/${branch}`)}`, + ); commands.push(`gh ${prCreateArgs(base, branch, title, draft).map(shellArg).join(" ")}`); return commands; } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 0f9d55d..c747240 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -517,6 +517,7 @@ describe("workflow", () => { const summary = await readFile(summaryPath, "utf8"); expect(result).toMatchObject({ + reviewed: 0, findings: 0, reportFindings: 1, }); @@ -3721,6 +3722,16 @@ describe("workflow", () => { ); expect(afterFailure?.git.commitSha).toMatch(/^[a-f0-9]{40}$/u); expect(afterFailure?.git.branchName).toBe("clawpatch/pat_open_pr_retry"); + const recordedCommit = afterFailure?.git.commitSha; + if (recordedCommit === null || recordedCommit === undefined) { + throw new Error("missing recorded patch commit"); + } + + await writeFixture(root, "src/unrelated.ts", "export const unrelated = true;\n"); + await checkCommand(root, "git add src/unrelated.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "unrelated"'); + const advancedHead = (await runCommand("git rev-parse HEAD", root)).stdout.trim(); + expect(advancedHead).not.toBe(recordedCommit); process.env["CLAWPATCH_GH"] = successGh; await expect( @@ -3731,6 +3742,12 @@ describe("workflow", () => { ).resolves.toMatchObject({ pr: "https://github.com/openclaw/clawpatch/pull/999", }); + const remoteHead = ( + await runCommand("git ls-remote --heads origin clawpatch/pat_open_pr_retry", root) + ).stdout + .trim() + .split(/\s+/u)[0]; + expect(remoteHead).toBe(recordedCommit); } finally { if (previousGh === undefined) { delete process.env["CLAWPATCH_GH"]; From 4eb9a55d8f70d63eef09b52b3bf28b8f9eef3eb5 Mon Sep 17 00:00:00 2001 From: Vincent Koc <vincentkoc@ieee.org> Date: Mon, 18 May 2026 16:16:03 +0800 Subject: [PATCH 25/25] fix(open-pr): anchor PR branches to patch base --- CHANGELOG.md | 2 + src/app.ts | 34 +++++++++++++++-- src/exec.test.ts | 8 ++++ src/exec.ts | 25 ++++++++----- src/workflow.test.ts | 87 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af19da7..7e695b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. - Fixed `clawpatch open-pr` so repositories without default-branch metadata use a dedicated patch branch and let GitHub choose the PR base. - Fixed `clawpatch open-pr` retries to push the recorded patch commit instead of any later local branch tip. +- Fixed first-time `clawpatch open-pr` branch creation to start from the recorded patch base. +- Fixed command execution so providers that exit before reading stdin do not surface benign `EPIPE` errors. - Fixed `clawpatch ci --since` empty-review output so it reports `reviewed: 0`. - Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi. - Fixed Bun package-manager detection to recognize the text `bun.lock` lockfile, thanks @austinm911. diff --git a/src/app.ts b/src/app.ts index de66328..05be074 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1119,12 +1119,18 @@ export async function openPrCommand( let commitSha = patch.git.commitSha; const hadRecordedCommit = commitSha !== null; if (commitSha === null) { + const patchBaseSha = assertDefined(patch.git.baseSha, "missing patch base"); + const targetBranchExists = await localBranchExists(git.root, branch); + if (targetBranchExists) { + await assertRefAtPatchBase(git.root, branch, patch); + } if (git.currentBranch !== branch) { - const switchArgs = (await localBranchExists(git.root, branch)) + const switchArgs = targetBranchExists ? ["switch", branch] - : ["switch", "-c", branch]; + : ["switch", "-c", branch, patchBaseSha]; await checkedRun("git switch", runCommandArgs("git", switchArgs, git.root)); } + await assertRefAtPatchBase(git.root, "HEAD", patch); const stagePlan = await patchStagePlan(git.root, patchWorktree); if (stagePlan.addFiles.length > 0) { await checkedRun( @@ -1528,11 +1534,14 @@ function plannedPrCommands( ): string[] { const commands: string[] = []; if (patch.git.commitSha === null) { + const patchBaseSha = assertDefined(patch.git.baseSha, "missing patch base"); const commitFiles = stagePlan?.commitFiles ?? gitFiles; const addFiles = stagePlan?.addFiles ?? gitFiles; const updateFiles = stagePlan?.updateFiles ?? []; commands.push( - branchExists ? `git switch ${shellArg(branch)}` : `git switch -c ${shellArg(branch)}`, + branchExists + ? `git switch ${shellArg(branch)}` + : `git switch -c ${shellArg(branch)} ${shellArg(patchBaseSha)}`, ); if (addFiles.length > 0) { commands.push(`git add -- ${shellPathspecArgs(addFiles)}`); @@ -1735,6 +1744,25 @@ async function localBranchExists(gitRoot: string, branch: string): Promise<boole return result.exitCode === 0; } +async function assertRefAtPatchBase( + gitRoot: string, + ref: string, + patch: PatchAttempt, +): Promise<void> { + const head = await checkedRun( + "git rev-parse", + runCommandArgs("git", ["rev-parse", ref], gitRoot), + ); + const sha = head.stdout.trim(); + if (sha !== patch.git.baseSha) { + const message = [ + `patch attempt ${patch.patchAttemptId} was recorded from ${patch.git.baseSha},`, + `but ${ref} is ${sha}`, + ].join(" "); + throw new ClawpatchError(message, 2, "invalid-input"); + } +} + function firstUrl(output: string): string | null { return /https?:\/\/\S+/u.exec(output)?.[0] ?? null; } diff --git a/src/exec.test.ts b/src/exec.test.ts index f104a9e..f8270fc 100644 --- a/src/exec.test.ts +++ b/src/exec.test.ts @@ -31,6 +31,14 @@ describe("runCommandArgs", () => { expect(result.stderr).toContain("clawpatch-missing-executable-for-test"); }); + it("does not surface EPIPE when a child exits before reading stdin", async () => { + const dir = await mkdtemp(join(tmpdir(), "clawpatch-exec-stdin-")); + const input = "x".repeat(1_000_000); + const result = await runCommandArgs(process.execPath, ["-e", "process.exit(0)"], dir, input); + + expect(result.exitCode).toBe(0); + }); + it("terminates commands that exceed a timeout", async () => { const dir = await mkdtemp(join(tmpdir(), "clawpatch-exec-timeout-")); const script = join(dir, "hang.mjs"); diff --git a/src/exec.ts b/src/exec.ts index a370b4b..5e339af 100644 --- a/src/exec.ts +++ b/src/exec.ts @@ -46,11 +46,7 @@ export async function runCommandRaw( }); child.on("close", resolve); }); - if (input !== undefined) { - child.stdin.end(input); - } else { - child.stdin.end(); - } + endChildStdin(child, input); const exitCode = await exitCodePromise; if (spawnErrorMessage !== null) { stderr += stderr.length === 0 ? spawnErrorMessage : `\n${spawnErrorMessage}`; @@ -137,11 +133,7 @@ export async function runCommandArgs( }); }, options.timeoutMs); } - if (input !== undefined) { - child.stdin.end(input); - } else { - child.stdin.end(); - } + endChildStdin(child, input); const exitCode = await exitCodePromise; if (spawnErrorMessage !== null) { stderr += stderr.length === 0 ? spawnErrorMessage : `\n${spawnErrorMessage}`; @@ -229,6 +221,19 @@ function signalExitCode(signal: NodeJS.Signals): number { function noop(): void {} +function endChildStdin(child: ReturnType<typeof spawn>, input: string | undefined): void { + const stdin = child.stdin; + if (stdin === null) { + return; + } + stdin.on("error", noop); + if (input !== undefined) { + stdin.end(input); + } else { + stdin.end(); + } +} + function commandSpawnSpec( program: string, args: string[], diff --git a/src/workflow.test.ts b/src/workflow.test.ts index c747240..0f37312 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -3757,6 +3757,93 @@ describe("workflow", () => { } }); + it("creates first PR branches from the recorded patch base", async () => { + const root = await fixtureRoot("clawpatch-open-pr-recorded-base-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "open-pr-recorded-base", bin: { open: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + await initGit(root); + await checkCommand(root, "git add package.json src/index.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"'); + const baseSha = (await runCommand("git rev-parse HEAD", root)).stdout.trim(); + const origin = await fixtureRoot("clawpatch-open-pr-recorded-base-origin-"); + await checkCommand(root, `git init --bare -q ${origin}`); + await checkCommand(root, `git remote add origin ${origin}`); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + await initCommand(context, {}); + await writeFixture(root, "src/index.ts", "export const value = 'fixed';\n"); + await writeFixture(root, "src/unrelated.ts", "export const unrelated = true;\n"); + await checkCommand(root, "git add src/unrelated.ts"); + await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "unrelated"'); + const advancedHead = (await runCommand("git rev-parse HEAD", root)).stdout.trim(); + expect(advancedHead).not.toBe(baseSha); + const now = new Date().toISOString(); + await writePatchAttempt(paths, { + schemaVersion: 1, + patchAttemptId: "pat_open_pr_recorded_base", + findingIds: [], + featureIds: [], + status: "applied", + plan: "Replace the marker value.", + filesChanged: ["src/index.ts"], + commandsRun: [], + testResults: [ + { + command: "pnpm test", + cwd: root, + exitCode: 0, + durationMs: 1, + stdout: "", + stderr: "", + }, + ], + provider: null, + git: { + baseSha, + commitSha: null, + branchName: null, + prUrl: null, + }, + createdAt: now, + updatedAt: now, + }); + const ghScripts = await fixtureRoot("clawpatch-open-pr-recorded-base-gh-"); + const successGh = join(ghScripts, "success-gh.sh"); + await writeFixture( + ghScripts, + "success-gh.sh", + "#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1005\n", + ); + await chmod(successGh, 0o755); + const previousGh = process.env["CLAWPATCH_GH"]; + try { + process.env["CLAWPATCH_GH"] = successGh; + const opened = (await openPrCommand(context, { + patch: "pat_open_pr_recorded_base", + base: "main", + branch: "clawpatch/pat_open_pr_recorded_base", + })) as { commit: string; pr: string }; + const parent = ( + await runCommand(`git show -s --format=%P ${opened.commit}`, root) + ).stdout.trim(); + const committed = await runCommand(`git show --name-only --format= ${opened.commit}`, root); + + expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1005"); + expect(parent).toBe(baseSha); + expect(committed.stdout.trim().split("\n")).toEqual(["src/index.ts"]); + } finally { + if (previousGh === undefined) { + delete process.env["CLAWPATCH_GH"]; + } else { + process.env["CLAWPATCH_GH"] = previousGh; + } + } + }); + it("switches to an existing patch branch when opening a PR", async () => { const root = await fixtureRoot("clawpatch-open-pr-existing-branch-"); await writeFixture(