[TheGreatAxios]

When code review finds problems at or above the policy's clean.below (any finding when it is unset), a pull request its rules would pass now fails instead of passing with approve and merge held. The reason says how many problems the review found, the comment becomes a Fail that keeps what the earlier comment said, including a judge's words, and asks the author to fix the review's problems, the labels are the Fail ones, the policy's fail actions run, and the head is not approvable when the policy approves, so an approval given to an earlier head is taken back.

A verdict that is Unsure, including one waiting on CI or one the decision model could not make, or already Fail keeps its verdict. It gains the review as a failed check, its reason, reasons and next step name the review's problems too, and approve and merge stay held. Whatever the verdict, its comment asks the author to fix the review's problems, so a verdict that posted no comment now posts one.

The review's problems count even while another check the policy turns on can't run yet. A degraded verdict that names a head is posted with the review's problems like any other; one that names no head is still not posted, and a take that names no head keeps the verdict as the rules made it with approve and merge held. An Unsure or Fail keeps its next step and adds fixing the review's problems to it. A review that did not report, did not read the whole change or did not run still holds approve and merge as before. The verdict is rebuilt from the rules item of the same pull request and head, and the step fails with a clear message when there is none. If the verdict with the review's problems cannot be made, the verdict goes up as the rules made it, with approve and merge held.

The manager's cleared step now receives the rules output and builds the verdict through pr-triage's own evaluation: the verdict-building tail of evaluate is extracted so the manager reuses it, and render gains a function that adds a later failed check to a verdict. The user manual and the setup skill's reference describe the new behaviour.

Verified with bun run check, bun run lint, the triage-manager and triage-workflows tests (368 pass) and bun run test:chain (7 pass). New tests cover:

• A Pass with one P2 finding, a finding exactly at clean.below and a finding with no priority: each fails, request changes runs, and a Pass that was approvable ends not approvable.
• A clean review: the verdict is unchanged.
• A Fail and an Unsure from failed CI: each keeps its verdict with both reasons, the joined reason and the review check.
• A waiting Unsure and a degraded Unsure gaining the review's check, reason, next step and comment.
• A Pass failing while another check can't run yet, a Pass under a policy that never approves staying without an approvable, a degraded verdict with no head staying unposted, and a take with no head leaving the verdict unchanged.
• A comment that already opens with the new title holding it once.
• A verdict that cannot be made with the review (held as before), and no matching rules item failing the step.
• A Pass and a Fail with no comment gaining one that names the review in the next step.
• The judge and review test checks that the judge's words and the review line are both in the posted comment.

triage-manager has no eval, and the judge's behaviour is unchanged, so no eval was run.

Closes CL-11593

[linear-code]

CL-11593

[corbits-triage]

Triage: Pass

No changes needed from you. A maintainer will review this pull request.

Notes:

• Risk: medium (3 recently fixed files)

[corbits-triage]

Turns a code review that finds problems into a Fail verdict instead of a held approve, with the review reported as a failed check, request-changes running, and approval on the head withdrawn. The reason, comment, labels and fail actions all follow the Fail path, and the manager reuses pr-triage's own verdict-building tail. Two defects need addressing: the escalation condition also catches Unsure verdicts, and the new Fail path is not guarded against render errors.

[corbits-triage]

P2 · Unsure verdicts are escalated to Fail by a code review finding, not only Pass

The decided check only excludes degraded, waiting and closed verdicts. It does not require the verdict to be Pass, so an Unsure verdict with a finding on the head also becomes Fail with request-changes. The description only covers Pass turning to Fail, and before this change an Unsure verdict only held approve. For a verdict waiting on a maintainer, failedBy also replaces the rules' own reason (such as a Decide: ask) with the review's reason.

Trigger: a policy rule with onFail: ask, or any path that yields Unsure with degraded: null, followed by code review finding one problem on the head. The verdict becomes Fail with needs-author-update, the maintainer's decision request is lost, and request-changes runs.

