From 4db98046c51c34871d4a2219fb31d441dd61bc38 Mon Sep 17 00:00:00 2001 From: Security Engineer Date: Tue, 15 Sep 2026 09:14:55 +0000 Subject: [PATCH 1/5] fix(digest): stop the ageing digest suppressing rows whose gate died unanswered (PEN-3089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The human-gated ageing digest is this company's only instrument for surfacing work that has gone quiet behind a human gate. It was removing rows from its own attention list at exactly the moment they most needed one. Two defects, one layer apart. 1. `probeApprovalGate` had no abandoned branch. `APPROVAL_UNDECIDED` listed `pending` and `revision_requested`, and *everything else* fell through to "decided" — so a card the requesting agent `withdrew` rendered identically to one the board `approved`, under a heading reading "Every linked approval has been decided". Nobody decided anything; the asker gave up. Unrecognised statuses fell through the same way, inverting the module's own stated property 2 ("fails toward still-gated") that the sibling interaction probe honours 250 lines later in the same file. The probe now splits four ways, mirroring `probePendingInteraction`: unknown -> `still-gated`, granted -> `approval-granted`, refused -> `approval-refused`, else -> `approval-abandoned` (a non-self-clearing kind). 2. The caller read `verdict === "resolved-but-open"` as "this row is not still waiting". Those are different propositions. A row whose gate cleared and which has *not moved since* is not the least deserving of escalation, it is the most: the thing that explained its silence is gone and nothing replaced it. Approving is also the single write that removes a card from the pending queue, so a grant simultaneously blinds the only other surface watching the ask. `withheldFromAgeRankingIssueIds` is now a strict subset of `resolvedButOpenIssueIds`: every resolved row still renders with its age, but only resolutions that left nothing owed are exempted from escalation. `approval-refused`, `interaction-answered` and `blocker-done-row-not-moved` stay withheld, so this is a narrowing rather than an "escalate everything". The predicate reads every probe on the row rather than the single `resolutionKind` elected for display, so the exemption cannot depend on which probe won a heading. The section heading no longer asserts "these are not still waiting" over rows it no longer withholds; each kind states its own escalation disposition. Live instance: PEN-2224, the root blocker of a critical credential-exposure chain, was withheld for 26 days on the strength of a card its requester retracted. PEN-2526 (P0, approved with the founder's instruction-to-begin on the row) was withheld for 19 days for the opposite reason. Verification: 167 tests across the five human-gated suites, `pnpm -r typecheck` clean. Three separate mutations each fail a distinct, targeted set — emptying `APPROVAL_ABANDONED`, dropping `approval-granted` from `ACTION_OWED_RESOLUTION_ KINDS` (fails the end-to-end producer test too), and dropping `approval-abandoned` from `NON_SELF_CLEARING_RESOLUTION_KINDS`. Co-Authored-By: Claude Signed-off-by: Security Engineer --- .../blo-30608-gate-revalidation-backfill.ts | 4 +- ...man-gated-gate-revalidation-wiring.test.ts | 76 +++++- .../human-gated-gate-revalidation.test.ts | 168 ++++++++++++- .../src/services/human-gated-ageing-digest.ts | 24 +- .../services/human-gated-gate-revalidation.ts | 226 ++++++++++++++++-- 5 files changed, 465 insertions(+), 33 deletions(-) diff --git a/scripts/blo-30608-gate-revalidation-backfill.ts b/scripts/blo-30608-gate-revalidation-backfill.ts index db2bfe8fcf4f..715f062a39d9 100644 --- a/scripts/blo-30608-gate-revalidation-backfill.ts +++ b/scripts/blo-30608-gate-revalidation-backfill.ts @@ -383,8 +383,10 @@ export function renderReport( "resolved-but-open by who can clear it:", ` blocker edge cancelled (never self-clears) : ${report.countsByResolutionKind["blocker-cancelled-edge-stuck"]}`, ` every question card withdrawn/expired : ${report.countsByResolutionKind["interaction-abandoned"]}`, + ` every board card withdrawn/cancelled : ${report.countsByResolutionKind["approval-abandoned"]}`, ` all blockers done, row never moved : ${report.countsByResolutionKind["blocker-done-row-not-moved"]}`, - ` every linked approval decided : ${report.countsByResolutionKind["approval-decided"]}`, + ` board granted the ask, row unperformed : ${report.countsByResolutionKind["approval-granted"]}`, + ` board refused the ask : ${report.countsByResolutionKind["approval-refused"]}`, ` at least one question card answered : ${report.countsByResolutionKind["interaction-answered"]}`, "", "unverifiable by why no gate was checkable:", diff --git a/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts b/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts index 7060d221d773..d8143beed7cd 100644 --- a/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts @@ -228,7 +228,7 @@ describeEmbeddedPostgres("gate re-validation (wired into the digest producer)", const markdown = section!.markdown; // Reported in its own section... - expect(markdown).toContain("Resolved but still open — 1"); + expect(markdown).toContain("Gate resolved but row still open — 1"); expect(markdown).toContain("GRW-1 (41.0d silent)"); expect(markdown).toContain("GRW-2=done"); // ...and NOT aged as if still blocked. @@ -354,7 +354,75 @@ describeEmbeddedPostgres("gate re-validation (wired into the digest producer)", await linkApproval(companyId, stale, "approved"); const markdown = (await collect(companyId))!.markdown; - expect(markdown).toContain("Every linked approval has been decided"); + expect(markdown).toContain("resolved-but-open 1"); + expect(markdown).toContain( + "The board granted the ask and the row has not moved since — authorised, unperformed", + ); + }); + + it("escalates a granted-but-unperformed row instead of exempting it (PEN-3089)", async () => { + // PEN-2526's shape: the board approved, the founder posted the + // instruction-to-begin, and the row then sat `todo` for 19 days. The gate + // really did resolve — into work nobody performed — and the digest read + // that resolution as "this row is not still waiting" and dropped it from + // the one list the founder reads. `threshold (1)` is the whole fix: the + // pre-PEN-3089 producer rendered `(0)` for exactly this input. + const { companyId } = await createCompany("GRG"); + const authorised = await insertIssue({ + companyId, + identifier: "GRG-1", + status: "in_review", + createdAt: daysAgo(41), + }); + await linkApproval(companyId, authorised, "approved"); + + const markdown = (await collect(companyId))!.markdown; + expect(markdown).toContain("Human-gated work past its human-silence threshold (1)"); + expect(markdown).toContain("GRG-1"); + // Still rendered in the resolved section too, carrying its age — being + // escalated must not cost the reader the diagnosis of *why* it is stalled. + expect(markdown).toContain("GRG-1 (41.0d silent)"); + expect(markdown).toContain("⛔ still escalated"); + }); + + it("escalates a row whose only board card the requester withdrew (PEN-3089)", async () => { + // PEN-2224's shape, end to end. `withdrawn` used to render identically to + // `approved` — the probe had no abandoned branch at all — so the root + // blocker of a critical credential-exposure chain sat 26 days inside a + // section headed "these are not still waiting". + const { companyId } = await createCompany("GRD"); + const dropped = await insertIssue({ + companyId, + identifier: "GRD-1", + status: "in_review", + createdAt: daysAgo(41), + }); + await linkApproval(companyId, dropped, "withdrawn"); + + const markdown = (await collect(companyId))!.markdown; + expect(markdown).toContain( + "Every board card was withdrawn or cancelled — the board was asked and never answered", + ); + expect(markdown).toContain("someone must re-ask or drop the row"); + expect(markdown).toContain("Human-gated work past its human-silence threshold (1)"); + }); + + it("still exempts a refused ask from the age-ranked list (PEN-3089)", async () => { + // The narrowing has to stay a narrowing. A rejection is a real answer: the + // ask is over and the row needs closing, not escalating. If this one ever + // starts escalating, the change has stopped discriminating and the digest + // is on its way back to being muted. + const { companyId } = await createCompany("GRR"); + const refused = await insertIssue({ + companyId, + identifier: "GRR-1", + status: "in_review", + createdAt: daysAgo(41), + }); + await linkApproval(companyId, refused, "rejected"); + + const markdown = (await collect(companyId))!.markdown; + expect(markdown).toContain("The board refused the ask"); expect(markdown).toContain("Human-gated work past its human-silence threshold (0)"); }); @@ -389,7 +457,7 @@ describeEmbeddedPostgres("gate re-validation (wired into the digest producer)", const section = await collect(companyId); expect(section).not.toBeNull(); - expect(section!.markdown).toContain("Resolved but still open — 1"); + expect(section!.markdown).toContain("Gate resolved but row still open — 1"); }); it("reports an unreadable-clock row as malformed instead of throwing the producer", async () => { @@ -421,7 +489,7 @@ describeEmbeddedPostgres("gate re-validation (wired into the digest producer)", const section = await collect(companyId); // The producer survived, and the readable row is still classified. expect(section).not.toBeNull(); - expect(section!.markdown).toContain("Resolved but still open — 1"); + expect(section!.markdown).toContain("Gate resolved but row still open — 1"); expect(section!.markdown).toContain("GRM-1"); }); diff --git a/server/src/__tests__/human-gated-gate-revalidation.test.ts b/server/src/__tests__/human-gated-gate-revalidation.test.ts index 370f0782e1a6..d8d40e2321b9 100644 --- a/server/src/__tests__/human-gated-gate-revalidation.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation.test.ts @@ -20,6 +20,7 @@ import { probePendingInteraction, resolvedButOpenIssueIds, revalidateGates, + withheldFromAgeRankingIssueIds, type GateEvidenceInput, } from "../services/human-gated-gate-revalidation.js"; @@ -118,10 +119,67 @@ describe("probeApprovalGate", () => { evidence({ approvals: [{ approvalId: "a1", approvalType: "request_board_approval", approvalStatus: status }] }), ); expect(result?.verdict).toBe("resolved-but-open"); - expect(result?.resolutionKind).toBe("approval-decided"); }, ); + it("separates a granted card from a refused one (PEN-3089)", () => { + // Both resolve the gate, and they resolve it into opposite obligations: a + // grant is an instruction to perform, a refusal ends the ask. Only the + // first leaves the row owing work, which is what decides whether it stays + // in the age-ranked escalation list. + expect( + probeApprovalGate(evidence({ approvals: [{ approvalId: "a1", approvalStatus: "approved" }] })) + ?.resolutionKind, + ).toBe("approval-granted"); + expect( + probeApprovalGate(evidence({ approvals: [{ approvalId: "a1", approvalStatus: "rejected" }] })) + ?.resolutionKind, + ).toBe("approval-refused"); + }); + + it.each(["withdrawn", "cancelled"])( + "reports a %s card as abandoned, not decided (PEN-3089)", + (status) => { + // The requester retracting its own card is not an answer: `decidedByUserId` + // is null on both statuses. Reporting it as a decision is how PEN-2224 — + // the root blocker of a critical credential-exposure chain — spent 26 days + // under a heading asserting it was not still waiting. + const result = probeApprovalGate( + evidence({ approvals: [{ approvalId: "a1", approvalStatus: status }] }), + ); + expect(result?.resolutionKind).toBe("approval-abandoned"); + expect(result?.evidence).toContain("never answered"); + expect(result?.evidence).toContain("re-ask"); + }, + ); + + it("prefers a grant over a sibling refusal", () => { + // One live authorisation means work is owed; the escalation surface must + // not lose it behind a card that was refused. + const result = probeApprovalGate( + evidence({ + approvals: [ + { approvalId: "a1", approvalStatus: "rejected" }, + { approvalId: "a2", approvalStatus: "approved" }, + ], + }), + ); + expect(result?.resolutionKind).toBe("approval-granted"); + expect(result?.evidence).toContain("a2=approved"); + }); + + it("reads an unrecognised status as still-gated, not as a resolution (PEN-3089)", () => { + // `approvals.status` is a plain text column, so a new status is reachable. + // Property 2: a false `still-gated` ages a row one more week, a false + // resolution deletes it from the escalation list. This is the default the + // sibling interaction probe has always had and this one lacked. + const result = probeApprovalGate( + evidence({ approvals: [{ approvalId: "a1", approvalStatus: "escalated_to_board" }] }), + ); + expect(result?.verdict).toBe("still-gated"); + expect(result?.evidence).toContain("a1=escalated_to_board"); + }); + it("stays gated while any one of several cards is undecided", () => { const result = probeApprovalGate( evidence({ @@ -399,8 +457,10 @@ describe("revalidateGates", () => { expect(report.countsByResolutionKind).toEqual({ "blocker-cancelled-edge-stuck": 1, "interaction-abandoned": 0, + "approval-abandoned": 0, "blocker-done-row-not-moved": 1, - "approval-decided": 1, + "approval-granted": 1, + "approval-refused": 0, "interaction-answered": 0, }); }); @@ -449,7 +509,7 @@ describe("revalidateGates", () => { }); describe("resolvedButOpenIssueIds", () => { - it("returns exactly the ids to withhold from the age-ranked list", () => { + it("returns every row whose gate re-tested as resolved", () => { const report = revalidateGates([ evidence({ issueId: "gated", blockers: [{ blockerIssueId: "b", blockerStatus: "todo" }] }), evidence({ issueId: "resolved", blockers: [{ blockerIssueId: "b", blockerStatus: "done" }] }), @@ -457,6 +517,83 @@ describe("resolvedButOpenIssueIds", () => { ]); expect([...resolvedButOpenIssueIds(report)]).toEqual(["resolved"]); }); + + it("keeps covering rows the age-ranking no longer withholds (PEN-3089)", () => { + // This set drives the *age map*, so it must stay wide even as the + // withholding set narrows. If they were collapsed back into one, an + // escalated row would render in the resolved section with no age — trading + // one silent information loss for another. + const report = revalidateGates([ + evidence({ issueId: "abandoned", approvals: [{ approvalId: "a", approvalStatus: "withdrawn" }] }), + evidence({ issueId: "refused", approvals: [{ approvalId: "a", approvalStatus: "rejected" }] }), + ]); + expect(resolvedButOpenIssueIds(report)).toEqual(new Set(["abandoned", "refused"])); + expect(withheldFromAgeRankingIssueIds(report)).toEqual(new Set(["refused"])); + }); +}); + +describe("withheldFromAgeRankingIssueIds (PEN-3089)", () => { + it("does not withhold a row whose only approval was withdrawn", () => { + // The finding, minimally stated. PEN-2224's single linked card was + // withdrawn by the requesting agent with `decidedByUserId: null`, and that + // alone removed the root blocker of a critical credential-exposure chain + // from the founder's only attention list. + const report = revalidateGates([ + evidence({ issueId: "pen-2224", approvals: [{ approvalId: "7291f2b7", approvalStatus: "withdrawn" }] }), + ]); + expect(withheldFromAgeRankingIssueIds(report).has("pen-2224")).toBe(false); + }); + + it("does not withhold a row whose approval was granted and which never moved", () => { + // PEN-2526, the P0: approved 2026-08-27 with the founder's own + // instruction-to-begin on the row, still `todo` 19 days later. The gate + // genuinely resolved — into work nobody performed. + const report = revalidateGates([ + evidence({ issueId: "pen-2526", approvals: [{ approvalId: "5c57f5ee", approvalStatus: "approved" }] }), + ]); + expect(withheldFromAgeRankingIssueIds(report).has("pen-2526")).toBe(false); + }); + + it("still withholds the kinds whose resolution leaves nothing owed", () => { + // The narrowing has to stay a narrowing. A refused ask, an answered + // question and a done blocker chain are all genuinely finished as gates; + // escalating them would flood the list and teach the reader to mute it. + const report = revalidateGates([ + evidence({ issueId: "refused", approvals: [{ approvalId: "a", approvalStatus: "rejected" }] }), + evidence({ + issueId: "answered", + interactions: [{ interactionId: "i", interactionStatus: "answered" }], + }), + evidence({ issueId: "done", blockers: [{ blockerIssueId: "b", blockerStatus: "done" }] }), + ]); + expect(withheldFromAgeRankingIssueIds(report)).toEqual( + new Set(["refused", "answered", "done"]), + ); + }); + + it("escalates on any action-owed probe, not just the one elected primary", () => { + // PEN-2224's real shape: an abandoned board card *and* an answered question + // card. `combineProbeVerdicts` elects one `resolutionKind` for display, so + // hanging the exemption off that election would make escalation depend on + // which probe won a heading. Reading every probe makes it order-independent + // — this row must escalate whichever kind is shown. + const report = revalidateGates([ + evidence({ + issueId: "mixed", + approvals: [{ approvalId: "a", approvalStatus: "withdrawn" }], + interactions: [{ interactionId: "i", interactionStatus: "answered" }], + }), + ]); + expect(withheldFromAgeRankingIssueIds(report).has("mixed")).toBe(false); + }); + + it("never withholds a row that is still gated or unverifiable", () => { + const report = revalidateGates([ + evidence({ issueId: "gated", approvals: [{ approvalId: "a", approvalStatus: "pending" }] }), + evidence({ issueId: "silent" }), + ]); + expect(withheldFromAgeRankingIssueIds(report).size).toBe(0); + }); }); describe("formatGateRevalidationSections", () => { @@ -483,13 +620,36 @@ describe("formatGateRevalidationSections", () => { const markdown = formatGateRevalidationSections(report, { ageDaysByIssueId: new Map([["resolved", 41.2]]), }); - expect(markdown).toContain("Resolved but still open — 1"); + expect(markdown).toContain("Gate resolved but row still open — 1"); expect(markdown).toContain("withheld from the age-ranked list"); // Reclassification must not lose information the reader already had. expect(markdown).toContain("BLO-29399 (41.2d silent)"); expect(markdown).toContain("BLO-29004=done"); }); + it("marks an escalated kind and does not claim it was withheld (PEN-3089)", () => { + // The section heading used to assert "these are not still waiting" over + // every row in it. Leaving that in place while escalating some of them + // would trade one suppression for a plain contradiction: the reader would + // see the row in both lists with only one of them telling the truth. + const report = revalidateGates([ + evidence({ + issueId: "granted", + identifier: "PEN-2526", + approvals: [{ approvalId: "5c57f5ee", approvalStatus: "approved" }], + }), + evidence({ + issueId: "refused", + identifier: "PEN-2077", + approvals: [{ approvalId: "e1e9ba01", approvalStatus: "rejected" }], + }), + ]); + const markdown = formatGateRevalidationSections(report); + expect(markdown).not.toContain("these are not still waiting"); + expect(markdown).toContain("authorised, unperformed — 1** (⛔ still escalated"); + expect(markdown).toContain("needs closing, not re-asking — 1** (withheld"); + }); + it("leads with the resolution kind that cannot clear itself", () => { const report = revalidateGates([ evidence({ issueId: "finished", blockers: [{ blockerIssueId: "b", blockerStatus: "done" }] }), diff --git a/server/src/services/human-gated-ageing-digest.ts b/server/src/services/human-gated-ageing-digest.ts index 48c71857d4d5..14def0a1a03d 100644 --- a/server/src/services/human-gated-ageing-digest.ts +++ b/server/src/services/human-gated-ageing-digest.ts @@ -63,6 +63,7 @@ import { formatGateRevalidationSections, resolvedButOpenIssueIds, revalidateGates, + withheldFromAgeRankingIssueIds, type GateEvidenceInput, } from "./human-gated-gate-revalidation.js"; // The second producer on this seam (BLO-30259). Reads @@ -492,10 +493,15 @@ export async function loadGateEvidence( * the BLO-30608 gate re-validation pass in front of it. * * Order matters and is the point of BLO-30608: re-validation runs **first**, and - * the rows it finds `resolved-but-open` are withheld from the age-ranked list - * rather than aged another day as if they were still waiting. They are not - * dropped — they are rendered in their own section, carrying their age, so - * reclassification never loses information a reader had before. + * a row whose gate re-tests as resolved is reported in its own section, carrying + * its age, rather than aged another day as if the gate were still live. + * + * PEN-3089 narrowed what that section *exempts*. Being resolved no longer + * removes a row from the age-ranked list on its own: only a resolution that + * left nothing owed does ({@link withheldFromAgeRankingIssueIds}). A gate that + * resolved by the requester withdrawing the ask, or by the board granting work + * nobody then performed, keeps the row escalated — those are the states where + * the row most needs a reader, and they were the ones being dropped. */ export const humanGatedAgeingProducer: DigestProducer = { key: "human-gated-ageing", @@ -522,11 +528,17 @@ export const humanGatedAgeingProducer: DigestProducer = { const byAgeDescending = orderByHumanSilenceDescending(candidates, now); const evidence = await loadGateEvidence(db, companyId, byAgeDescending); const revalidation = revalidateGates(evidence, { maxProbes: DEFAULT_MAX_PROBES }); - const withheld = resolvedButOpenIssueIds(revalidation); + // Two sets, deliberately not one (PEN-3089). Every resolved-but-open row is + // rendered with its age, so `resolvedRows` drives the age map; only the + // rows whose resolution left nothing owed are exempted from escalation, so + // the narrower `withheld` drives the filter. Using one set for both is what + // let an ask the requester withdrew delete its row from the age-ranked list. + const resolvedRows = resolvedButOpenIssueIds(revalidation); + const withheld = withheldFromAgeRankingIssueIds(revalidation); const ageDaysByIssueId = new Map( candidates - .filter((candidate) => withheld.has(candidate.id)) + .filter((candidate) => resolvedRows.has(candidate.id)) .map((candidate) => [candidate.id, rankableSilenceDays(candidate, now)]) // An unrankable row has no age to render; omitting it leaves the entry // absent so the renderer prints no age rather than a fabricated one. diff --git a/server/src/services/human-gated-gate-revalidation.ts b/server/src/services/human-gated-gate-revalidation.ts index abf89ede4735..b1586b404d3e 100644 --- a/server/src/services/human-gated-gate-revalidation.ts +++ b/server/src/services/human-gated-gate-revalidation.ts @@ -115,9 +115,23 @@ export type GateProbeKind = * ever answered and no answer is now coming. The row reads as "waiting on a * human" while the thing it was waiting on no longer exists. Like a stuck * edge it cannot self-clear — someone has to re-ask or drop the row. + * - `approval-abandoned` (PEN-3089) is the approval-side twin of + * `interaction-abandoned`: every linked card was `withdrawn` by its own + * requester or `cancelled`, so the board never answered and no answer is + * coming. Before PEN-3089 these rows were reported as `approval-decided` — + * identically to an `approved` card — which is how PEN-2224, the root blocker + * of a critical credential-exposure chain, spent 26 days inside a section + * headed "these are not still waiting". * - `blocker-done-row-not-moved` is a row whose blockers all completed; the * platform already considers it dependency-ready and it is merely still open. - * - `approval-decided` is a row whose every linked approval has been answered. + * - `approval-granted` is a row whose board gate opened: at least one linked + * approval is `approved`. The gate really did resolve — but what it resolved + * *into* is an instruction to perform, so the row is now authorised and + * unperformed. See {@link ACTION_OWED_RESOLUTION_KINDS} for why that is kept + * in the escalation list rather than withheld from it. + * - `approval-refused` is a row whose board gate closed with a `rejected` card + * and no grant. The ask was answered "no"; nothing further is owed by the + * gate and the row needs closing, not escalating. * - `interaction-answered` is a row where at least one question card got a real * human decision (accepted / rejected / answered) and which is still open * anyway. Deliberately *not* "every card": a row whose remaining cards were @@ -127,8 +141,10 @@ export type GateProbeKind = export type GateResolutionKind = | "blocker-cancelled-edge-stuck" | "interaction-abandoned" + | "approval-abandoned" | "blocker-done-row-not-moved" - | "approval-decided" + | "approval-granted" + | "approval-refused" | "interaction-answered"; /** @@ -168,6 +184,32 @@ const BLOCKER_TERMINAL_NON_RESOLVING_STATUSES: ReadonlySet = new Set(["c /** Approval statuses that mean the board has not answered yet. */ const APPROVAL_UNDECIDED: ReadonlySet = new Set(["pending", "revision_requested"]); +/** + * Approval statuses where the board actually answered (PEN-3089). + * + * Split into grant and refusal because the two resolve the gate into opposite + * obligations. A grant is an instruction to perform, so it leaves work owed by + * whoever the row is assigned to; a refusal ends the ask outright. The digest + * treats them differently — see {@link ACTION_OWED_RESOLUTION_KINDS}. + */ +const APPROVAL_GRANTED: ReadonlySet = new Set(["approved"]); +const APPROVAL_REFUSED: ReadonlySet = new Set(["rejected"]); + +/** + * Approval statuses where the ask went away *without* a board answer. + * + * `withdrawn` is the requesting agent retracting its own card — routinely and + * correctly, to keep a human queue honest — and `cancelled` is the same shape + * from the platform side. Neither is a decision: `decidedByUserId` is `null` on + * both, and the exact mechanism that keeps the queue honest is what used to + * delete the row from this digest. + * + * Mirrors {@link INTERACTION_ABANDONED}, which the interaction probe has + * distinguished since BLO-30627. The approval probe simply never grew the + * branch. + */ +const APPROVAL_ABANDONED: ReadonlySet = new Set(["withdrawn", "cancelled"]); + /** * Interaction statuses that mean a human still owes an answer. * @@ -393,27 +435,79 @@ export function probeBlockerPremise(input: GateEvidenceInput): ProbeResult | nul * Reads the linked cards' own statuses. For `gate.kind: github_actions_run` * cards that status is already maintained against the live run by * `approval-gate-reconciler.ts`, so this needs no GitHub call of its own. + * + * Four outcomes (PEN-3089 split the last three out of one): + * + * - any card still undecided, **or carrying a status this module does not + * recognise** → `still-gated`; + * - otherwise, any card `approved` → `approval-granted`; + * - otherwise, any card `rejected` → `approval-refused`; + * - otherwise every card was withdrawn or cancelled → the board was asked and + * never answered (`approval-abandoned`). + * + * Grant beats refusal on a mixed row for the same reason unknown beats + * everything: a single live authorisation means work is owed, and the + * escalation surface must not lose it behind a sibling refusal. + * + * The unknown-status branch is property 2 (fail toward `still-gated`) applied + * to schema drift. `approvals.status` is a plain `text` column, and before + * PEN-3089 this probe read *every* non-undecided value as a resolution — the + * exact inversion of the default {@link probePendingInteraction} has honoured + * since BLO-30627, in the same file. */ export function probeApprovalGate(input: GateEvidenceInput): ProbeResult | null { if (input.approvals.length === 0) return null; - const undecided = input.approvals.filter((approval) => - APPROVAL_UNDECIDED.has(approval.approvalStatus), + const live = input.approvals.filter( + (approval) => + APPROVAL_UNDECIDED.has(approval.approvalStatus) || + (!APPROVAL_GRANTED.has(approval.approvalStatus) && + !APPROVAL_REFUSED.has(approval.approvalStatus) && + !APPROVAL_ABANDONED.has(approval.approvalStatus)), ); - if (undecided.length > 0) { + if (live.length > 0) { return { probe: "approval-gate", verdict: "still-gated", - evidence: `${undecided.length} of ${input.approvals.length} linked approval${input.approvals.length === 1 ? "" : "s"} still undecided: ${undecided.map(describeApproval).join(", ")}`, + evidence: `${live.length} of ${input.approvals.length} linked approval${input.approvals.length === 1 ? "" : "s"} still undecided: ${live.map(describeApproval).join(", ")}`, + }; + } + + const granted = input.approvals.filter((approval) => + APPROVAL_GRANTED.has(approval.approvalStatus), + ); + + if (granted.length > 0) { + return { + probe: "approval-gate", + verdict: "resolved-but-open", + resolutionKind: "approval-granted", + evidence: `${granted.map(describeApproval).join(", ")} — ${granted.length} of ${input.approvals.length} linked approval${input.approvals.length === 1 ? " was" : "s were"} granted and the row has not moved since, so it is authorised and unperformed: the gate opened and whoever the row is assigned to still owes the work`, + }; + } + + const refused = input.approvals.filter((approval) => + APPROVAL_REFUSED.has(approval.approvalStatus), + ); + + if (refused.length > 0) { + return { + probe: "approval-gate", + verdict: "resolved-but-open", + resolutionKind: "approval-refused", + evidence: `all ${input.approvals.length} linked approval${input.approvals.length === 1 ? " was" : "s were"} answered and none was granted: ${input.approvals.map(describeApproval).join(", ")} — the ask was refused, so this row needs closing rather than re-asking`, }; } + // The card refs lead, as they do in the cancelled-blocker and abandoned- + // interaction branches: the rendered evidence is length-bounded, and *which* + // ask died is the only part a reader can act on. return { probe: "approval-gate", verdict: "resolved-but-open", - resolutionKind: "approval-decided", - evidence: `all ${input.approvals.length} linked approval${input.approvals.length === 1 ? " has" : "s have"} been decided: ${input.approvals.map(describeApproval).join(", ")}`, + resolutionKind: "approval-abandoned", + evidence: `${input.approvals.map(describeApproval).join(", ")} — all ${input.approvals.length} linked approval${input.approvals.length === 1 ? " was" : "s were"} withdrawn or cancelled, so the board was asked and never answered and no answer is coming; someone must re-ask or drop the row`, }; } @@ -496,14 +590,61 @@ const PROBES: ReadonlyArray<(input: GateEvidenceInput) => ProbeResult | null> = /** * Resolution kinds that can never clear themselves, most severe first. * - * Both describe a gate whose counterparty is gone: a cancelled blocker edge no - * `done` can ever satisfy, and a question every card for which was withdrawn. - * They are reported ahead of the merely-finished kinds because they are the - * ones a reader has to *act* on rather than notice. + * Each describes a gate whose counterparty is gone: a cancelled blocker edge no + * `done` can ever satisfy, a question every card for which was withdrawn, and a + * board ask every card for which was withdrawn or cancelled. They are reported + * ahead of the merely-finished kinds because they are the ones a reader has to + * *act* on rather than notice. + * + * This ordering also decides which kind a multi-probe row is filed under + * ({@link combineProbeVerdicts}), so membership here is what makes PEN-2224 — + * an abandoned board card plus an answered question card — report as abandoned + * rather than as answered. */ const NON_SELF_CLEARING_RESOLUTION_KINDS: readonly GateResolutionKind[] = Object.freeze([ "blocker-cancelled-edge-stuck", "interaction-abandoned", + "approval-abandoned", +]); + +/** + * Resolution kinds that leave an action owed, so the row keeps its place in the + * age-ranked escalation list (PEN-3089). + * + * This is the set {@link withheldFromAgeRankingIssueIds} inverts, and the + * distinction it draws is the one the caller used to get wrong. "The gate + * blocking this row resolved" and "this row is not still waiting" are different + * propositions, and for human-gated work they come apart completely: a row + * whose gate cleared and which has *not moved since* is not the least deserving + * of escalation, it is the most — the thing that explained its silence is gone + * and nothing replaced it. + * + * Two reasons land a kind here, and only one of them is "the counterparty is + * gone": + * + * - every {@link NON_SELF_CLEARING_RESOLUTION_KINDS} kind — nobody answered and + * nobody will, so someone must re-ask or clear the edge; + * - `approval-granted` — somebody *did* answer, and the answer was "yes, do it". + * An authorisation is not a completion. Approving is also the single write + * that removes the card from the pending-approval queue, so the grant + * simultaneously ends the only other surface that was watching the ask; if + * the digest exempts the row too, authorised-but-unperformed work becomes + * unobserved by construction. Resolution does fire a one-shot wake at the + * *requesting* agent (`REQUESTER_WAKE_REASONS`, `approval-resolution.ts`), + * but a single wake at decision time is not a standing watch, and it reaches + * the asker rather than whoever the row is assigned to. + * + * Deliberately excluded, so this stays a narrowing and not an "escalate + * everything": `approval-refused` (the ask was answered "no" — nothing further + * is owed by the gate), `interaction-answered` (a human engaged and the answer + * is on the row), and `blocker-done-row-not-moved` (the platform already treats + * the row as dependency-ready, so it is ordinary un-started work that every + * agent-side sweep can already see). All three keep rendering in the + * resolved-but-open section with their age; they are simply not escalated. + */ +const ACTION_OWED_RESOLUTION_KINDS: ReadonlySet = new Set([ + ...NON_SELF_CLEARING_RESOLUTION_KINDS, + "approval-granted", ]); /** Statuses whose `unverifiable` residual is a contradiction, not an absence. */ @@ -663,8 +804,10 @@ export function revalidateGates( const countsByResolutionKind: Record = { "blocker-cancelled-edge-stuck": 0, "interaction-abandoned": 0, + "approval-abandoned": 0, "blocker-done-row-not-moved": 0, - "approval-decided": 0, + "approval-granted": 0, + "approval-refused": 0, "interaction-answered": 0, }; const countsByUnverifiableReason: Record = { @@ -694,7 +837,7 @@ export function revalidateGates( }; } -/** Ids the caller must withhold from the age-ranked list (AC2). */ +/** Ids whose gate re-tested as resolved, so the renderer can carry their age. */ export function resolvedButOpenIssueIds(report: GateRevalidationReport): Set { return new Set( report.classifications @@ -703,6 +846,39 @@ export function resolvedButOpenIssueIds(report: GateRevalidationReport): Set { + return new Set( + report.classifications + .filter( + (classification) => + classification.verdict === "resolved-but-open" && + !classification.probes.some( + (probe) => probe.resolutionKind && ACTION_OWED_RESOLUTION_KINDS.has(probe.resolutionKind), + ), + ) + .map((classification) => classification.issueId), + ); +} + /** Longest evidence string rendered per row, before ellipsis. */ const MAX_RENDERED_EVIDENCE_CHARS = 300; @@ -711,8 +887,12 @@ const RESOLUTION_KIND_HEADINGS: Record = { "Blocker edge is cancelled — permanently un-checkoutable until an operator clears it", "interaction-abandoned": "Every question card was withdrawn or expired — the human was asked and never answered", + "approval-abandoned": + "Every board card was withdrawn or cancelled — the board was asked and never answered", "blocker-done-row-not-moved": "Every blocker is done — the row simply never moved", - "approval-decided": "Every linked approval has been decided", + "approval-granted": + "The board granted the ask and the row has not moved since — authorised, unperformed", + "approval-refused": "The board refused the ask — this row needs closing, not re-asking", // Not "every card was answered": the branch that assigns this kind fires // whenever *at least one* card got a real decision, so the rest may have been // cancelled, expired, or failed. The evidence line already says "closed, N by @@ -871,14 +1051,17 @@ export function formatGateRevalidationSections( body.push( "", - `#### Resolved but still open — ${resolved.length} (withheld from the age-ranked list; these are not still waiting)`, + `#### Gate resolved but row still open — ${resolved.length}`, + "", + "Each row below had its gate re-tested and the gate is no longer live. That is *not* the same as 'no longer waiting': where the resolution left an action owed, the row is marked ⛔ and still appears in the age-ranked list. Only the unmarked rows are withheld from it.", ); // Order by resolution kind so the ones that cannot self-clear lead. const kindOrder: GateResolutionKind[] = [ ...NON_SELF_CLEARING_RESOLUTION_KINDS, + "approval-granted", "blocker-done-row-not-moved", - "approval-decided", + "approval-refused", "interaction-answered", ]; @@ -887,7 +1070,14 @@ export function formatGateRevalidationSections( const inKind = resolved.filter((classification) => classification.resolutionKind === kind); if (inKind.length === 0) continue; - body.push("", `**${RESOLUTION_KIND_HEADINGS[kind]} — ${inKind.length}**`); + // Stated per kind rather than once at the top: the escalation consequence + // is a property of the kind, and a reader scanning one block should not + // have to hold the legend in their head to know whether these rows reached + // the list below. + const disposition = ACTION_OWED_RESOLUTION_KINDS.has(kind) + ? "⛔ still escalated — an action is owed" + : "withheld from the age-ranked list"; + body.push("", `**${RESOLUTION_KIND_HEADINGS[kind]} — ${inKind.length}** (${disposition})`); for (const classification of inKind) { if (listed >= maxListed) break; const ref = formatRef( From ef6efed3eb66198660834ab3655c6532727508da Mon Sep 17 00:00:00 2001 From: Security Engineer Date: Tue, 15 Sep 2026 18:39:05 +0000 Subject: [PATCH 2/5] fix(digest): derive the resolved-row disposition per row, not per kind (PEN-3089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ally's review on #1872 found that the fix did not reach the renderer. The per-kind disposition label read `ACTION_OWED_RESOLUTION_KINDS.has(kind)` — the single kind `combineProbeVerdicts` elects — while the escalation filter `withheldFromAgeRankingIssueIds` reads *every* probe on the row. Those two inputs disagree, so the section could state the opposite of the truth. `approval-granted` is the one action-owed kind absent from `NON_SELF_CLEARING_RESOLUTION_KINDS`, so it wins the election only by being `resolved[0]` — and `probeBlockerPremise` runs first. A row with every blocker `done` plus one `approved` card therefore files under `blocker-done-row-not-moved` and renders as "(withheld from the age-ranked list)" while being in that list. That is the same false claim this ticket removed from the global heading, reintroduced per kind. The renderer now reads the same set the filter does. The heading is a grouping label carrying a tally of the per-row verdicts beneath it, mixed blocks say so, and each row gets the per-row ⛔ marker the section legend already promised. Also from the same review: - `probeApprovalGate` splits `live` into `undecided` and `unrecognised`, so schema drift no longer renders as "1 of 1 linked approval still undecided: a1=escalated_to_board". An unrecognised status is not a pending card, and the digest is where that drift would be noticed. Both still fail toward `still-gated`, so the safety property is unchanged. - `kindOrder` becomes `RESOLUTION_KIND_RENDER_RANK`, a `Record`, so the compiler refuses a new union member nobody ranked. The plain array could not: a new kind rendered *nowhere*, which is the silent-omission class this ticket exists to fix. Verified by adding a member to the union and watching tsc reject it. - `itemCount` in the ageing producer is documented as item *mentions* rather than distinct rows. The two addends stopped being disjoint when action-owed rows began appearing in both sections; the value is telemetry only, never control flow, so it is left as a sum with its meaning recorded. Tests: the pre-existing "escalates on any action-owed probe" case used `approval-abandoned`, which is non-self-clearing and therefore *wins* the election, so it passed without exercising the divergence it named. Added the `approval-granted` + `blocker-done-row-not-moved` shape at both the predicate and the renderer, plus the mixed-block tally and the unrecognised-status evidence split. Mutation-checked both ways: making the renderer read the elected kind fails only the new renderer test, and making the predicate read the elected kind fails only the two new tests — the old one passes under both, as Ally said. 117 tests pass across the classifier, digest and embedded-Postgres wiring suites; `pnpm -r typecheck` clean. Co-Authored-By: Claude Signed-off-by: Security Engineer --- .../human-gated-gate-revalidation.test.ts | 85 +++++++++++++ .../src/services/human-gated-ageing-digest.ts | 12 ++ .../services/human-gated-gate-revalidation.ts | 118 ++++++++++++++---- 3 files changed, 191 insertions(+), 24 deletions(-) diff --git a/server/src/__tests__/human-gated-gate-revalidation.test.ts b/server/src/__tests__/human-gated-gate-revalidation.test.ts index d8d40e2321b9..6fe7b5000427 100644 --- a/server/src/__tests__/human-gated-gate-revalidation.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation.test.ts @@ -178,6 +178,26 @@ describe("probeApprovalGate", () => { ); expect(result?.verdict).toBe("still-gated"); expect(result?.evidence).toContain("a1=escalated_to_board"); + // Schema drift must not be reported as a real card state. "Still + // undecided" describes a pending card, which this is not; the digest is + // where an unrecognised status would be noticed, so it has to say so. + expect(result?.evidence).toContain("does not recognise"); + expect(result?.evidence).not.toContain("still undecided"); + }); + + it("names undecided and unrecognised cards separately on a mixed row", () => { + const result = probeApprovalGate( + evidence({ + approvals: [ + { approvalId: "a1", approvalStatus: "pending" }, + { approvalId: "a2", approvalStatus: "escalated_to_board" }, + ], + }), + ); + expect(result?.verdict).toBe("still-gated"); + expect(result?.evidence).toContain("1 of 2 linked approvals still undecided: a1=pending"); + expect(result?.evidence).toContain("does not recognise"); + expect(result?.evidence).toContain("a2=escalated_to_board"); }); it("stays gated while any one of several cards is undecided", () => { @@ -587,6 +607,34 @@ describe("withheldFromAgeRankingIssueIds (PEN-3089)", () => { expect(withheldFromAgeRankingIssueIds(report).has("mixed")).toBe(false); }); + it("escalates a granted card that lost the kind election to a blocker-done probe", () => { + // The only shape where the election and the exemption come apart, so the + // only one that actually tests "reads every probe". `approval-granted` is + // the single `ACTION_OWED_RESOLUTION_KINDS` member absent from + // `NON_SELF_CLEARING_RESOLUTION_KINDS`, so it wins the election only by + // being `resolved[0]` — and `probeBlockerPremise` runs first, so a + // blocker-done probe takes the heading instead. + // + // The sibling test above cannot detect this: its withdrawn card elects + // `approval-abandoned`, which is non-self-clearing and therefore *wins*, so + // a predicate reading the elected kind would pass it too. + const report = revalidateGates([ + evidence({ + issueId: "granted-behind-blocker", + blockers: [{ blockerIssueId: "b", blockerStatus: "done" }], + approvals: [{ approvalId: "a", approvalStatus: "approved" }], + }), + ]); + const [classification] = report.classifications; + // Guard the premise: if the election ever stops picking the blocker probe, + // this test silently stops exercising the divergence it is named for. + expect(classification?.resolutionKind).toBe("blocker-done-row-not-moved"); + expect(classification?.probes.map((probe) => probe.resolutionKind)).toContain( + "approval-granted", + ); + expect(withheldFromAgeRankingIssueIds(report).has("granted-behind-blocker")).toBe(false); + }); + it("never withholds a row that is still gated or unverifiable", () => { const report = revalidateGates([ evidence({ issueId: "gated", approvals: [{ approvalId: "a", approvalStatus: "pending" }] }), @@ -650,6 +698,43 @@ describe("formatGateRevalidationSections", () => { expect(markdown).toContain("needs closing, not re-asking — 1** (withheld"); }); + it("marks the row, not the kind, so an escalated row under a withheld kind is not mislabelled (PEN-3089)", () => { + // The divergence the per-kind label reintroduced. Both rows elect + // `blocker-done-row-not-moved` — a kind that is *not* action-owed — but the + // first also carries a granted card, so `withheldFromAgeRankingIssueIds` + // keeps it escalated. Labelling the block from its kind printed "withheld + // from the age-ranked list" over a row that was in that list, which is the + // same false claim this ticket removed from the global heading. + const report = revalidateGates([ + evidence({ + issueId: "escalated", + identifier: "PEN-3000", + blockers: [{ blockerIssueId: "b", blockerStatus: "done" }], + approvals: [{ approvalId: "a1", approvalStatus: "approved" }], + }), + evidence({ + issueId: "withheld", + identifier: "PEN-3001", + blockers: [{ blockerIssueId: "c", blockerStatus: "done" }], + }), + ]); + const markdown = formatGateRevalidationSections(report); + + // One heading, holding rows with opposite dispositions: it must report the + // split rather than assert either disposition over both. + expect(markdown).toContain("never moved — 2** (⛔ 1 still escalated · 1 withheld"); + // The escalated row is marked; the withheld one is not. + expect(markdown).toContain("- ⛔ PEN-3000"); + expect(markdown).toContain("- PEN-3001"); + expect(markdown).not.toContain("- ⛔ PEN-3001"); + + // The label can never disagree with the filter: every marked row is absent + // from `withheld`, and every unmarked one is in it. + const withheld = withheldFromAgeRankingIssueIds(report); + expect(withheld.has("escalated")).toBe(false); + expect(withheld.has("withheld")).toBe(true); + }); + it("leads with the resolution kind that cannot clear itself", () => { const report = revalidateGates([ evidence({ issueId: "finished", blockers: [{ blockerIssueId: "b", blockerStatus: "done" }] }), diff --git a/server/src/services/human-gated-ageing-digest.ts b/server/src/services/human-gated-ageing-digest.ts index 14def0a1a03d..cbf5e18d6b8e 100644 --- a/server/src/services/human-gated-ageing-digest.ts +++ b/server/src/services/human-gated-ageing-digest.ts @@ -577,6 +577,18 @@ export const humanGatedAgeingProducer: DigestProducer = { return { key: "human-gated-ageing", markdown, + // Item *mentions* across the two rendered sections, not distinct rows. + // Before PEN-3089 the two addends were disjoint — every resolved-but-open + // row was withheld from the age-ranked list — so the sum was an exact row + // count. Now an action-owed resolved row is rendered in both sections and + // counted twice, and since escalation also requires passing the silence + // threshold, that overlap covers most of the newly-escalated population. + // Left as a sum deliberately: this value is telemetry (a digest-size + // signal and a log field), never control flow, and an exact distinct + // count would mean plumbing the over-threshold issue ids out of + // `selectAgedHumanGatedIssues` to serve a number nothing branches on. The + // meaning is recorded here so the next reader does not mistake it for a + // row count. itemCount: report.totalOverThreshold + revalidation.counts["resolved-but-open"], }; }, diff --git a/server/src/services/human-gated-gate-revalidation.ts b/server/src/services/human-gated-gate-revalidation.ts index b1586b404d3e..56f2ee7ee932 100644 --- a/server/src/services/human-gated-gate-revalidation.ts +++ b/server/src/services/human-gated-gate-revalidation.ts @@ -458,19 +458,42 @@ export function probeBlockerPremise(input: GateEvidenceInput): ProbeResult | nul export function probeApprovalGate(input: GateEvidenceInput): ProbeResult | null { if (input.approvals.length === 0) return null; - const live = input.approvals.filter( + // Two reasons a gate reads as live, kept apart because they mean different + // things to whoever reads the digest: a card nobody has decided yet, and a + // card carrying a status this module has never heard of. Both fail toward + // `still-gated` (property 2), so the safety behaviour is identical — but + // reporting schema drift as "still undecided" would describe a real card + // state that is not the one observed, and the digest is the surface where + // that drift would be noticed. + const undecided = input.approvals.filter((approval) => + APPROVAL_UNDECIDED.has(approval.approvalStatus), + ); + const unrecognised = input.approvals.filter( (approval) => - APPROVAL_UNDECIDED.has(approval.approvalStatus) || - (!APPROVAL_GRANTED.has(approval.approvalStatus) && - !APPROVAL_REFUSED.has(approval.approvalStatus) && - !APPROVAL_ABANDONED.has(approval.approvalStatus)), + !APPROVAL_UNDECIDED.has(approval.approvalStatus) && + !APPROVAL_GRANTED.has(approval.approvalStatus) && + !APPROVAL_REFUSED.has(approval.approvalStatus) && + !APPROVAL_ABANDONED.has(approval.approvalStatus), ); - if (live.length > 0) { + if (undecided.length > 0 || unrecognised.length > 0) { + const total = input.approvals.length; + const plural = total === 1 ? "" : "s"; + const clauses: string[] = []; + if (undecided.length > 0) { + clauses.push( + `${undecided.length} of ${total} linked approval${plural} still undecided: ${undecided.map(describeApproval).join(", ")}`, + ); + } + if (unrecognised.length > 0) { + clauses.push( + `${unrecognised.length} of ${total} linked approval${plural} carr${unrecognised.length === 1 ? "ies" : "y"} a status this module does not recognise, so the gate is read as live rather than resolved: ${unrecognised.map(describeApproval).join(", ")}`, + ); + } return { probe: "approval-gate", verdict: "still-gated", - evidence: `${live.length} of ${input.approvals.length} linked approval${input.approvals.length === 1 ? "" : "s"} still undecided: ${live.map(describeApproval).join(", ")}`, + evidence: clauses.join("; "), }; } @@ -903,6 +926,37 @@ const RESOLUTION_KIND_HEADINGS: Record = { "At least one question card was answered — any remaining cards closed without an answer", }; +/** + * Render rank per resolution kind — lower renders first. + * + * A `Record` rather than an ordered array so the + * compiler refuses a new union member that nobody gave a rank. The previous + * plain array could not: adding a kind left it absent from the render order, + * and rows of that kind rendered *nowhere* in the digest. A silent omission + * from the escalation surface is precisely the failure PEN-3089 exists to fix, + * so the next person to widen the union should not have to remember this list. + * + * Ranks 0-2 intentionally mirror {@link NON_SELF_CLEARING_RESOLUTION_KINDS}: + * the kinds whose counterparty is gone lead, because they are the ones a reader + * must act on rather than notice. + */ +const RESOLUTION_KIND_RENDER_RANK: Record = { + "blocker-cancelled-edge-stuck": 0, + "interaction-abandoned": 1, + "approval-abandoned": 2, + "approval-granted": 3, + "blocker-done-row-not-moved": 4, + "approval-refused": 5, + "interaction-answered": 6, +}; + +/** Every resolution kind, in render order. Total by construction. */ +function resolutionKindRenderOrder(): GateResolutionKind[] { + return (Object.keys(RESOLUTION_KIND_RENDER_RANK) as GateResolutionKind[]).sort( + (a, b) => RESOLUTION_KIND_RENDER_RANK[a] - RESOLUTION_KIND_RENDER_RANK[b], + ); +} + /** * One line per `unverifiable` reason, phrased as what a reader should conclude. * @@ -1056,27 +1110,39 @@ export function formatGateRevalidationSections( "Each row below had its gate re-tested and the gate is no longer live. That is *not* the same as 'no longer waiting': where the resolution left an action owed, the row is marked ⛔ and still appears in the age-ranked list. Only the unmarked rows are withheld from it.", ); - // Order by resolution kind so the ones that cannot self-clear lead. - const kindOrder: GateResolutionKind[] = [ - ...NON_SELF_CLEARING_RESOLUTION_KINDS, - "approval-granted", - "blocker-done-row-not-moved", - "approval-refused", - "interaction-answered", - ]; + // The rendered disposition is read from the *same* set that drives the + // exclusion filter, rather than recomputed from the elected `resolutionKind` + // (PEN-3089). Those two inputs disagree: escalation reads every probe on the + // row, while the election picks one kind to file the row under, and + // `approval-granted` is the one action-owed kind that can lose that election + // (it is absent from `NON_SELF_CLEARING_RESOLUTION_KINDS`, so it only wins by + // being `resolved[0]`, and `probeBlockerPremise` runs first). A row with + // every blocker `done` plus one granted card is therefore filed under + // `blocker-done-row-not-moved` while being escalated — and labelling it from + // its kind printed "withheld from the age-ranked list" over a row that was in + // that list. Deriving from `withheld` makes the label unable to contradict + // the filter by construction, which is the whole point of this module. + const withheld = withheldFromAgeRankingIssueIds(report); + const isEscalated = (classification: GateClassification): boolean => + !withheld.has(classification.issueId); let listed = 0; - for (const kind of kindOrder) { + for (const kind of resolutionKindRenderOrder()) { const inKind = resolved.filter((classification) => classification.resolutionKind === kind); if (inKind.length === 0) continue; - // Stated per kind rather than once at the top: the escalation consequence - // is a property of the kind, and a reader scanning one block should not - // have to hold the legend in their head to know whether these rows reached - // the list below. - const disposition = ACTION_OWED_RESOLUTION_KINDS.has(kind) - ? "⛔ still escalated — an action is owed" - : "withheld from the age-ranked list"; + // The heading is now a grouping label only. Its disposition summary is a + // tally of the per-row verdicts below, so a block that is genuinely mixed + // says so instead of asserting one disposition over rows that do not share + // it. A reader scanning one block still learns the consequence without + // holding the legend in their head. + const escalatedCount = inKind.filter(isEscalated).length; + const disposition = + escalatedCount === inKind.length + ? "⛔ still escalated — an action is owed" + : escalatedCount === 0 + ? "withheld from the age-ranked list" + : `⛔ ${escalatedCount} still escalated · ${inKind.length - escalatedCount} withheld from the age-ranked list`; body.push("", `**${RESOLUTION_KIND_HEADINGS[kind]} — ${inKind.length}** (${disposition})`); for (const classification of inKind) { if (listed >= maxListed) break; @@ -1086,7 +1152,11 @@ export function formatGateRevalidationSections( ); const ageDays = ages?.get(classification.issueId); const age = typeof ageDays === "number" ? ` (${ageDays.toFixed(1)}d silent)` : ""; - body.push(`- ${ref}${age} — ${boundEvidence(classification.evidence)}`); + // Per-row marker, as the section legend above already promises. The + // heading tally can be scanned; this is what makes an individual row + // unambiguous when the block is mixed. + const mark = isEscalated(classification) ? "⛔ " : ""; + body.push(`- ${mark}${ref}${age} — ${boundEvidence(classification.evidence)}`); listed += 1; } } From 64a4461a00d2477e05eb44526cb2fd7a6d6565ac Mon Sep 17 00:00:00 2001 From: Security Engineer Date: Wed, 16 Sep 2026 17:12:33 +0000 Subject: [PATCH 3/5] fix(digest): stop a sibling refusal masking an ask that died unanswered (PEN-3089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two rendered claims were wider than the code computing them. `probeApprovalGate` tested `rejected` before `withdrawn`/`cancelled`, so a single refused card classified the whole row `approval-refused` — not action-owed, therefore withheld from the age-ranked list — while a sibling card the requester had retracted went dark without its own state changing. That is the exact suppression this ticket exists to remove, re-entered through a multi-card row, and multi-card rows are normal on this seam (a resubmit after `revision_requested`, a moot card withdrawn beside a live one, a refused ask followed by a re-ask). A refusal answers its own ask and says nothing about ask B, so `approval-abandoned` now ranks ahead of `approval-refused`. This stays a narrowing: refusal still withholds the row when refusal is the row's whole approval story, which is also what makes that branch's "all N were answered" evidence true rather than merely usually true. Mixed rows now name the split instead of claiming every card was abandoned. The section legend claimed a ⛔ row "still appears in the age-ranked list", but not-withheld is only the first of three filters: `selectAgedHumanGatedIssues` then drops anything under its per-priority silence threshold and caps the result at `DEFAULT_MAX_ESCALATED`. `loadHumanGatedIssues` applies no age predicate at all, so most rows in this section are younger than their threshold and most ⛔ marks carried a false claim. The label now asserts exactly what `withheld` decides — not withheld — which keeps the "label cannot contradict the filter" property without plumbing the over-threshold ids back out of the ageing pass to serve a label. Also: the per-kind cap check is hoisted so a heading cannot push a disposition tally over rows the reader cannot see, and the granted evidence's verb agrees with the granted count rather than the card count. Tests: a `withdrawn` + `rejected` row must classify abandoned (mutation check: requiring every card be abandoned fails this test and only this test), and a refusal-only row must still be withheld. Signed-off-by: Security Engineer --- ...man-gated-gate-revalidation-wiring.test.ts | 2 +- .../human-gated-gate-revalidation.test.ts | 47 ++++++++- .../services/human-gated-gate-revalidation.ts | 97 ++++++++++++++----- 3 files changed, 118 insertions(+), 28 deletions(-) diff --git a/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts b/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts index d8143beed7cd..ea0dd458d5c9 100644 --- a/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts @@ -382,7 +382,7 @@ describeEmbeddedPostgres("gate re-validation (wired into the digest producer)", // Still rendered in the resolved section too, carrying its age — being // escalated must not cost the reader the diagnosis of *why* it is stalled. expect(markdown).toContain("GRG-1 (41.0d silent)"); - expect(markdown).toContain("⛔ still escalated"); + expect(markdown).toContain("⛔ action owed"); }); it("escalates a row whose only board card the requester withdrew (PEN-3089)", async () => { diff --git a/server/src/__tests__/human-gated-gate-revalidation.test.ts b/server/src/__tests__/human-gated-gate-revalidation.test.ts index 6fe7b5000427..167e3acbefdc 100644 --- a/server/src/__tests__/human-gated-gate-revalidation.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation.test.ts @@ -168,6 +168,49 @@ describe("probeApprovalGate", () => { expect(result?.evidence).toContain("a2=approved"); }); + it("does not let a sibling refusal mask an abandoned card (PEN-3089)", () => { + // The ticket's suppression, re-entered through a multi-card row. `rejected` + // used to be tested before `withdrawn`, so one refused card classified the + // whole row `approval-refused` — not action-owed, therefore withheld — and + // the withdrawn ask went dark without its own state changing. A refusal + // answers its own ask and says nothing about a card the requester retracted. + const result = probeApprovalGate( + evidence({ + approvals: [ + { approvalId: "a1", approvalStatus: "rejected" }, + { approvalId: "a2", approvalStatus: "withdrawn" }, + ], + }), + ); + expect(result?.resolutionKind).toBe("approval-abandoned"); + // The evidence leads with the card that died, and says so without claiming + // the refused sibling was also abandoned. + expect(result?.evidence).toContain("a2=withdrawn"); + expect(result?.evidence).toContain("1 of 2 linked approvals was withdrawn or cancelled"); + expect(result?.evidence).toContain("remaining 1 refused"); + expect(result?.evidence).toContain("re-ask"); + }); + + it("still withholds a row whose whole approval story is refusal (PEN-3089)", () => { + // The guard on the reorder above: making abandonment unmaskable must stay a + // narrowing. With no abandoned card the row is still `approval-refused`, and + // the evidence's "all N were answered" claim is true precisely because the + // abandoned branch has already returned by the time this one is reached. + const report = revalidateGates([ + evidence({ + issueId: "refused-only", + approvals: [ + { approvalId: "a1", approvalStatus: "rejected" }, + { approvalId: "a2", approvalStatus: "rejected" }, + ], + }), + ]); + const [classification] = report.classifications; + expect(classification?.resolutionKind).toBe("approval-refused"); + expect(classification?.evidence).toContain("all 2 linked approvals were answered"); + expect(withheldFromAgeRankingIssueIds(report).has("refused-only")).toBe(true); + }); + it("reads an unrecognised status as still-gated, not as a resolution (PEN-3089)", () => { // `approvals.status` is a plain text column, so a new status is reachable. // Property 2: a false `still-gated` ages a row one more week, a false @@ -694,7 +737,7 @@ describe("formatGateRevalidationSections", () => { ]); const markdown = formatGateRevalidationSections(report); expect(markdown).not.toContain("these are not still waiting"); - expect(markdown).toContain("authorised, unperformed — 1** (⛔ still escalated"); + expect(markdown).toContain("authorised, unperformed — 1** (⛔ action owed"); expect(markdown).toContain("needs closing, not re-asking — 1** (withheld"); }); @@ -722,7 +765,7 @@ describe("formatGateRevalidationSections", () => { // One heading, holding rows with opposite dispositions: it must report the // split rather than assert either disposition over both. - expect(markdown).toContain("never moved — 2** (⛔ 1 still escalated · 1 withheld"); + expect(markdown).toContain("never moved — 2** (⛔ 1 action owed · 1 withheld"); // The escalated row is marked; the withheld one is not. expect(markdown).toContain("- ⛔ PEN-3000"); expect(markdown).toContain("- PEN-3001"); diff --git a/server/src/services/human-gated-gate-revalidation.ts b/server/src/services/human-gated-gate-revalidation.ts index 56f2ee7ee932..cdd2fc82d096 100644 --- a/server/src/services/human-gated-gate-revalidation.ts +++ b/server/src/services/human-gated-gate-revalidation.ts @@ -129,9 +129,11 @@ export type GateProbeKind = * *into* is an instruction to perform, so the row is now authorised and * unperformed. See {@link ACTION_OWED_RESOLUTION_KINDS} for why that is kept * in the escalation list rather than withheld from it. - * - `approval-refused` is a row whose board gate closed with a `rejected` card - * and no grant. The ask was answered "no"; nothing further is owed by the - * gate and the row needs closing, not escalating. + * - `approval-refused` is a row whose board gate closed with a `rejected` card, + * no grant, and no abandoned sibling — refusal is the row's whole approval + * story. The ask was answered "no"; nothing further is owed by the gate and + * the row needs closing, not escalating. A refusal alongside a withdrawn card + * is `approval-abandoned` instead: answering ask A does not answer ask B. * - `interaction-answered` is a row where at least one question card got a real * human decision (accepted / rejected / answered) and which is still open * anyway. Deliberately *not* "every card": a row whose remaining cards were @@ -441,13 +443,27 @@ export function probeBlockerPremise(input: GateEvidenceInput): ProbeResult | nul * - any card still undecided, **or carrying a status this module does not * recognise** → `still-gated`; * - otherwise, any card `approved` → `approval-granted`; - * - otherwise, any card `rejected` → `approval-refused`; - * - otherwise every card was withdrawn or cancelled → the board was asked and - * never answered (`approval-abandoned`). + * - otherwise, any card `withdrawn`/`cancelled` → the board was asked and never + * answered (`approval-abandoned`); + * - otherwise every card was `rejected` → `approval-refused`. * - * Grant beats refusal on a mixed row for the same reason unknown beats - * everything: a single live authorisation means work is owed, and the - * escalation surface must not lose it behind a sibling refusal. + * Grant beats everything below it for the same reason unknown beats everything: + * a single live authorisation means work is owed, and the escalation surface + * must not lose it behind a sibling that resolved into no obligation. + * + * Abandonment beats refusal for the narrower version of that argument, and that + * ordering is load-bearing. A refusal answers *its own* ask and nothing else; it + * says nothing about a sibling card the requester retracted. Testing `rejected` + * first let a single refused card mask every withdrawn card on the row, classify + * it `approval-refused` — not action-owed — and withhold it: the exact + * suppression PEN-3089 exists to remove, re-entered through a multi-card row. + * Firing `approval-abandoned` only when *every* card was abandoned would make it + * the easiest kind to mask, and multi-card rows are normal on this seam (a + * resubmit after `revision_requested`, a moot card withdrawn beside a live one, + * a refused ask followed by a re-ask). + * + * This stays a narrowing rather than an "escalate everything": refusal still + * withholds the row when refusal is the row's whole approval story. * * The unknown-status branch is property 2 (fail toward `still-gated`) applied * to schema drift. `approvals.status` is a plain `text` column, and before @@ -458,6 +474,9 @@ export function probeBlockerPremise(input: GateEvidenceInput): ProbeResult | nul export function probeApprovalGate(input: GateEvidenceInput): ProbeResult | null { if (input.approvals.length === 0) return null; + const total = input.approvals.length; + const cards = total === 1 ? "" : "s"; + // Two reasons a gate reads as live, kept apart because they mean different // things to whoever reads the digest: a card nobody has decided yet, and a // card carrying a status this module has never heard of. Both fail toward @@ -477,8 +496,7 @@ export function probeApprovalGate(input: GateEvidenceInput): ProbeResult | null ); if (undecided.length > 0 || unrecognised.length > 0) { - const total = input.approvals.length; - const plural = total === 1 ? "" : "s"; + const plural = cards; const clauses: string[] = []; if (undecided.length > 0) { clauses.push( @@ -506,31 +524,43 @@ export function probeApprovalGate(input: GateEvidenceInput): ProbeResult | null probe: "approval-gate", verdict: "resolved-but-open", resolutionKind: "approval-granted", - evidence: `${granted.map(describeApproval).join(", ")} — ${granted.length} of ${input.approvals.length} linked approval${input.approvals.length === 1 ? " was" : "s were"} granted and the row has not moved since, so it is authorised and unperformed: the gate opened and whoever the row is assigned to still owes the work`, + evidence: `${granted.map(describeApproval).join(", ")} — ${granted.length} of ${total} linked approval${cards} ${granted.length === 1 ? "was" : "were"} granted and the row has not moved since, so it is authorised and unperformed: the gate opened and whoever the row is assigned to still owes the work`, }; } - const refused = input.approvals.filter((approval) => - APPROVAL_REFUSED.has(approval.approvalStatus), + const abandoned = input.approvals.filter((approval) => + APPROVAL_ABANDONED.has(approval.approvalStatus), ); - if (refused.length > 0) { + // Ahead of the refusal branch, so a sibling `rejected` card cannot mask an ask + // that died unanswered — see the ordering argument in the docblock. Every card + // still in play here is refused or abandoned, so the non-abandoned remainder + // on a mixed row is exactly the refused set and can be named as such. + if (abandoned.length > 0) { + // The card refs lead, as they do in the cancelled-blocker and abandoned- + // interaction branches: the rendered evidence is length-bounded, and *which* + // ask died is the only part a reader can act on. On a mixed row that means + // the abandoned refs specifically, not every card. + const scope = + abandoned.length === total + ? `all ${total} linked approval${cards} ${total === 1 ? "was" : "were"} withdrawn or cancelled` + : `${abandoned.length} of ${total} linked approvals ${abandoned.length === 1 ? "was" : "were"} withdrawn or cancelled and the remaining ${total - abandoned.length} refused`; return { probe: "approval-gate", verdict: "resolved-but-open", - resolutionKind: "approval-refused", - evidence: `all ${input.approvals.length} linked approval${input.approvals.length === 1 ? " was" : "s were"} answered and none was granted: ${input.approvals.map(describeApproval).join(", ")} — the ask was refused, so this row needs closing rather than re-asking`, + resolutionKind: "approval-abandoned", + evidence: `${abandoned.map(describeApproval).join(", ")} — ${scope}, so the board was asked and never answered and no answer is coming; someone must re-ask or drop the row`, }; } - // The card refs lead, as they do in the cancelled-blocker and abandoned- - // interaction branches: the rendered evidence is length-bounded, and *which* - // ask died is the only part a reader can act on. + // Terminal: the undecided/unrecognised guard, the granted branch and the + // abandoned branch have each returned, so every card is `rejected` — which is + // what lets this evidence say "all were answered" without qualification. return { probe: "approval-gate", verdict: "resolved-but-open", - resolutionKind: "approval-abandoned", - evidence: `${input.approvals.map(describeApproval).join(", ")} — all ${input.approvals.length} linked approval${input.approvals.length === 1 ? " was" : "s were"} withdrawn or cancelled, so the board was asked and never answered and no answer is coming; someone must re-ask or drop the row`, + resolutionKind: "approval-refused", + evidence: `all ${total} linked approval${cards} ${total === 1 ? "was" : "were"} answered and none was granted: ${input.approvals.map(describeApproval).join(", ")} — the ask was refused, so this row needs closing rather than re-asking`, }; } @@ -1107,7 +1137,7 @@ export function formatGateRevalidationSections( "", `#### Gate resolved but row still open — ${resolved.length}`, "", - "Each row below had its gate re-tested and the gate is no longer live. That is *not* the same as 'no longer waiting': where the resolution left an action owed, the row is marked ⛔ and still appears in the age-ranked list. Only the unmarked rows are withheld from it.", + "Each row below had its gate re-tested and the gate is no longer live. That is *not* the same as 'no longer waiting': where the resolution left an action owed, the row is marked ⛔ and is **not** withheld from the age-ranked list — it escalates there once it passes its human-silence threshold. Only the unmarked rows are withheld outright.", ); // The rendered disposition is read from the *same* set that drives the @@ -1122,12 +1152,29 @@ export function formatGateRevalidationSections( // its kind printed "withheld from the age-ranked list" over a row that was in // that list. Deriving from `withheld` makes the label unable to contradict // the filter by construction, which is the whole point of this module. + // + // What the label therefore claims is exactly what `withheld` decides — + // *not withheld*, rather than *listed*. Membership is two further filters + // downstream and neither is visible here: `selectAgedHumanGatedIssues` drops + // anything under its per-priority silence threshold (14d critical/high, 30d + // medium, 45d low/unset) and then caps the list at `DEFAULT_MAX_ESCALATED`. + // Since `loadHumanGatedIssues` applies no age predicate at all, most rows in + // this section are younger than their threshold, so "still appears in the + // age-ranked list" would be false on most ⛔ marks. Claiming membership + // honestly would mean plumbing the over-threshold ids back out of the ageing + // pass — a new seam, for a label; claiming non-withholding needs no seam and + // keeps the property that the label cannot contradict the filter. const withheld = withheldFromAgeRankingIssueIds(report); const isEscalated = (classification: GateClassification): boolean => !withheld.has(classification.issueId); let listed = 0; for (const kind of resolutionKindRenderOrder()) { + // Checked before the heading, not just before each row: a heading pushed + // after the cap is spent would assert a disposition tally over rows the + // reader cannot see, ahead of the "... N further omitted" line that is + // supposed to account for them. + if (listed >= maxListed) break; const inKind = resolved.filter((classification) => classification.resolutionKind === kind); if (inKind.length === 0) continue; @@ -1139,10 +1186,10 @@ export function formatGateRevalidationSections( const escalatedCount = inKind.filter(isEscalated).length; const disposition = escalatedCount === inKind.length - ? "⛔ still escalated — an action is owed" + ? "⛔ action owed — not withheld from the age-ranked list" : escalatedCount === 0 ? "withheld from the age-ranked list" - : `⛔ ${escalatedCount} still escalated · ${inKind.length - escalatedCount} withheld from the age-ranked list`; + : `⛔ ${escalatedCount} action owed · ${inKind.length - escalatedCount} withheld from the age-ranked list`; body.push("", `**${RESOLUTION_KIND_HEADINGS[kind]} — ${inKind.length}** (${disposition})`); for (const classification of inKind) { if (listed >= maxListed) break; From f2bc6bd70a7745993fccb123077fad81e375d26b Mon Sep 17 00:00:00 2001 From: Cto Date: Thu, 17 Sep 2026 04:39:30 +0000 Subject: [PATCH 4/5] fix(digest): stop the approval-abandoned heading claiming every card was withdrawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PEN-3089 reorder that put the abandoned branch ahead of the refusal branch is what stops a sibling `rejected` card masking a retracted ask. It also cost the kind its terminal property: `approval-abandoned` now fires on *at least one* abandoned card, so a mixed row reaches the heading with refused cards still on it. Two strings were left asserting the old, terminal claim: - `RESOLUTION_KIND_HEADINGS["approval-abandoned"]`, which read "Every board card was withdrawn or cancelled". On the very mixed row the reorder exists to surface, that heading contradicted the evidence line printed directly beneath it ("...and the remaining 1 refused"). - the `GateResolutionKind` docblock, which described the kind as "every linked card was `withdrawn` ... or `cancelled`" — contradicting both the sibling note fifteen lines below it ("A refusal alongside a withdrawn card is `approval-abandoned` instead") and the ordering argument in `probeApprovalGate`. Both now state what the branch actually guarantees. The remainder on a mixed row is exactly the refused set, because the granted, undecided and unrecognised branches have each already returned, so the second clause names it rather than hedging. `interaction-abandoned` keeps its "Every" wording: it is its probe's fall-through and so is genuinely terminal. The docblock now records that asymmetry, since the two kinds are otherwise described as twins and the next reader would reasonably assume the headings should match. Follows the in-file precedent set by `interaction-answered`, whose heading was weakened for the same reason and carries the same kind of comment against re-strengthening. Tests: adds a render-level regression asserting the mixed refused/withdrawn row is not headed as if every card was withdrawn, and that heading, evidence and the escalation filter agree on it. Mutating the heading back to the old wording fails that test and only that test (1 failed | 75 passed). Signed-off-by: Cto --- ...man-gated-gate-revalidation-wiring.test.ts | 2 +- .../human-gated-gate-revalidation.test.ts | 30 ++++++++++++++++++ .../services/human-gated-gate-revalidation.ts | 31 ++++++++++++++----- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts b/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts index ea0dd458d5c9..887a564af53c 100644 --- a/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts @@ -401,7 +401,7 @@ describeEmbeddedPostgres("gate re-validation (wired into the digest producer)", const markdown = (await collect(companyId))!.markdown; expect(markdown).toContain( - "Every board card was withdrawn or cancelled — the board was asked and never answered", + "At least one board card was withdrawn or cancelled — that ask died unanswered", ); expect(markdown).toContain("someone must re-ask or drop the row"); expect(markdown).toContain("Human-gated work past its human-silence threshold (1)"); diff --git a/server/src/__tests__/human-gated-gate-revalidation.test.ts b/server/src/__tests__/human-gated-gate-revalidation.test.ts index 167e3acbefdc..32b02e9d0618 100644 --- a/server/src/__tests__/human-gated-gate-revalidation.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation.test.ts @@ -787,6 +787,36 @@ describe("formatGateRevalidationSections", () => { expect(markdown.indexOf("cancelled")).toBeLessThan(markdown.indexOf("never moved")); }); + it("does not head a mixed refused/withdrawn row as if every card was withdrawn", () => { + // The sibling of the `interaction-answered` case below, and a direct + // consequence of the PEN-3089 reorder: moving the abandoned branch ahead of + // the refusal branch is what stops a `rejected` card masking a retracted + // ask, and it is also what makes this kind non-terminal. It now fires on + // *at least one* abandoned card, so this row — one refused, one withdrawn — + // lands under the heading with a refused card still on it. The heading must + // not claim every card was withdrawn; the evidence line directly beneath it + // says "the remaining 1 refused", and the two cannot disagree about the + // same row. + const report = revalidateGates([ + evidence({ + issueId: "mixed", + identifier: "PEN-2224", + approvals: [ + { approvalId: "a1", approvalStatus: "rejected" }, + { approvalId: "a2", approvalStatus: "withdrawn" }, + ], + }), + ]); + const markdown = formatGateRevalidationSections(report); + expect(markdown).toContain("At least one board card was withdrawn or cancelled"); + expect(markdown).not.toContain("Every board card was withdrawn"); + // Heading and evidence must agree: the refused sibling is still reported, + // and the row is still action-owed because the retracted ask needs re-asking. + expect(markdown).toContain("the remaining 1 refused"); + expect(markdown).toContain("a2=withdrawn"); + expect(withheldFromAgeRankingIssueIds(report).has("mixed")).toBe(false); + }); + it("does not head a mixed answered/expired row as if every card was answered", () => { // The `interaction-answered` kind is assigned whenever *at least one* card // got a decision, so this row — one answered, one expired — lands under diff --git a/server/src/services/human-gated-gate-revalidation.ts b/server/src/services/human-gated-gate-revalidation.ts index cdd2fc82d096..5d4426e5ae3c 100644 --- a/server/src/services/human-gated-gate-revalidation.ts +++ b/server/src/services/human-gated-gate-revalidation.ts @@ -116,12 +116,18 @@ export type GateProbeKind = * human" while the thing it was waiting on no longer exists. Like a stuck * edge it cannot self-clear — someone has to re-ask or drop the row. * - `approval-abandoned` (PEN-3089) is the approval-side twin of - * `interaction-abandoned`: every linked card was `withdrawn` by its own - * requester or `cancelled`, so the board never answered and no answer is - * coming. Before PEN-3089 these rows were reported as `approval-decided` — - * identically to an `approved` card — which is how PEN-2224, the root blocker - * of a critical credential-exposure chain, spent 26 days inside a section - * headed "these are not still waiting". + * `interaction-abandoned`: at least one linked card was `withdrawn` by its + * own requester or `cancelled`, so that ask never got a board answer and no + * answer is coming. Deliberately *not* "every card", and this is where the + * twin stops being symmetric — `interaction-abandoned` is its probe's + * fall-through and so is terminal, whereas this kind is assigned *ahead* of + * `approval-refused` so a sibling refusal cannot mask a retracted ask (see + * {@link probeApprovalGate} for that ordering argument). A mixed row + * therefore lands here with refused cards on it; the heading and evidence + * line both say so. Before PEN-3089 these rows were reported as + * `approval-decided` — identically to an `approved` card — which is how + * PEN-2224, the root blocker of a critical credential-exposure chain, spent + * 26 days inside a section headed "these are not still waiting". * - `blocker-done-row-not-moved` is a row whose blockers all completed; the * platform already considers it dependency-ready and it is merely still open. * - `approval-granted` is a row whose board gate opened: at least one linked @@ -940,8 +946,19 @@ const RESOLUTION_KIND_HEADINGS: Record = { "Blocker edge is cancelled — permanently un-checkoutable until an operator clears it", "interaction-abandoned": "Every question card was withdrawn or expired — the human was asked and never answered", + // Not "every card": unlike `interaction-abandoned` — which is its probe's + // fall-through and so is terminal — this kind is assigned ahead of the + // refusal branch, precisely so a sibling `rejected` card cannot mask an ask + // the requester retracted. That reorder is what makes the kind correct and + // is also what costs it the "every" claim: the branch fires on *at least + // one* abandoned card, so a mixed row reaches this heading with refused + // cards on it. The remainder is exactly the refused set — `granted`, + // undecided and unrecognised have each already returned — which is why the + // second clause can name it rather than hedge. Claiming "every" here would + // contradict the evidence line printed directly beneath it ("…and the + // remaining N refused") on the very rows the reorder exists to surface. "approval-abandoned": - "Every board card was withdrawn or cancelled — the board was asked and never answered", + "At least one board card was withdrawn or cancelled — that ask died unanswered; any remaining cards were refused", "blocker-done-row-not-moved": "Every blocker is done — the row simply never moved", "approval-granted": "The board granted the ask and the row has not moved since — authorised, unperformed", From 0f5444106848d4cecf60d513dc4df27691a54df9 Mon Sep 17 00:00:00 2001 From: Cto Date: Sat, 19 Sep 2026 00:18:25 +0000 Subject: [PATCH 5/5] fix(digest): stop the backfill legend claiming every board card was withdrawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ally review 5233404050 (head be38034a4), Important 1 — carried from the fd8a130 review as still-present at its second site. `approval-abandoned` is assigned on `abandoned.length > 0`, ahead of the refusal branch, so a mixed row lands under it with refused cards still on it. The service-file heading was corrected for this in be38034a4; the backfill legend at :386 still read `every board card withdrawn/cancelled` and so kept the terminal claim the reorder had made false. Takes the reviewer's first suggested label rather than its alternative: `at least one board card withdrawn/cancelled` is 43 characters and the legend column is padded to 42, so that phrasing would have broken the alignment the same recommendation asked to preserve. The review's proposed drift guard — deriving both labels from a shared constant — rested on "no test file anywhere in the tree", which is not the case: server/src/__tests__/human-gated-gate-revalidation-backfill.ts already imports and exercises `renderReport`. Guarding it there instead pins the label and its padding directly, without coupling a 110-char heading sentence to a 42-char column label. Verified by positive control: the assertion fails on the old label and passes on the new one. Also takes both Suggestions: - inline the `plural` alias of `cards` (:505) - pin that a cap landing mid-block tallies its heading over the whole kind rather than the printed subset, which nothing exercised Refs PEN-3089, PEN-3330 Signed-off-by: Cto --- .../blo-30608-gate-revalidation-backfill.ts | 8 +++- ...n-gated-gate-revalidation-backfill.test.ts | 29 +++++++++++++ .../human-gated-gate-revalidation.test.ts | 43 +++++++++++++++++++ .../services/human-gated-gate-revalidation.ts | 5 +-- 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/scripts/blo-30608-gate-revalidation-backfill.ts b/scripts/blo-30608-gate-revalidation-backfill.ts index 715f062a39d9..9ef99508b5be 100644 --- a/scripts/blo-30608-gate-revalidation-backfill.ts +++ b/scripts/blo-30608-gate-revalidation-backfill.ts @@ -383,7 +383,13 @@ export function renderReport( "resolved-but-open by who can clear it:", ` blocker edge cancelled (never self-clears) : ${report.countsByResolutionKind["blocker-cancelled-edge-stuck"]}`, ` every question card withdrawn/expired : ${report.countsByResolutionKind["interaction-abandoned"]}`, - ` every board card withdrawn/cancelled : ${report.countsByResolutionKind["approval-abandoned"]}`, + // "a", not "every": `approval-abandoned` is assigned ahead of the refusal + // branch, on *at least one* abandoned card, so a mixed row is counted here + // with refused cards still on it. Deliberately asymmetric with the line + // above — `interaction-abandoned` is its probe's fall-through and so is + // genuinely terminal. See RESOLUTION_KIND_HEADINGS in + // human-gated-gate-revalidation.ts, which carries the full argument. + ` a board card withdrawn/cancelled : ${report.countsByResolutionKind["approval-abandoned"]}`, ` all blockers done, row never moved : ${report.countsByResolutionKind["blocker-done-row-not-moved"]}`, ` board granted the ask, row unperformed : ${report.countsByResolutionKind["approval-granted"]}`, ` board refused the ask : ${report.countsByResolutionKind["approval-refused"]}`, diff --git a/server/src/__tests__/human-gated-gate-revalidation-backfill.test.ts b/server/src/__tests__/human-gated-gate-revalidation-backfill.test.ts index 93548c0574bc..f676dbaffbf5 100644 --- a/server/src/__tests__/human-gated-gate-revalidation-backfill.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation-backfill.test.ts @@ -168,6 +168,35 @@ describe("BLO-30608 backfill — API acquisition", () => { expect(rendered).toContain("Probed : 2 (3 beyond the budget)"); }); + // The legend label for `approval-abandoned` must not claim "every". The kind + // is assigned on *at least one* abandoned card, ahead of the refusal branch, + // so a mixed row is counted under it with refused cards still on it. The + // service-file heading for this same kind is pinned in + // human-gated-gate-revalidation.test.ts; this is the second site carrying the + // claim, and it drifted out of step with the first once already (PEN-3089). + // Asserted with the padding, because the label sits in a fixed-width column + // that nothing else exercises. + it("does not claim every board card was withdrawn in the resolution legend", async () => { + stub = stubApi({ blocked: humanGatedRows() }); + + const acquisition = await acquireFromApi(COMPANY_ID, null, NOW); + const report = revalidateGates(acquisition.evidence, {}); + const rendered = renderReport(report, { + population: acquisition.population, + calls: acquisition.calls, + elapsedMs: 1_000, + source: "api", + notProbed: 0, + }); + + expect(rendered).toContain(" a board card withdrawn/cancelled : "); + expect(rendered).not.toContain("every board card"); + // Deliberately asymmetric with the line above it: `interaction-abandoned` + // is its probe's fall-through and so is genuinely terminal, which is what + // entitles that one to "every". + expect(rendered).toContain(" every question card withdrawn/expired : "); + }); + it("excludes agent-owned, hidden, and digest rows from the population", async () => { stub = stubApi({ blocked: [...humanGatedRows(), ...excludedRows()] }); diff --git a/server/src/__tests__/human-gated-gate-revalidation.test.ts b/server/src/__tests__/human-gated-gate-revalidation.test.ts index 32b02e9d0618..7835c10b1451 100644 --- a/server/src/__tests__/human-gated-gate-revalidation.test.ts +++ b/server/src/__tests__/human-gated-gate-revalidation.test.ts @@ -919,4 +919,47 @@ describe("formatGateRevalidationSections", () => { const markdown = formatGateRevalidationSections(revalidateGates(inputs), { maxListed: 2 }); expect(markdown).toContain("3 further resolved-but-open rows omitted"); }); + + it("tallies a capped heading over the whole kind, not the rows that fit", () => { + // The cap is checked before the heading, so a block that is cut mid-way + // still heads with a tally over every row of its kind. That is deliberate + // — the "... N further omitted" line accounts for the difference — but + // nothing exercised `maxListed` against a *mixed* block, where the + // disposition split is what a reader would otherwise mis-scan as covering + // only the printed rows. Four rows elect `blocker-done-row-not-moved`; the + // two carrying a granted card stay escalated, and the cap prints two. + const report = revalidateGates([ + evidence({ + issueId: "esc-1", + identifier: "PEN-4000", + blockers: [{ blockerIssueId: "b", blockerStatus: "done" }], + approvals: [{ approvalId: "a1", approvalStatus: "approved" }], + }), + evidence({ + issueId: "esc-2", + identifier: "PEN-4001", + blockers: [{ blockerIssueId: "c", blockerStatus: "done" }], + approvals: [{ approvalId: "a2", approvalStatus: "approved" }], + }), + evidence({ + issueId: "wit-1", + identifier: "PEN-4002", + blockers: [{ blockerIssueId: "d", blockerStatus: "done" }], + }), + evidence({ + issueId: "wit-2", + identifier: "PEN-4003", + blockers: [{ blockerIssueId: "e", blockerStatus: "done" }], + }), + ]); + const markdown = formatGateRevalidationSections(report, { maxListed: 2 }); + + // Tally spans all four, including the two rows below the cut. + expect(markdown).toContain("never moved — 4** (⛔ 2 action owed · 2 withheld"); + // ...and only two rows actually print, with the remainder accounted for. + expect(markdown).toContain("- ⛔ PEN-4000"); + expect(markdown).toContain("- ⛔ PEN-4001"); + expect(markdown).not.toContain("PEN-4002"); + expect(markdown).toContain("2 further resolved-but-open rows omitted"); + }); }); diff --git a/server/src/services/human-gated-gate-revalidation.ts b/server/src/services/human-gated-gate-revalidation.ts index 5d4426e5ae3c..9e76d0367ac9 100644 --- a/server/src/services/human-gated-gate-revalidation.ts +++ b/server/src/services/human-gated-gate-revalidation.ts @@ -502,16 +502,15 @@ export function probeApprovalGate(input: GateEvidenceInput): ProbeResult | null ); if (undecided.length > 0 || unrecognised.length > 0) { - const plural = cards; const clauses: string[] = []; if (undecided.length > 0) { clauses.push( - `${undecided.length} of ${total} linked approval${plural} still undecided: ${undecided.map(describeApproval).join(", ")}`, + `${undecided.length} of ${total} linked approval${cards} still undecided: ${undecided.map(describeApproval).join(", ")}`, ); } if (unrecognised.length > 0) { clauses.push( - `${unrecognised.length} of ${total} linked approval${plural} carr${unrecognised.length === 1 ? "ies" : "y"} a status this module does not recognise, so the gate is read as live rather than resolved: ${unrecognised.map(describeApproval).join(", ")}`, + `${unrecognised.length} of ${total} linked approval${cards} carr${unrecognised.length === 1 ? "ies" : "y"} a status this module does not recognise, so the gate is read as live rather than resolved: ${unrecognised.map(describeApproval).join(", ")}`, ); } return {