Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdk/typescript/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1690,6 +1690,8 @@ its read-only Codex sandbox.
to select findings and add patch instructions. Results include a `patches`
entry per finding with status `verified`, `no_change`, `blocked`, or `failed`.
Verified and already-fixed findings no longer fail `--fail-on-severity`.
Patching shows each finding's position, elapsed time, and live Codex activity.
Progress goes to stderr; completed results stay in the terminal history.

`--create-pr` commits generated patch files and opens a draft GitHub pull request
with `gh` or a draft GitLab merge request with `glab`. Install and authenticate
Expand Down
51 changes: 40 additions & 11 deletions sdk/typescript/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -776,11 +776,12 @@ class PublicationProgressPresenter {
}
}

class VerificationProgressPresenter {
class FindingProgressPresenter {
readonly #stream: Writable;
readonly #dependencies: CliDependencies;
readonly #repository: string;
readonly #total: number;
readonly #progress: Progress;
readonly #seenActivities = new Set<string>();
readonly #reasoning = new Map<string, string>();
#dashboard: ScanDashboard | null = null;
Expand All @@ -790,19 +791,23 @@ class VerificationProgressPresenter {
dependencies: CliDependencies,
repository: string,
total: number,
interactive = true,
) {
this.#stream = stream;
this.#dependencies = dependencies;
this.#repository = repository;
this.#total = total;
this.#progress = new Progress(
stream,
dependencies,
interactive &&
dependencies.environment["CI"] === undefined &&
dependencies.environment["TERM"] !== "dumb",
);
}

public start(): void {
if (
this.#stream.isTTY === true &&
this.#dependencies.environment["CI"] === undefined &&
this.#dependencies.environment["TERM"] !== "dumb"
) {
public startVerification(): void {
if (this.#progress.interactive) {
const dashboard = new ScanDashboard(this.#stream, {
repository: this.#repository,
presentation: "verification",
Expand All @@ -828,6 +833,16 @@ class VerificationProgressPresenter {
);
}

public startPatch(finding: Finding, index: number): void {
try {
this.#progress.startTimer(
`Patching ${index + 1}/${this.#total} · ${safePatchText(finding.title)}`,
);
} catch {
this.stop();
}
}

public observe(event: Readonly<Record<string, unknown>>): void {
const method = event["method"];
const params = event["params"];
Expand Down Expand Up @@ -918,14 +933,17 @@ class VerificationProgressPresenter {

public stop(): void {
try {
this.#progress.stopTimer();
this.#dashboard?.stop();
} catch {}
this.#dashboard = null;
}

#write(message: string): void {
try {
this.#stream.write(`${safePatchText(message)}\n`);
this.#progress.writeAboveTimer(() => {
this.#stream.write(`${safePatchText(message)}\n`);
});
} catch {}
}
}
Expand Down Expand Up @@ -4800,13 +4818,13 @@ export async function main(
return true;
},
};
const progress = new VerificationProgressPresenter(
const progress = new FindingProgressPresenter(
errorOutput,
dependencies,
repository,
identifiers.length,
);
progress.start();
progress.startVerification();
try {
exitCode = await runSkill(
"verify-fix",
Expand Down Expand Up @@ -7039,6 +7057,7 @@ async function runFindingPatches(
stderr: Writable,
dependencies: CliDependencies,
options: Omit<SkillRunOptions, "directory" | "findings"> = {},
interactive = true,
): Promise<FindingPatch[]> {
if (selected.findings.length === 0) {
stderr.write("No matching open findings to patch.\n");
Expand All @@ -7059,6 +7078,14 @@ async function runFindingPatches(
},
};
const instruction = options.findingInstructions?.[finding.occurrenceId];
const progress = new FindingProgressPresenter(
stderr,
dependencies,
selected.repository,
selected.findings.length,
interactive,
);
progress.startPatch(finding, patches.length);
const status = await runSkill(
"fix-finding",
[],
Expand All @@ -7074,8 +7101,9 @@ async function runFindingPatches(
findingInstructions: instruction?.trim()
? { [finding.occurrenceId]: instruction }
: undefined,
onEvent: progress.observe.bind(progress),
},
);
).finally(() => progress.stop());
if (status === 130 || status === 143) {
throw new CodexSecurityError("Patch operation was interrupted.");
}
Expand Down Expand Up @@ -8714,6 +8742,7 @@ async function executeScan(
auth,
findingInstructions: patchSelection?.instructions,
},
progress?.interactive === true,
);
scanData = { ...scanData, patchSeverity: patchThreshold, patches };
if (
Expand Down
124 changes: 124 additions & 0 deletions sdk/typescript/tests-ts/cli-patch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,130 @@ describe("scan and patch workflow", () => {
}
},
);
test("shows each patch and live activity before it finishes, with clean JSON output", async () => {
for (const args of [
["scan", "--patch", "--patch-severity", "high"],
["patch", "--scan", "scan-1", "--json"],
]) {
const result = resultWithFindings(["high", "high"]);
const stdout = capture();
const stderr = capture(true);
let now = 0;
let index = 0;
const timers = new Map<NodeJS.Timeout, () => void>();
const current = dependencies({
result,
onWorkbench: () => savedScan(result),
onCodex: (args, output) => {
index += 1;
const label = `Patching ${index}/2 · Finding ${index}`;
expect(stderr.text()).toContain(label);
expect(stderr.text()).not.toContain(`VERIFIED Finding ${index}`);
output!.appServer!.onEvent!({
method: "item/reasoning/summaryTextDelta",
params: { itemId: "reasoning-1", delta: "Checking the fix." },
});
expect(stderr.text()).toContain("Codex: Checking the fix.");
now += 84_000;
for (const tick of [...timers.values()]) tick();
expect(stderr.text()).toContain(`[01:24] ${label}`);
completePatches(args, output);
return 0;
},
});
current.now = () => now;
current.setInterval = (callback) => {
const timer = {} as NodeJS.Timeout;
timers.set(timer, callback);
return timer;
};
current.clearInterval = (timer) => {
timers.delete(timer);
};

expect(
await main(args, stdout.stream, stderr.stream, current),
stderr.text(),
).toBe(0);
expect(index).toBe(2);
expect(timers.size).toBe(0);
const progress = stderr.text();
expect(progress.indexOf("VERIFIED Finding 1")).toBeLessThan(
progress.indexOf("Patching 2/2"),
);
expect(progress).toContain("VERIFIED Finding 2");
expect(progress.match(/Codex: Checking the fix\./gu)).toHaveLength(2);
if (args.includes("--json")) {
expect(JSON.parse(stdout.text()).patches).toMatchObject([
{ occurrenceId: "occ_1", status: "verified" },
{ occurrenceId: "occ_2", status: "verified" },
]);
}
expect(stdout.text()).not.toContain("\u001B");
}
});

test("uses plain patch progress for noninteractive runs", async () => {
const saved = ["patch", "--scan", "scan-1", "--json"];
for (const [interactive, environment, args] of [
[false, {}, saved],
[true, { CI: "1" }, saved],
[true, { TERM: "dumb" }, saved],
[true, {}, ["scan", "--patch", "--headless"]],
[true, {}, ["scan", "--patch", "--json"]],
] as const) {
const result = resultWithFindings(["high"]);
const outcome = await runWorkflow(
[...args],
{
result,
environment,
onWorkbench: () => savedScan(result),
},
{ interactive },
);
expect(outcome.exitCode).toBe(0);
expect(outcome.stderr).toContain("Patching 1/1 · Finding 1");
expect(outcome.stderr).not.toContain("\u001B");
}
});

test("stops patch progress on interruption or an agent error", async () => {
for (const status of [130, "error"] as const) {
const result = resultWithFindings(["high", "high"]);
let timers = 0;
const outcome = await runWorkflow(
["patch", "--scan", "scan-1", "--json"],
{
result,
onWorkbench: () => savedScan(result),
onCodex: () => {
if (status === "error") throw new Error("Agent failed");
return status;
},
},
{
interactive: true,
configure: (current) => {
current.setInterval = () => {
timers += 1;
return {} as NodeJS.Timeout;
};
current.clearInterval = () => {
timers -= 1;
};
},
},
);
expect(outcome.exitCode).not.toBe(0);
expect(timers).toBe(0);
expect(outcome.stderr).toContain("\u001B[?25h");
expect(outcome.stderr).not.toContain("Patching 2/2");
expect(outcome.stderr).toContain(
status === "error" ? "Agent failed" : "Patch operation was interrupted",
);
}
});

test("assesses patch risk only when the patch flag is selected", async () => {
for (const enabled of [false, true]) {
Expand Down