Fix: restrict the escalation to Pass verdicts, for example by adding verdict.decision === "pass" (or the equivalent field) to the condition, and keep the held approve for every other verdict. Add a test with an Unsure verdict and a finding that asserts the verdict and reason are unchanged.

[corbits-triage]

P3 · failedVerdict rendering is not guarded, so a render error aborts the cleared step

verdictOf and verdictFrom run inside a try/catch that turns any error into a degraded verdict. The new failedVerdict path calls prepareQualityEvaluation, failedBy (with withChecks) and verdictFrom with no guard. Any exception there propagates out of cleared and fails the whole manager pass, where previously it would have posted a degraded verdict.

Trigger: a head whose review has findings, where rendering the Fail throws, for example on unexpected facts or sources in the item.

Fix: wrap the failedVerdict construction in the same try/catch used by verdictOf, so a render failure degrades the verdict the same way as other render failures:

try {
  return failedVerdict(it, verdict, failed);
} catch (err) {
  return degradedVerdict(it, err);
}

Add a test where rendering throws and assert that the pass still completes with a degraded verdict.

[corbits-triage]

Turns a verdict that would pass into a Fail when the code review finds problems at or above the policy's clean.below threshold, and adds the review as a failed check on Unsure and Fail verdicts. The manager's cleared step now builds the verdict through pr-triage's shared evaluation tail, with a new render function that adds a later failed check. The main defect is that an empty rules items array throws before the fallback, so no verdict is posted.

[corbits-triage]

P2 · Missing rules item throws outside the fallback, failing the whole cleared step

The description says that when the verdict with the review's problems cannot be made, the verdict goes up as the rules made it, with approve and merge held. However, const [item] = items; runs before the try/catch, and when items is empty the destructuring yields undefined, which then throws. The cleared step errors and nothing is posted.

Trigger: a decided verdict whose review has findings, while the rules output carries no item (items: []).

Fix: check for an empty items and take the fallback path explicitly, or move the lookup inside the try block so that any failure there falls back to the rules verdict.

[corbits-triage]

P2 · Stale verdict fields carried into the rebuilt Fail verdict

verdictWithFailure spreads the previous Verdict (including changesWanted, mark and approvable) into withFailure, then into verdictFrom. verdictFrom only overrides these fields when it recomputes a value. When the Fail recomputation yields undefined, the Pass's values survive. For example, approvable can stay true after the Fail is applied, which would let an earlier approval stand on a head that should not be approvable.

Trigger: a Pass carrying changesWanted, mark or approvable is turned to Fail, and the recomputed value is undefined, such as when packApproves is false.

Fix: explicitly reset changesWanted, mark and approvable to the Fail values when building the Fail verdict, rather than relying on verdictFrom to overwrite them.

[corbits-triage]

P2 · Unsure or Fail with an empty comment posts no comment asking the author to fix the review's problems

The spec says an Unsure or already Fail verdict's comment asks the author to fix the review's problems. When the rules verdict has no feedback, withFailure returns the verdict unchanged, so nothing is posted. authorComment also returns an empty string when the only failing check is a maintainer's requested changes (NO_CHANGES_REQUESTED), so the review's problems never reach the author.

Trigger: a Fail verdict whose only failing check is NO_CHANGES_REQUESTED, with code review finding one problem at or above clean.below. The verdict keeps Fail and gains the review reason, but no comment is posted.

Fix: when feedback is empty, build the review's fix-request comment instead of returning early, so the author is always asked to fix the review's problems.

[corbits-triage]

P3 · A Pass with no comment gains a posted comment when turned to Fail

The doc comment says a verdict with no comment still posts none, and the Unsure/Fail branch honours that. The Pass branch always builds a feedback body, so a Pass that had no comment now receives a Fail comment.

Trigger: a pack or flow where the Pass verdict's feedback is empty and the code review then finds a problem.

Fix: keep the existing behaviour by posting a comment only when the previous verdict had feedback, or document and test the intended change if a comment is wanted in that case.

[corbits-triage]

