01a04284 - Publish rejected findings as a review, not a bare comment

[davidleomay]

EN:
A rejected gate published its findings as a plain issue comment whose body was whatever the reviewer typed, so the first real one reached an author as two findings run together in a paragraph, wrapped in STATUS=complete and a session id. They now post as a pull-request review of type COMMENT, which is where an author looks and which a runner can count as a review artefact. The contract also says what the evidence should look like, since specifying the mechanism and leaving the presentation unsaid is what produced that comment. COMMENT and never REQUEST_CHANGES: a bot that can hold a merge closed through branch protection is a different tool from one that reports.

DE:
Ein abgelehntes Gate veröffentlichte seine Befunde als gewöhnlichen Kommentar, dessen Text war, was der Reviewer tippte — der erste echte erreichte den Autor als zwei ineinanderlaufende Befunde, eingepackt in STATUS=complete und eine Session-ID. Sie erscheinen jetzt als Pull-Request-Review vom Typ COMMENT, dort schaut ein Autor hin und ein Runner kann es als Review-Artefakt zählen. Der Vertrag sagt jetzt auch, wie die Evidenz auszusehen hat, denn genau das Offenlassen der Darstellung hat jenen Kommentar erzeugt. COMMENT und nie REQUEST_CHANGES: ein Bot, der einen Merge über Branch Protection zuhalten kann, ist ein anderes Werkzeug als einer, der berichtet.

Details

What the author actually received, from the first unattended run of the path added in #32:

`codex-pr` / `quality` (codex) rejected at `3bb00508aefca39c3908f546583fcf8307d85443`

Complete Codex quality pass STATUS=complete. Rejected: update-fiat-output.dto.ts:91 bic
@IsOptional not @IsOptionalButNotNull (CONTRIBUTING Update DTO rule). fiat-output-bic.dto.spec.ts:74
optionality test covers omission only. Sibling Update fields already @IsOptional are inherited.
session=rollout-2026-08-27T09-02-07-01a04206-a92d-7e91-ae73-ed2633ff7eab

The mechanism worked and the delivery defeated it. Internal stage names, a 40-character SHA, two findings in one paragraph, and lane bookkeeping that means nothing outside the runner.

Change. A new review.post activity with an executor that submits POST /repos/{o}/{n}/pulls/{n}/reviews with event=COMMENT, de-duplicated by the same activity marker the comment path uses, so a retry finds its own review rather than posting a second. _queue_gate_findings emits it with a header a human can read — **Codex quality — rejected** at \3bb0050`— and the contract gains a paragraph on the evidence itself: one finding per line,file:linefirst, noSTATUS=` or session ids.

It also corrects a measurement. A runner asks whether a review happened by looking for a review artefact; issue comments are not one. The first unattended run therefore finished success with work_performed: no — the two fields disagreeing about the same job. Findings posted as a review make that field honest without weakening the check.

Verification. 442 passed, 1 skipped. Five behaviours reverted individually and each confirmed red: the activity type back to comment.post, the short SHA back to full, review.post removed from the executable types, event=COMMENT swapped for REQUEST_CHANGES, and the evidence paragraph deleted from the contract.

Three of those five started green and are the reason the run was worth doing. Asserting head[:7] in body passes for the full SHA too, since the long form contains the short one — it now also asserts the full SHA is absent. Nothing verified review.post was executable, though omitting it from EXECUTABLE_ACTIVITY_TYPES alone would mean findings are queued and never sent, silently. And the executor had no test at all, so the event=COMMENT mutation reported red only because no test matched — which reads identically to a real failure.

Two failures in tests/test_dashboard_control.py are unrelated and reproduce on a clean develop carrying none of this change; they depend on whether an agent daemon is installed on the machine running the suite.

[davidleomay]

EN:
Ready after 6 review passes at HEAD 9b0b4bf. A rejected gate published its findings as a plain comment whose body was whatever the reviewer typed, and the first unattended one reached an author as two findings run together in a paragraph wrapped in STATUS=complete and a session id. Findings now post as a pull-request review, a passing set can APPROVE, and REQUEST_CHANGES is refused in the executor. Findings per pass were 5, 2, 1, 1, 1, then 0.

