Skip to content
Merged
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
11 changes: 8 additions & 3 deletions actions/setup/js/codex_harness.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const { runProcess, formatDuration, sleep, MIN_POST_RESULT_WATCHDOG_TIMEOUT_MS,
const {
AWF_API_PROXY_REFLECT_URL,
AWF_REFLECT_OUTPUT_PATH,
AWF_REFLECT_TIMEOUT_MS,
AWF_MODELS_URL_TIMEOUT_MS,
GEMINI_MODEL_NAME_PREFIX,
enrichReflectModels,
Expand Down Expand Up @@ -536,7 +535,10 @@ async function main() {

// Fetch AWF API proxy reflection data before running the agent to capture initial proxy state.
// This is best-effort: failures are logged but do not affect the agent run.
await fetchAWFReflect({ logger: log });
// Skip when AWF_REFLECT_ENABLED is not "1" (e.g. no api-proxy running in sandbox or test mode).
if (process.env.AWF_REFLECT_ENABLED === "1") {
await fetchAWFReflect({ logger: log });
}
const codexHome = process.env.CODEX_HOME || "";
let codexEnv = codexChildEnv;
const providerConfig = configureCodexProviderFromReflect({
Expand Down Expand Up @@ -752,7 +754,10 @@ async function main() {
}

// Fetch AWF API proxy reflection data and persist to disk for post-run step summary.
await fetchAWFReflect({ logger: log });
// Skip when AWF_REFLECT_ENABLED is not "1" (e.g. no api-proxy running in sandbox or test mode).
if (process.env.AWF_REFLECT_ENABLED === "1") {
await fetchAWFReflect({ logger: log });
}

log(`done: exitCode=${lastExitCode} totalDuration=${formatDuration(Date.now() - driverStartTime)}`);
process.exit(lastExitCode);
Expand Down
3 changes: 2 additions & 1 deletion actions/setup/js/copilot_harness.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@
return parsed;
} catch (parseErr) {
const preview = serverArgsEnv.length > MAX_ENV_VAR_PREVIEW_LENGTH ? serverArgsEnv.slice(0, MAX_ENV_VAR_PREVIEW_LENGTH) + "…" : serverArgsEnv;
logger(`copilot-sdk driver mode: failed to parse GH_AW_COPILOT_SDK_SERVER_ARGS: ${parseErr} (value: ${preview})`);

Check warning on line 860 in actions/setup/js/copilot_harness.cjs

View workflow job for this annotation

GitHub Actions / lint-js

Directly interpolating caught error 'parseErr' in a template literal is unsafe — for Error objects it produces 'Error: message' (redundant prefix); for non-Error throws it produces '[object Object]'. Use ${getErrorMessage(parseErr)} if it is available, or ${String(parseErr)} as an import-free alternative
return [];
}
}
Expand Down Expand Up @@ -1272,7 +1272,8 @@
// only armed after hasTerminalSafeOutput is true, so watchdogFired on a no-stdio-output
// run means the agent completed its task (wrote safe-output) but produced no console
// output before the watchdog terminated the idle process.
if ((failureClass === "partial_execution" || failureClass === "long_run_exit" || (failureClass === "no_output" && result.watchdogFired)) && safeOutputsPath && hasTerminalSafeOutput(safeOutputsPath)) {
const isExpectedLateExit = failureClass === "partial_execution" || failureClass === "long_run_exit" || (failureClass === "no_output" && result.watchdogFired) || (failureClass === "authentication_failed" && result.watchdogFired);
if (isExpectedLateExit && safeOutputsPath && hasTerminalSafeOutput(safeOutputsPath)) {
const reason = result.watchdogFired ? "post-result watchdog fired after terminal safe-output was emitted" : "partial execution after terminal safe-output was already produced";
log(`attempt ${attempt + 1}: ${reason} — treating as success (late-activity exit suppressed)`);
lastExitCode = 0;
Expand Down
77 changes: 77 additions & 0 deletions actions/setup/js/copilot_harness.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2620,6 +2620,83 @@ setInterval(() => {}, 1000);`,
expect(result.stderr).toContain("post-result watchdog fired after terminal safe-output was emitted");
expect(result.stderr).toContain("late-activity exit suppressed");
});

it('exits 0 without retrying when watchdog fires after terminal safe-output was produced and output contains benign "not logged in" tool text', () => {
const tempDir = makeHarnessTempDir("copilot-watchdog-authentication-failed-suppression-");
const safeOutputsPath = path.join(tempDir, "safe-outputs.jsonl");
const stubPath = path.join(tempDir, "stub.cjs");
const promptPath = path.join(tempDir, "prompt.txt");
const callsPath = path.join(tempDir, "calls.jsonl");
fs.writeFileSync(
stubPath,
`const fs = require("fs");
const callsPath = process.env.COPILOT_HARNESS_STUB_CALLS;
const safeOutputsPath = process.env.GH_AW_SAFE_OUTPUTS;
fs.appendFileSync(callsPath, JSON.stringify({args: process.argv.slice(2)}) + "\\n");
fs.appendFileSync(safeOutputsPath, JSON.stringify({type:"add_comment",body:"Daily report posted"}) + "\\n");
process.stdout.write(JSON.stringify({
type: "tool.execution_complete",
tool: "bash",
output: "You are not logged into any GitHub hosts. To log in, run: gh auth login"
}) + "\\n");
process.on("SIGTERM", () => process.exit(1));
setInterval(() => {}, 1000);`,
"utf8"
);
fs.writeFileSync(promptPath, "generate the report", "utf8");

const result = spawnSync(process.execPath, ["copilot_harness.cjs", process.execPath, stubPath, "--prompt-file", promptPath], {
cwd: path.dirname(require.resolve("./copilot_harness.cjs")),
env: {
...process.env,
COPILOT_HARNESS_STUB_CALLS: callsPath,
GH_AW_SAFE_OUTPUTS: safeOutputsPath,
GH_AW_HARNESS_WATCHDOG_TIMEOUT_MS: "100",
},
encoding: "utf8",
timeout: 15000,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The test verifies the happy path well, but does not assert the negative case: that a genuine authentication_failed (no terminal safe-output yet) is not rescued. Without that guard, a future regression could silently swallow real auth failures.

💡 Suggested companion test

Add a sibling test where safeOutputsPath points to an empty file (no terminal safe-output entry), the watchdog fires, and the harness exits non-zero (or retries). The structure mirrors this test but omits the add_comment write to the safe-outputs file before the watchdog fires.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the negative test: "does not rescue authentication_failed when no terminal safe-output was produced before the watchdog fires". The stub emits "Error: No authentication information found." and exits 1 without writing any safe-output entry; the harness exits non-zero and the late-activity exit suppressed log line is absent.

});
const callCount = fs.readFileSync(callsPath, "utf8").trim().split("\n").filter(Boolean).length;
expect(callCount).toBe(1);
expect(result.status).toBe(0);
expect(result.stderr).toContain("post-result watchdog fired after terminal safe-output was emitted");
expect(result.stderr).toContain("late-activity exit suppressed");
});

it("does not rescue authentication_failed when no terminal safe-output was produced before the watchdog fires", () => {
const tempDir = makeHarnessTempDir("copilot-watchdog-auth-failed-no-output-");
const safeOutputsPath = path.join(tempDir, "safe-outputs.jsonl");
const stubPath = path.join(tempDir, "stub.cjs");
const promptPath = path.join(tempDir, "prompt.txt");
const callsPath = path.join(tempDir, "calls.jsonl");
// Stub emits auth-failure-looking text but does NOT write any safe-output entry.
// Without terminal safe-output the watchdog never arms, so authentication_failed
// falls through to the normal non-retryable failure path and exits non-zero.
fs.writeFileSync(
stubPath,
`const fs = require("fs");
const callsPath = process.env.COPILOT_HARNESS_STUB_CALLS;
fs.appendFileSync(callsPath, JSON.stringify({args: process.argv.slice(2)}) + "\\n");
process.stdout.write("Error: No authentication information found.\\n");
process.exit(1);`,
"utf8"
);
fs.writeFileSync(promptPath, "generate the report", "utf8");

const result = spawnSync(process.execPath, ["copilot_harness.cjs", process.execPath, stubPath, "--prompt-file", promptPath], {
cwd: path.dirname(require.resolve("./copilot_harness.cjs")),
env: {
...process.env,
COPILOT_HARNESS_STUB_CALLS: callsPath,
GH_AW_SAFE_OUTPUTS: safeOutputsPath,
},
encoding: "utf8",
timeout: 15000,
});
// Harness exits non-zero: genuine auth failure with no terminal safe-output is not rescued
expect(result.status).not.toBe(0);
expect(result.stderr).not.toContain("late-activity exit suppressed");
});
});

describe("AI credits budget enforcement exits 0", () => {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/pr_code_quality_reviewer_workflow_contract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ func TestPRCodeQualityReviewerWorkflowSubAgentModelContract(t *testing.T) {

text := string(content)
assert.Contains(t, text, "## agent: `grumpy-coder`", "Workflow should define the grumpy-coder sub-agent")
assert.Contains(t, text, "model: claude-haiku-4.5", "Sub-agent should pin a supported Haiku model")
assert.Contains(t, text, "model: small", "Sub-agent should use the portable small alias")
assert.NotContains(t, text, "model: inherited", "Sub-agent should not inherit an unsupported tier-specific model")
}
Loading