Makes the manager turn code review findings at or above the policy's clean.below threshold into a failed verdict, so a pull request that its rules would pass no longer passes while the review reports problems. The new failure is added through pr-triage's own evaluation and rendering, and one next-step bug remains: an Unsure or Fail verdict keeps its old next step after the review failure is added.

[corbits-triage]

P2 · Unsure or Fail verdict keeps its old next step after a review failure is added

The withFailure function recomputes nextStep only on the Pass-to-Fail branch. On the Unsure/Fail branch, the rendered actor and next action still describe the earlier failures, so the author is not told to fix the review's problems.

Trigger: a pull request whose CI is still running gets a code-review finding. The verdict stays Unsure and still says "Wait for checks to finish" (actor system), though the author now has work to do. The same happens to a Fail whose only step was "Review".

Fix: recompute the actor and next action on the Unsure/Fail branch as well, for example by re-running the next-step logic on the combined set of failures, before returning the rendered verdict:

const nextStep = nextStepFor({ ...rendered, reason: combinedReason, reasons, checks });
return { ...rendered, reason: combinedReason, reasons, checks, feedback, ...nextStep };

[corbits-triage]

Makes the manager fail a pull request whose rules would pass when the code review reports problems, rebuilding the verdict through pr-triage's own evaluation and adding the review's problems to the Fail reason and comment. The main defects are that other waiting checks can silently skip the review findings, and that degraded or waiting Unsure verdicts miss the review reason and comment.

P2 · Inbox reads the pre-review evaluateRules verdict, not the cleared verdict · apps/hub/src/github/triage-runs.ts:54

verdictsOf takes the last completed step from VERDICT_STEPS, which does not include cleared. The manager now produces the authoritative verdict in cleared, which can turn a Pass into a Fail or add the review reason. The Inbox therefore shows the pre-review Pass with approve enabled while GitHub received the Fail.

Triggered by a manager pass where the code review finds problems on a Pass that the rules would pass: evaluateRules completes with Pass and approve, cleared makes it Fail with approve held, and the Inbox reads evaluateRules.

Fix by adding cleared to VERDICT_STEPS and ordering it after evaluateRules:

const VERDICT_STEPS = new Set(["evaluate", "evaluateRules", "cleared", "render", "renderJudged", "renderDet", "renderAll"]);

[corbits-triage]

P2 · Review findings ignored whenever another policy check is waiting

The waiting-check hold in uncleared returns before the code review is examined. When security or deslop is also enabled and still waiting, a review with findings never reaches the found branch, so a Pass stays a Pass with approve and merge held. It is not failed, no fail actions run, and the comment does not ask the author to fix the review's problems.

Triggered by a policy with code-review and security on, where the review reports a P0 finding on a PR whose rules pass.

Fix by checking the review findings before returning the waiting hold, so a found review always produces the Fail verdict regardless of other waiting checks:

const reviewFound = /* existing review findings check */;
if (reviewFound) return { found };
if (waiting.length > 0) return { held: `The policy's ${waiting.join(" and ")} check can't run yet.` };

[corbits-triage]

P2 · Degraded or waiting Unsure verdicts get no review reason and no comment

The spec says an Unsure verdict keeps its verdict, gains the review's problems in its reasons, and its comment asks the author to fix them. The decided guard excludes degraded and waiting verdicts, which are Unsure, so for those cleared only holds approve and merge. The review is not added to reasons or failed checks, and no comment is posted.

Triggered by a PR whose verdict is Unsure with degraded: "inference-outage" (or waiting) and a code review with a P2 finding. The verdict is left with only the held approve reason.

Fix by treating degraded and waiting Unsure verdicts the same as decided ones for the review part: add the review as a failed check, append its problems to reasons, and post the fix-the-review comment, while still holding approve and merge.

[corbits-triage]

P3 · Verdict rebuilt from the first rules item without checking it matches

reviewed always takes items[0] and passes it to verdictWithFailure. Nothing checks that this item is the one the evaluateRules verdict was made for, for example by matching its head. If the rules output holds more than one item, or the evaluated item is not first, the verdict is rebuilt from another item's facts, pack and Dos.

