Skip to content

Commit 658e542

Browse files
fix: surface hosted review runtime failures (#11)
1 parent 977254a commit 658e542

2 files changed

Lines changed: 89 additions & 3 deletions

File tree

src/adapter.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export interface AdapterResponse {
5151
body: JsonObject;
5252
}
5353

54-
interface CommandResult extends JsonObject {
54+
export interface CommandResult extends JsonObject {
5555
args: string[];
5656
returncode: number;
5757
stdout: string;
@@ -179,7 +179,7 @@ function createFreshChildDirectory(parent: string, name: string, label: string):
179179
return path;
180180
}
181181

182-
function createFreshTaskAttemptDirectory(root: string, taskId: string, attempt: number, label: string): string {
182+
export function createFreshTaskAttemptDirectory(root: string, taskId: string, attempt: number, label: string): string {
183183
if (!validRecordId(taskId)) throw new Error(`Invalid task ID for ${label}`);
184184
if (!Number.isSafeInteger(attempt) || attempt <= 0) throw new Error(`Invalid attempt number for ${label}`);
185185
const taskRoot = ensureManagedChildDirectory(root, taskId, `${label} task directory`);
@@ -1613,6 +1613,7 @@ async function runTaskUnlocked(config: AdapterConfig, taskId: string): Promise<J
16131613
task.session_brief_path = firstCycle.brief_path;
16141614
task.session_brief_sha256 = fileSha256(String(firstCycle.brief_path));
16151615
task.runtime_exit_code = firstCycle.run.returncode;
1616+
task.runtime_diagnostic = runtimeDiagnostic(firstCycle.run) || undefined;
16161617
task.result_path = firstCycle.result_path;
16171618
writeJsonAtomic(path, task);
16181619

@@ -1641,6 +1642,7 @@ async function runTaskUnlocked(config: AdapterConfig, taskId: string): Promise<J
16411642
});
16421643
task.review_fix_loops = loopRecords;
16431644
task.runtime_exit_code = repairCycle.run.returncode;
1645+
task.runtime_diagnostic = runtimeDiagnostic(repairCycle.run) || undefined;
16441646
task.result_path = repairCycle.result_path;
16451647
task.updated_at = utcNow();
16461648
writeJsonAtomic(path, task);
@@ -3588,6 +3590,10 @@ function publicationCommentBody(task: JsonObject, result: JsonObject, heading =
35883590
if (prBody.trim() && prBody.trim() !== summary.trim()) {
35893591
parts.push("", prBody.trim());
35903592
}
3593+
const diagnostic = safePublicationText(String(task.runtime_diagnostic || ""), 1000).trim();
3594+
if (diagnostic && result.status !== "success") {
3595+
parts.push("", "### Runtime diagnostic", diagnostic);
3596+
}
35913597
parts.push(...reviewFixLoopLines(task), "", "### Evidence");
35923598
if (Object.keys(evidence).length) {
35933599
const changedFiles = Array.isArray(evidence.changed_files) ? evidence.changed_files : [];
@@ -3741,13 +3747,37 @@ function codexTokenCandidates(config: AdapterConfig): string[] {
37413747
}
37423748

37433749
function redactedCommandResult(result: CommandResult): JsonObject {
3750+
const diagnostic = runtimeDiagnostic(result);
37443751
return {
37453752
...result,
37463753
stdout: redactTokenish(result.stdout),
37473754
stderr: redactTokenish(result.stderr),
3755+
...(diagnostic ? {runtime_diagnostic: diagnostic} : {}),
37483756
};
37493757
}
37503758

3759+
export function runtimeDiagnostic(result: CommandResult): string | null {
3760+
if (result.returncode === 0 && !result.signal && !result.timed_out && !result.spawn_error) {
3761+
return null;
3762+
}
3763+
if (result.timed_out) {
3764+
return "Coven Code timed out before completing the hosted review.";
3765+
}
3766+
if (result.signal) {
3767+
return `Coven Code stopped after signal ${safePublicationText(String(result.signal), 40)}.`;
3768+
}
3769+
const raw = String(result.stderr || result.spawn_error || "").trim();
3770+
if (!raw) {
3771+
return `Coven Code exited ${result.returncode} without a diagnostic.`;
3772+
}
3773+
const safe = redactTokenish(raw)
3774+
.replace(/\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}\b/gi, "[redacted email]")
3775+
.replace(/\bon account\s+[^\n.]+/gi, "on the configured account")
3776+
.replace(/\s+/g, " ")
3777+
.trim();
3778+
return safePublicationText(safe, 1000);
3779+
}
3780+
37513781
export function sanitizedRuntimeEnvironment(source: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
37523782
const env: NodeJS.ProcessEnv = {};
37533783
for (const key of [
@@ -3782,6 +3812,8 @@ export function redactTokenish(text: string): string {
37823812
.replace(/\b(?:gh[pousr]_|github_pat_)[A-Za-z0-9_-]{6,}/g, "[redacted github token]")
37833813
.replace(/\bsk-(?:proj-)?[A-Za-z0-9_-]{8,}/g, "[redacted OpenAI token]")
37843814
.replace(/\bBearer\s+[^\s'\"]+/gi, "Bearer [redacted]")
3815+
.replace(/\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}\b/gi, "[redacted email]")
3816+
.replace(/\bon account\s+`?[^`\n.]+`?/gi, "on the configured account")
37853817
.replace(/\beyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\b/g, "[redacted JWT]")
37863818
.replace(/x-access-token:[^@\s'\"]+/gi, "x-access-token:[redacted]")
37873819
.replace(/(https?:\/\/)[^/\s:@]+:[^@\s/]+@/gi, "$1[redacted]@");

tests/webhook-adapter.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { pathToFileURL } from "node:url";
1010

1111
import {
1212
buildTaskFromEvent,
13+
createFreshTaskAttemptDirectory,
1314
createConfig,
1415
githubRequestAllPages,
1516
handleRequest,
@@ -24,6 +25,7 @@ import {
2425
resumeTaskPublication,
2526
runtimeInstallationTokenRequest,
2627
runtimeIsolationIssue,
28+
runtimeDiagnostic,
2729
runtimeProcessEnvironment,
2830
runtimeSandboxArgs,
2931
runnableTaskIds,
@@ -2893,11 +2895,12 @@ test("redacts credentials and passes only allowlisted ambient environment keys",
28932895
const secretText = [
28942896
"ghs_1234567890", "ghp_1234567890", "github_pat_1234567890",
28952897
"sk-proj-1234567890", "Bearer topsecret", "eyJabc.def.ghi",
2898+
"API error 429 on account `reviewer (reviewer@example.com)`.",
28962899
"https://user:password@example.com/path",
28972900
"-----BEGIN PRIVATE KEY-----\nprivate-data\n-----END PRIVATE KEY-----",
28982901
].join("\n");
28992902
const redacted = redactTokenish(secretText);
2900-
assert.doesNotMatch(redacted, /1234567890|topsecret|password|private-data|eyJabc/);
2903+
assert.doesNotMatch(redacted, /1234567890|topsecret|password|private-data|eyJabc|reviewer@example\.com|reviewer \(/);
29012904
const env = sanitizedRuntimeEnvironment({
29022905
PATH: "/bin", LANG: "C.UTF-8", SSH_AUTH_SOCK: "/tmp/agent.sock",
29032906
DATABASE_URL: "postgres://user:pass@db", AWS_ACCESS_KEY_ID: "AKIASECRET",
@@ -2906,6 +2909,57 @@ test("redacts credentials and passes only allowlisted ambient environment keys",
29062909
assert.deepEqual(env, {PATH: "/bin", LANG: "C.UTF-8"});
29072910
});
29082911

2912+
test("preserves a useful redacted provider diagnostic for failed reviews", async () => {
2913+
const diagnostic = runtimeDiagnostic({
2914+
args: ["coven-code", "--headless"],
2915+
returncode: 2,
2916+
stdout: "",
2917+
stderr: "API error 429: rate limit for model `gpt-5.4-mini` on account `reviewer (reviewer@example.com)`. Bearer topsecret sk-proj-1234567890\nProvider: codex",
2918+
signal: null,
2919+
timed_out: false,
2920+
spawn_error: "",
2921+
});
2922+
assert.match(String(diagnostic), /API error 429/);
2923+
assert.match(String(diagnostic), /gpt-5\.4-mini/);
2924+
assert.match(String(diagnostic), /configured account/);
2925+
assert.doesNotMatch(String(diagnostic), /reviewer@example\.com|reviewer \(|topsecret|1234567890/);
2926+
2927+
const stateDir = tempStateDir();
2928+
const config = testConfig(stateDir);
2929+
const task = reviewTask("provider-failure-diagnostic");
2930+
task.runtime_diagnostic = diagnostic;
2931+
prepareReviewWorkspace(config, task);
2932+
const result = completeReview();
2933+
result.status = "failure";
2934+
const resultPath = join(stateDir, "provider-failure.json");
2935+
writeFileSync(resultPath, JSON.stringify(result));
2936+
let payload: JsonObject = {};
2937+
await withGithubApiMock((url, init) => {
2938+
const read = githubReadFixture(url, init);
2939+
if (read) return read;
2940+
payload = JSON.parse(String(init.body)) as JsonObject;
2941+
return {id: 409, state: "PENDING", html_url: "https://github.com/OpenCoven/example/pull/7#pullrequestreview-409"};
2942+
}, async () => publishResultIfConfigured(config, task, resultPath, "token"));
2943+
assert.equal(payload.event, "COMMENT");
2944+
assert.match(String(payload.body), /### Runtime diagnostic/);
2945+
assert.match(String(payload.body), /API error 429/);
2946+
assert.doesNotMatch(String(payload.body), /reviewer@example\.com|reviewer \(|topsecret|1234567890/);
2947+
});
2948+
2949+
test("creates isolated attempt directories and refuses stale artifact reuse", () => {
2950+
const root = tempStateDir();
2951+
const first = createFreshTaskAttemptDirectory(root, "review-task", 1, "test attempt");
2952+
writeFileSync(join(first, "run.json"), "stale artifact\n");
2953+
const second = createFreshTaskAttemptDirectory(root, "review-task", 2, "test attempt");
2954+
assert.notEqual(first, second);
2955+
assert.equal(readFileSync(join(first, "run.json"), "utf8"), "stale artifact\n");
2956+
assert.deepEqual(readdirSync(second), []);
2957+
assert.throws(
2958+
() => createFreshTaskAttemptDirectory(root, "review-task", 1, "test attempt"),
2959+
/already exists/,
2960+
);
2961+
});
2962+
29092963
test("keeps idempotency marker after truncating and redacts issue publication text", async () => {
29102964
const stateDir = tempStateDir();
29112965
const task: JsonObject = {

0 commit comments

Comments
 (0)