DE:
Bereit nach 6 Review-Durchläufen auf HEAD 9b0b4bf. Ein abgelehntes Gate veröffentlichte seine Befunde als gewöhnlichen Kommentar, dessen Text war, was der Reviewer tippte — der erste unbeaufsichtigte erreichte den Autor als zwei ineinanderlaufende Befunde, eingepackt in STATUS=complete und eine Session-ID. Befunde erscheinen jetzt als Pull-Request-Review, ein bestandener Satz darf APPROVE setzen, und REQUEST_CHANGES weist der Executor ab. Befunde je Durchlauf: 5, 2, 1, 1, 1, dann 0.

Details

What an author actually received from the path added in #32, unattended:

`codex-pr` / `quality` (codex) rejected at `3bb00508aefca39c3908f546583fcf8307d85443`

Complete Codex quality pass STATUS=complete. Rejected: update-fiat-output.dto.ts:91 bic
@IsOptional not @IsOptionalButNotNull (CONTRIBUTING Update DTO rule). ... session=rollout-2026-08-27T09-02-07-01a04206-…

The mechanism worked and the delivery defeated it. Internal stage names, a 40-character SHA, two findings in one paragraph, and lane bookkeeping that means nothing outside the runner.

Change. A review.post activity and executor submitting POST /repos/{o}/{n}/pulls/{n}/reviews, de-duplicated by the same activity marker the comment path uses. event is COMMENT or APPROVE; REQUEST_CHANGES is refused in the executor rather than left to convention, because an account that can request changes can hold a merge closed through branch protection. The gate path cannot approve at all — a test asserts APPROVE appears nowhere in its source. The contract gains the state that justifies approving (four approved verdicts on this head, CI green) and a paragraph on the evidence itself: one finding per line, file:line first, no STATUS= or session ids.

It also corrects a measurement. A runner asks whether a review happened by looking for a review artefact; issue comments are not one. The first unattended run therefore finished success with work_performed: no — two fields disagreeing about one job. Findings posted as a review make that honest without weakening the check.

Six passes, eight findings, every one a defect in this change. Three were defects I had fixed in a sibling repository a day earlier and reintroduced by modelling the new executor on _run_comment_post, which carries them: no author check on the marker, an id copied without validation, and a comment claiming more than the code did. Two concerned the same line from opposite directions — the quality lane saw gh api user fetched on every row, the logic lane saw that a flaked lookup made every candidate fail the author check and silently posted a second review. One fix satisfied both: look up identity only once a marker matches, and treat a failed lookup as retryable rather than as "not ours".

Four of the eight were documentation claiming what the code does not do — README, CONTRIBUTING, DESIGN, the module docstring and scan_github's own docstring each enumerate the executable activity types, and each omitted review.post. Rather than correct a fifth instance, the test now reads the types out of the dispatch with inspect and fails when any executed type is undocumented; injecting a fifth branch turns it red, which the hard-coded list it replaced did not.

Verification. 452 passed, 1 skipped. Every behaviour reverted individually and confirmed red: the activity type, the short SHA, executability, event=COMMENT, the REQUEST_CHANGES refusal, the author check, id validation, lazy identity, the retry on identity failure, the case-insensitive comparison, the unaltered evidence, and each contract paragraph.

Three mutations started green and are why the run was worth doing. head[:7] in body passes for the full SHA too, since the long form contains the short one. Nothing verified review.post was in EXECUTABLE_ACTIVITY_TYPES, though omitting it alone means findings are queued and never sent, silently. And the event=COMMENT mutation first reported red only because no test matched at all — which reads identically to a real failure.

Two failures in tests/test_dashboard_control.py are unrelated and reproduce on a clean develop carrying none of this change; they depend on whether an agent daemon is installed on the machine running the suite.

Engines: Codex CLI for both dimensions under a personal ChatGPT Plus plan with model training disabled. One quality pass failed with a capacity error and was re-run rather than counted; a lane that errored is not a lane that approved.

No open pull-request comments, reviews, or review threads — verified via paginated queries at this head (0/0/0). Mergeable, no conflicts (mergeStateStatus: CLEAN). CI: pytest passed in 2m10s on 9b0b4bf.

[TaprootFreakAI]

EN:
Marked ready for review: the completion comment reports six approved review passes at 9b0b4bf, the branch merges cleanly onto the current develop, and the full suite passes on that merge (550 passed, 1 skipped).

DE:
Auf „ready for review“ gesetzt: der Abschlusskommentar meldet sechs approved Review-Durchläufe auf 9b0b4bf, der Branch merged sauber auf das aktuelle develop, und die volle Suite ist auf diesem Merge grün (550 passed, 1 skipped).