Triggered when the rules step emits several items, or emits them in an order where the evaluated item is not first.

Fix by selecting the item whose head matches the evaluated verdict and holding the PR if none matches:

const item = items.find((i) => i.head === verdict.head);
if (!item) return { held: "The rules output does not match the evaluated head." };

[corbits-triage]

Adds a code review gate to the triage verdict: when the review reports problems at or above clean.below, a verdict that would otherwise pass becomes a Fail, and Unsure or already-Fail verdicts gain the review as a failed check and reason. The manager's cleared step now rebuilds the verdict from the rules item through pr-triage's shared evaluation, and render gains a helper to append a later failed check. Three of the four findings are about the new Fail and comment paths in evaluate and render, and one is about the head lookup in the manager step.

[corbits-triage]

P2 · Rules item looked up by the take's head; a missing head fails the whole step

Why it is wrong: uncleared can report findings from take.review even when take.head is undefined. reviewed then searches for an item whose facts.headSha is undefined, finds none, and throws. The step fails instead of holding approve and merge as it did before.

Trigger: take output carries review with findings but no head.

Suggested fix: identify the rules item by the verdict's own headSha instead of the take's head, and keep the clear failure message only for the case where no matching item exists. Something like function reviewed(items: Item[], verdict: Verdict, found: number): Verdict that matches on verdict.headSha.

[corbits-triage]

P2 · Degraded verdict's new review comment is never posted

Why it is wrong: a degraded verdict is built with headSha null and mirror: false (see degradedVerdict in render.ts). mirror.ts treats any verdict whose mirror is not true as stale. The headSha === null branch in verdictWithFailure only rewrites feedback and request.comment, and leaves mirror false, so the comment asking the author to fix the review's problems is never written. The PR says every verdict's comment asks for the fix, so this case breaks that promise.

Trigger: code review reports findings on a head whose evaluateRules verdict is degraded (for example an inference outage, with headSha null). cleared calls verdictWithFailure, the branch at line 226 runs, and mirrorRules skips the verdict as stale. The new test uses a verdict with a head and mirror: true, so it does not cover this path.

Suggested fix: set mirror to true on the degraded branch as well, or route the comment through a path that does not depend on mirror state. Add a test with a degraded verdict and review findings that asserts the comment is posted.

[corbits-triage]

P3 · Pass turned to Fail always gets approvable: false, even when the pack never approves

Why it is wrong: verdictFrom sets approvable only when the pack has approve or merge rules. Setting it unconditionally adds the field to verdicts that never had it. Revocation logic keyed on approvable being present can then act on pull requests whose policy never approves.

Trigger: a Pass with review findings under a pack that has no approve or merge Dos.

Suggested fix: only override approvable when the verdict already carries the field, for example verdict.verdict === "pass" && "approvable" in verdict ? { ...made, approvable: false } : made, or derive it from the pack the same way verdictFrom does.

[corbits-triage]

P3 · Comment whose first line is not the expected title gets a second title

Why it is wrong: when the first line of the prior feedback differs from commentTitle(verdict), commentBody returns that line as part of the body. withFailure then places a new title above it, so a comment titled differently (for example by a judge) ends up with two titles.

Trigger: prior feedback that starts with a line other than the exact verdict title.

Suggested fix: when the first line is not the expected title, strip or replace the existing title line rather than keeping it, or only add the new title when no title is present. Add a test with a judge-titled comment that checks for a single title.

[corbits-triage]

Makes code review findings fail a pull request's verdict when the rules would otherwise pass it, holding approve and merge. Unsure and Fail verdicts keep their verdict but gain the review as a failed check, and the manager now rebuilds the verdict from the rules output through pr-triage's shared evaluation. Three defects need fixing: the no-head case stops holding approve and merge, a missing rules item can crash the step, and the next step for Unsure verdicts is overwritten instead of extended.

[corbits-triage]

P1 · Review findings on a take with no head no longer hold approve/merge

