aggregator: an empty github-result report marks a dashboard comment failed — a pending comment is written off unposted, and a posted comment flips back to failed #1686

Summary

A GitHub-result report that carries neither a URL nor an error is recorded as a failure — and it can retroactively un-post a comment that already succeeded.

CommentsStore.markGithubResult (pm-stack/aggregator/src/comments.ts:202-214, sha256 3b0099d5):

if (result.url) {
  c.githubState = "posted";
  c.githubUrl = result.url;
} else {
  c.githubState = "failed";
}
await this.persist();

The else treats "no URL" as "failed", with no check that an error was actually reported, and no guard on the comment's current state. The HTTP boundary does not close the hole: POST /api/comments/github-result (src/server.ts:350-358) validates only that id is a string, then forwards url/error as undefined when absent — so a body of {"id": "..."} reaches the store as an empty result.

Verified behaviour (probe run on the laptop, main d39e611)

Probe test against the real CommentsStore (bun test, detached worktree, temp-dir store file):

• CTRL — a {url} report moves a pending comment to posted; an {error} report on a fresh comment moves it to failed. ✅
• BUG-A — markGithubResult(id, {}) on a pending comment returns it with githubState: "failed" and persists that. Per the consumption contract in the comments.ts header, a kind-github comment "sits at githubState pending until a fleet poster posts it … then reports the result: { id, url } or { id, error }" — a report with neither field reports no outcome, yet the comment is written off as failed and will never be posted.
• BUG-B — after a successful {url} report (posted, githubUrl set), a later empty report flips the same comment back to failed (the stale githubUrl remains set, so state and URL now contradict each other; githubCommentUrl still renders the URL of a "failed" comment).

Existing tests (test/slice3.test.ts:149-154, :367-368) cover pending→posted and the happy-path endpoint, but no empty, conflicting, or repeat report.

Why it matters

Dashboard comments of kind github are Drew's route for getting a comment onto an issue/PR via a fleet poster. A stray, truncated, or retried result report can silently mark an unposted comment as failed — the message never reaches GitHub and the dashboard says it was tried — or flip a genuinely posted comment to "failed", sending a poster to re-post a duplicate. The store is the system of record for this hand-off; it should refuse a result that reports nothing and never move a terminal state backwards.

Acceptance criteria

• A result report with neither url nor error is rejected (store returns an error / endpoint answers 400) and the comment's state is unchanged — pending stays pending.
• A comment already posted cannot be moved to failed by a later report (and a failed comment's path back, if any, is an explicit decision in the fix, pinned by a test); repeat identical reports are idempotent.
• A report carrying both url and error is rejected or resolved by a documented rule, pinned by a test — not silently decided by branch order.
• Regression tests cover: empty report on pending, empty report on posted, url+error report; the existing slice3 comment tests keep passing.

Scope notes

• Two candidate homes for the validation: the store (defends every caller) and/or the server.ts endpoint (400 for the HTTP caller). The store-level refusal is the load-bearing one; triage pins whether the endpoint also validates.
• Out of scope: the load path's parse-failure handling in the same file, comment author validation, and the triage endpoint.