A failed PR comment fails the gate: fork PRs exit 2 on a passing eval

[vsolano9]

Summary

On a pull request from a fork, the github.token is read-only, so POST /issues/{n}/comments returns 403. upsertComment throws, the error escapes to the top-level handler in src/cli/index.ts, and the process exits 2 — even when the comparison found no regression.

The action defaults to comment: "true" and github-token: ${{ github.token }}, so out of the box the gate fails on every fork PR. Since fork PRs are how outside contributions arrive, the check is red for exactly the contributors it is supposed to help.

The comment is a reporting side-effect. It should not be able to overturn the verdict.

Where it happens

src/cli/index.ts (cmdCompare):

if (boolFlag(args, "comment")) {
  const ctx = contextFromEnv(process.env);
  if (!ctx) {
    console.error("[evalgate] --comment set but no GitHub PR context found; skipping.");
  } else {
    await upsertComment(ctx, md);   // throws on 403 -> main().catch -> process.exit(2)
    console.log(...);
  }
}

return cmp.regressed && !boolFlag(args, "no-fail") ? 1 : 0;   // never reached

Note the missing-context branch already treats "cannot comment" as a warning and continues. A 403 is the same situation, arriving as an exception instead of a null.

Evidence from this repo's own CI

quality-gate on #22 and #23 is red on a green eval:

evalgate - baseline comparison
base 94.2% -> head 94.2% (0.0pp)

No regressions beyond tolerance.
[evalgate] failed to create comment (403)
##[error]Process completed with exit code 2.

(https://github.com/AgentPostmortem/Evalgate/actions/runs/32645906897/job/97210017077)

Reproduction (offline, no network, mock provider)

Point the CLI at a local stand-in for the API that answers GET with [] and 403s the POST, exactly as a fork's token behaves:

// repro.mjs — node repro.mjs, after npm run build
import { createServer } from "node:http";
import { spawn } from "node:child_process";

const srv = createServer((req, res) => {
  if (req.method === "GET") { res.writeHead(200, {"content-type":"application/json"}); res.end("[]"); return; }
  res.writeHead(403, {"content-type":"application/json"});
  res.end(JSON.stringify({ message: "Resource not accessible by integration" }));
});
srv.listen(0, "127.0.0.1", () => {
  const port = srv.address().port;
  const child = spawn(process.execPath, [
    "dist/cli/index.js", "compare", "examples/support-agent.eval.yaml",
    "--base", "examples/support-agent.baseline.json",
    "--provider", "mock", "--tolerance", "0.01", "--comment",
  ], { env: { ...process.env,
      GITHUB_TOKEN: "fake", GITHUB_REPOSITORY: "AgentPostmortem/Evalgate",
      EVALGATE_PR: "23", GITHUB_API_URL: `http://127.0.0.1:${port}` },
    stdio: "inherit" });
  child.on("exit", (code) => { console.log(`\n>>> evalgate exit code: ${code}`); srv.close(); });
});

Result on main (a42bd9a), Node 26.7.0:

No regressions beyond tolerance.
[evalgate] failed to create comment (403)

>>> evalgate exit code: 2

Expected 0. Adding --no-fail does not help — it still exits 2, because the throw happens before the return that --no-fail guards. There is currently no flag that lets a passing suite survive a failed comment post.

Suggested fix

Wrap the upsert so a comment failure warns and leaves the exit code to the comparison:

try {
  await upsertComment(ctx, md);
  console.log(`[evalgate] posted report to ${ctx.owner}/${ctx.repo}#${ctx.prNumber}`);
} catch (err) {
  console.error(`[evalgate] could not post the PR comment: ${(err as Error).message}`);
  if (/\((403|404)\)/.test((err as Error).message)) {
    console.error("[evalgate] a pull request from a fork gets a read-only token; the report above and any --md artifact still stand.");
  }
}

--md is written before this block, so the report artifact survives untouched either way.

Two things worth deciding, and I did not want to presume:

1. Whether any comment failure should be non-fatal, or only 403/404. Non-fatal for all of them is the simpler contract and matches the existing missing-context branch; a maintainer may prefer a real network error to stay loud.
2. Whether the action should additionally skip --comment when github.event.pull_request.head.repo.fork is true, so the fork case never produces a warning at all. That is complementary rather than an alternative — the CLI should still not exit 2 when someone runs it by hand.

I am at the two-claim limit right now (#8 and #17), so I am filing this rather than claiming it. Happy to open the PR the moment one of those lands, or leave it for someone else — just say which you prefer.