Why it is wrong: when the review comes from take.review and there is no head, cleared now returns the verdict untouched. The approve and merge holds that were applied before are skipped, so a pull request whose review found problems can be approved or merged.

Trigger: a TakeOutput with review containing counting findings but no head, where the rules verdict is a Pass under a policy that approves.

Suggested fix: when head is undefined, still apply the review's hold to the verdict instead of returning it unchanged, so approve and merge stay held as they were before this change.

  if (head === undefined) return holdApproveAndMerge(verdict, review);

[corbits-triage]

P2 · A degraded verdict with no head can crash the pass when no rules item matches the take's head

Why it is wrong: reviewed looks up the rules item by the take's head and throws when none matches. Degraded verdicts name no head, and their facts can come from a failed read with a different or empty head. The throw fails the whole cleared step, so the verdict is never posted and the hub never receives done. A degraded verdict with no head is not posted anyway, so the throw is unnecessary there.

Trigger: a degraded verdict with headSha null is cleared on a take whose head is HEAD and whose code review found problems, while the rules output has no item at HEAD for that pull request.

Suggested fix: when the verdict has no head, skip the rules lookup and return the verdict with only the review's check added, instead of throwing.

  if (!item && verdict.headSha === null) return verdict;
  if (!item) throw new Error(`cleared: the rules output has no item for #${verdict.number} at ${head}`);

[corbits-triage]

P2 · An Unsure or Fail verdict's next step is replaced, not extended, with the review's problem

Why it is wrong: nextStep is recomputed only over the failing checks. When code review is the only failing check, the verdict's earlier next step is overwritten. Examples are a degraded verdict's "Retry when data is available" with actor system, or the wait for CI on a waiting Unsure. The spec says the next step should name the review's problems as well, not drop what the verdict already asked for.

Trigger: a degraded or waiting Unsure verdict receives a code-review failure in withFailure. Its nextAction and actor become the author's "Fix: ..." instead of the earlier retry or wait.

Suggested fix: keep the verdict's existing next step and add the review's fix to it, rather than recomputing it only from the failing checks.

  return { ...rendered, reason: `${rendered.reason}; ${failed.reason}`, nextAction: rendered.nextAction ?? nextStep(checks, det.sources, reviewers, rendered.verdict).nextAction, reasons, checks, feedback };

[corbits-triage]

Makes code review findings fail an otherwise passing verdict, or add a failed review check, reason, next step and author comment to Unsure and Fail verdicts. The verdict is rebuilt in the manager from the matching rules item, with the evaluation tail extracted into pr-triage for reuse. Two defects stand out: the rules item lookup compares against the take's head rather than the verdict's head, which can throw and drop the post, and an Unsure or Fail that gains the review's fix keeps an actor that no longer matches the next step.

[corbits-triage]

P2 · Rules item looked up by the take's head instead of the verdict's head, so the step can throw

The lookup in reviewed() matches the rules item with facts.headSha === head, where head comes from the take. The verdict is stamped with the rules item's own facts.headSha, so that field should be the one compared. When the take head and the head the rules step read live differ, no item matches, and a non-degraded verdict reaches the throw. The whole cleared step then fails and nothing is posted.

Trigger: a push lands between the take and the rules read, so take.head is not verdict.headSha while the review has findings.

Fix: compare against the verdict's head so the match follows the rules item that produced the verdict:

  const item = items.find(({ facts }) => facts.number === verdict.number && (verdict.headSha === null || facts.headSha === verdict.headSha));

[corbits-triage]

P3 · An Unsure or Fail gaining the review's problem keeps an actor that no longer owns the next step

withFailure appends the review's fix to nextAction but leaves actor as the earlier verdict set it. The comment now asks the author to fix the review's problems, yet an Unsure keeps actor maintainer, and the Inbox shows the wrong owner.

Trigger: an Unsure with actor maintainer from a decided verdict, or a Fail whose actor was not author, gains a review finding. The next step then reads Fix: while the actor does not match it.

Fix: when the review's fix is added to the next step, set the actor to the party that now has to act, for example author, in the same place where nextAction is changed, so the two fields stay consistent.