From d603924961a6a13c51dcc6ecfda04dcf14aa13eb Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 29 Oct 2026 08:48:05 -0700 Subject: [PATCH 1/3] fix(contracts): report a level label in a labels step instead of failing the whole stored pack --- packages/triage-contracts/src/check-pack.test.ts | 8 +++++++- packages/triage-contracts/src/check-pack.ts | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/triage-contracts/src/check-pack.test.ts b/packages/triage-contracts/src/check-pack.test.ts index e41647409..aa69e1f5f 100644 --- a/packages/triage-contracts/src/check-pack.test.ts +++ b/packages/triage-contracts/src/check-pack.test.ts @@ -230,11 +230,17 @@ describe("check pack contract", () => { [{ ...ACTIONS, pass: [{ do: "comment", target: {} }] }, "Pass step 1 target must set exactly body."], [{ ...ACTIONS, pass: [{ do: "approve", for: "staff" }] }, "Pass step 1 for must be one of OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, FIRST_TIMER, NONE, MANNEQUIN, BOT."], [{ ...ACTIONS, fail: [{ kind: "comment", target: { body: "Hi" } }] }, "Fail step 1 must not set kind."], - [{ ...ACTIONS, pass: [{ do: "labels", target: { from: "list", labels: ["Risk: High"] } }] }, "Pass step 1 label Risk: High is one Triage takes back; name another."], ]; for (const [actions, reason] of rejected) expect(() => readCheckPack({ ...emptyPack(REPO), actions })).toThrow(reason); }); + test("a labels step holding a level label is left out and named, and the rest of the pack loads", () => { + const level = { do: "labels", target: { from: "list", labels: ["Risk: High"] } }; + const read = readCheckPack({ ...emptyPack(REPO), actions: { ...ACTIONS, pass: [level, COMMENT] } }); + expect(read.actions.pass).toEqual([COMMENT]); + expect(read.dropped).toEqual(["Pass step 1 label Risk: High is one Triage takes back; name another"]); + }); + test("the schema accepts what the parser reads, and a rejection says why", () => { const accepted = [ { kind: "corbits.triage.check-pack", schemaVersion: 2, repo: REPO }, diff --git a/packages/triage-contracts/src/check-pack.ts b/packages/triage-contracts/src/check-pack.ts index 9f466a4ba..5bbafd9e9 100644 --- a/packages/triage-contracts/src/check-pack.ts +++ b/packages/triage-contracts/src/check-pack.ts @@ -1283,6 +1283,19 @@ function withDropped(read: T, dropped: string[]): T & { droppe return { ...read, ...(dropped.length > 0 && { dropped }) }; } +/** A labels step naming a label Triage takes back would flip it across runs, so it is left out and named, and the rest of the pack still loads. */ +function withoutLevelLabelSteps(actions: PolicyActions, dropped: string[]): PolicyActions { + const kept = { ...actions }; + for (const list of VERDICTS) { + kept[list] = actions[list].filter(function keepStep(step, i) { + const level = step.do === "labels" && step.target.from === "list" ? step.target.labels.find((label) => levelLabelRule(label) !== undefined) : undefined; + if (level !== undefined) dropped.push(`${STEP_WHERE[list]} ${i + 1} label ${level} is one Triage takes back; name another`); + return level === undefined; + }); + } + return kept; +} + /** Reads a stored pack, throwing an Error whose message says why it is rejected. Version 1 reads as version 2. */ export function readCheckPack(raw: unknown, expectedRepo?: string): CheckPack { const stored = policyObject(raw); @@ -1293,7 +1306,7 @@ export function readCheckPack(raw: unknown, expectedRepo?: string): CheckPack { const repo = readRepo(body, expectedRepo); const checks = readChecks(body.checks); const custom = parseCustom(body.custom); - const actions = parseActions(body.actions, dropped); + const actions = withoutLevelLabelSteps(parseActions(body.actions, dropped), dropped); const labels = parseLabels(body.labels); const groups = parseGroups(body.groups); return withDropped(refuseTriageLabels({ kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION, repo, checks, custom, actions, labels, ...(groups && { groups }) }), dropped); From a7ba339d226435ceacbb213d57c1c4bcd1a49691 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 29 Oct 2026 08:54:39 -0700 Subject: [PATCH 2/3] fix(contracts): refuse a level label in a pushed file and name the skipped stored step by position --- apps/hub/src/github/repo-policy.ts | 2 +- packages/triage-contracts/README.md | 2 +- .../triage-contracts/src/check-pack.test.ts | 9 ++- packages/triage-contracts/src/check-pack.ts | 61 +++++++++---------- packages/triage-contracts/src/policy-file.ts | 2 +- .../triage-contracts/src/policy-rules.test.ts | 2 + 6 files changed, 41 insertions(+), 37 deletions(-) diff --git a/apps/hub/src/github/repo-policy.ts b/apps/hub/src/github/repo-policy.ts index 74bbe0b50..21a7c4a7d 100644 --- a/apps/hub/src/github/repo-policy.ts +++ b/apps/hub/src/github/repo-policy.ts @@ -45,7 +45,7 @@ async function fileResolved(deps: RepoPolicyDeps, tenantId: string, repo: string const shared = extendsOf(content); if (shared !== undefined) return overSharedFile(deps.policyFile, tenantId, repo, source, content, shared); refuseUnknownChecks(content); - return { pack: readCheckPack(content, repo) }; + return { pack: readCheckPack(content, repo, true) }; } /** Why triage would refuse a policy file holding `policy`, resolved as a pushed file is; undefined when it would run it. */ diff --git a/packages/triage-contracts/README.md b/packages/triage-contracts/README.md index cb8a9d7ca..2b404e711 100644 --- a/packages/triage-contracts/README.md +++ b/packages/triage-contracts/README.md @@ -11,7 +11,7 @@ and trigger mail. Every app and most packages import it. A policy sets which rules run on a repository's pull requests and what happens after. Its rules are stored under `checks` (built-in) and `custom`. `GET /api/integrations/pack-schema` returns its JSON Schema as `checkPack`, the part of a policy file that extends another repository's file as `policyOverride`, the repository settings as `repoPolicy`, and `.corbits-triage.yml` as `policyFile`: schema 3, a list of `rules` (each `text`, `check`, `reason` and `evidence`), `actions` (`on` and steps per verdict) and `labels`, which `readPolicyFile` maps onto the stored shape; `checkPolicyFile` answers the same or each problem with its line, and the file's v4 keys as `workflow`. Those keys (`policy-workflow.ts`) are `groups` (each `association`, `teams` or `users`), one key per block, `code-review` (`on`, and optionally `for`, `when: { triage, paths, rules }` and `provider`; a file setting the retired `clean` is refused with a pointer to a step's `when.findings`), with `security` and `deslop` reserved: `checkPolicyFile` reads a file without them and without any `decide` rule with a condition on one, names each as `pending`, and refuses the file as written if the rest is refused too; `patchPolicyFile` refuses a file still holding them unless the batch has `strip-reserved`, which removes them. The other v4 keys are `pace`, `pauseLabel` and `decide` (rules `{ if, do }`, `if` on `all`, `triage`, `event`, `author` and each block's outcome); they are validated and returned as written, and nothing acts on them yet except steps naming a group and, on GitHub, `pauseLabel`, whose label on a pull request stops the hub starting runs for it. A file that sets `extends` holds none of them. Groups are matched first match wins, in the file's order: an author is in at most one of the file's groups, then in each built-in group (`maintainers`, `external`, `first-timers`, `bots`) their author kind puts them in, which are facts matched independently, then in `everyone`, which always matches; a file group with a built-in group's name replaces it. `authorGroupsOf(groups, author)` answers that set, failing closed: it answers none, so no `{ group }` but `everyone` holds, when the author's kind is not known or teams that can't be read would decide their file group. A step's built-in group reads as author kinds, so it still holds when teams can't be read, and is not known only when the author's kind is. The teams read for an author are every team the policy names, in a step or a group (`policyTeams`). A step's `when.author` names groups as the policy holds them, `{ group }` for a file group or `everyone` and a built-in group's kinds otherwise, and holds when the author's set includes it; approving or merging must never rest on `everyone` alone. `{ all: clean }` holds when at least one block ran and every block that ran is clean; skipped blocks never count, so it fails when every block was skipped or none is on. A rule that approves or merges without `triage` matches only Pass (`ruleVerdicts`). Incomplete is never clean: a rule that approves or merges can't name `incomplete`. A file that sets `actions` and no `decide` gets its `decide` from `decideOf`: one rule `{ if: { triage: }, do: }` per verdict with steps, `do: nothing` for a Pass or Fail with none, and no rule for an Unsure with none, each limited to `event: ` when the actions wake on fewer than every event, every step as written; a file that extends another has no `decide`. A `decide` rule can't use `review` yet. A file setting any v4 key is read as schema 3 and needs `rules`. Problems name their line from the YAML parser's node positions (`yaml`, core schema, aliases and the keys `__proto__`, `constructor` and `prototype` refused); schema errors are said in the file's terms by `policy-file-problems.ts`. A file without `rules` or a v4 key is read as a policy or override without `kind`, `schemaVersion` and `repo`. Constraints the schema cannot state are checked when the policy is read: a custom rule's `options` and `failOn` must not repeat and `failOn` must be among `options`, a score's `failBelow` must be one of its levels after the first, and a custom id must not be a built-in rule's or read as a judge question's. A rule that fails any of these rejects the policy with its reason. Custom rules are decision-model questions (`kind: "model"`): `is-true`, `score` or `choose`, or text rules (`kind: "text"`) that fail when any of `fields` (`title`, `description`, `commits`) contains one of `phrases`, or with `failWhen: "not-contains"` when one contains none, ignoring case; a stored rule of the retired `kind: "rule"` rejects the policy. The reader ignores unknown built-in rows. Built-in rule ids renamed earlier (`size`, `issue`, `conflicts`, `ci`, `paths`, `draft`, `review`) are still read and mapped to their new ids by `RENAMED_RULE_IDS`; an old `size` row loads as both size limits. A new custom rule's id is its name as lowercase words joined by hyphens, numbered past any taken id; it never starts with `docs-` or `focused-` or ends in `-file`, `-hunk` or `-slice` and numbers, as judge question ids do. -`actions` holds one `when` (the events it wakes on) and an ordered list of steps per verdict. Each step is `{ do, target?, when? }`; its `when` holds conditions that must all hold for it to run (`whenHolds`): `author` (any of GitHub's `author_association` values `OWNER`, `MEMBER`, `COLLABORATOR`, `CONTRIBUTOR`, `FIRST_TIME_CONTRIBUTOR`, `FIRST_TIMER`, `NONE`, `MANNEQUIN`, or `BOT` for a bot whatever its association, `{ team }`, `{ users }`; a policy naming a group of `AUTHOR_GROUPS`, `maintainers`, `external`, `first-timers` or `bots`, reads it as exactly that group's kinds), `linkedIssue`, `ci` (any of `passing`, `failing`, `pending`, `none`), `files` and `lines` as `{ over?, atMost? }`, and `findings` as `{ above: P0–P3 }` or `{ any: boolean }`, which `whenHolds` leaves to the manager once code review reports and a policy file accepts only with `code-review` on. Every rule row may carry `text`, `reason` and `evidence` (`#44`, `owner/name#44`, https links or `maintainer-stated`). An action runs on its own only when the workspace's `pr-triage` system role holds `invoke` on `triage-do::/`; otherwise it waits for a person holding that grant, and `close` on `unsure` and `merge` on anything but `pass` always wait. `labels` names the label the mirror writes for each verdict; a verdict it leaves out keeps its default (`Triage: Ready`, `Triage: Not Ready`, `Triage: Needs Human Review`). Repository roles were removed, so a step whose target is a role is left out and named in the pack's `dropped`. +`actions` holds one `when` (the events it wakes on) and an ordered list of steps per verdict. Each step is `{ do, target?, when? }`; its `when` holds conditions that must all hold for it to run (`whenHolds`): `author` (any of GitHub's `author_association` values `OWNER`, `MEMBER`, `COLLABORATOR`, `CONTRIBUTOR`, `FIRST_TIME_CONTRIBUTOR`, `FIRST_TIMER`, `NONE`, `MANNEQUIN`, or `BOT` for a bot whatever its association, `{ team }`, `{ users }`; a policy naming a group of `AUTHOR_GROUPS`, `maintainers`, `external`, `first-timers` or `bots`, reads it as exactly that group's kinds), `linkedIssue`, `ci` (any of `passing`, `failing`, `pending`, `none`), `files` and `lines` as `{ over?, atMost? }`, and `findings` as `{ above: P0–P3 }` or `{ any: boolean }`, which `whenHolds` leaves to the manager once code review reports and a policy file accepts only with `code-review` on. Every rule row may carry `text`, `reason` and `evidence` (`#44`, `owner/name#44`, https links or `maintainer-stated`). An action runs on its own only when the workspace's `pr-triage` system role holds `invoke` on `triage-do::/`; otherwise it waits for a person holding that grant, and `close` on `unsure` and `merge` on anything but `pass` always wait. `labels` names the label the mirror writes for each verdict; a verdict it leaves out keeps its default (`Triage: Ready`, `Triage: Not Ready`, `Triage: Needs Human Review`). Repository roles were removed, so a step whose target is a role is left out and named in the pack's `dropped`. So is a labels step naming a label Triage takes back (a `sensitive: …` or `risk: …` level label): the whole step is left out and named in `dropped` with its position and its other labels, and the rest of the pack loads; a policy file or an override naming one is refused instead. ```json { diff --git a/packages/triage-contracts/src/check-pack.test.ts b/packages/triage-contracts/src/check-pack.test.ts index aa69e1f5f..3dc9d92fe 100644 --- a/packages/triage-contracts/src/check-pack.test.ts +++ b/packages/triage-contracts/src/check-pack.test.ts @@ -236,9 +236,14 @@ describe("check pack contract", () => { test("a labels step holding a level label is left out and named, and the rest of the pack loads", () => { const level = { do: "labels", target: { from: "list", labels: ["Risk: High"] } }; - const read = readCheckPack({ ...emptyPack(REPO), actions: { ...ACTIONS, pass: [level, COMMENT] } }); + const withOthers = { do: "labels", target: { from: "list", labels: ["needs-qa", "Risk: High"] } }; + const read = readCheckPack({ ...emptyPack(REPO), actions: { ...ACTIONS, pass: [COMMENT, COMMENT, level, withOthers] } }); expect(read.actions.pass).toEqual([COMMENT]); - expect(read.dropped).toEqual(["Pass step 1 label Risk: High is one Triage takes back; name another"]); + expect(read.dropped).toEqual([ + "Pass step 3 label Risk: High is one Triage takes back, so the whole step is skipped", + "Pass step 4 label Risk: High is one Triage takes back, so the whole step is skipped; its other labels (needs-qa) are not added either", + ]); + expect(() => readCheckPack({ ...emptyPack(REPO), actions: { ...ACTIONS, pass: [level] } }, REPO, true)).toThrow("Pass step 1 label Risk: High is one Triage takes back; name another."); }); test("the schema accepts what the parser reads, and a rejection says why", () => { diff --git a/packages/triage-contracts/src/check-pack.ts b/packages/triage-contracts/src/check-pack.ts index 5bbafd9e9..8603a6c94 100644 --- a/packages/triage-contracts/src/check-pack.ts +++ b/packages/triage-contracts/src/check-pack.ts @@ -1129,15 +1129,23 @@ function parseStep(row: Record, where: string): Step { } } -/** Repository roles were removed, so a step for one is left out and named in `dropped`. */ -function parseList(raw: unknown, where: string, dropped: string[]): Step[] { +/** The first label of a labels step that Triage takes back, which a step naming would flip on and off across runs. */ +function levelLabelOf(step: Step): string | undefined { + return step.do === "labels" && step.target.from === "list" ? step.target.labels.find((label) => levelLabelRule(label) !== undefined) : undefined; +} + +/** + * Repository roles were removed, so a step for one is left out and named in `dropped`. A step naming a level label is + * left out and named the same way, or refused when `refuse` is set, as a policy file is. + */ +function parseList(raw: unknown, where: string, dropped: string[], refuse: boolean): Step[] { if (!Array.isArray(raw)) throw new Error(`${where} must be a list.`); if (raw.length > STEPS_MAX) throw new Error(`${where} must list at most ${STEPS_MAX} steps.`); return raw.flatMap(function stepAt(item, i) { const at = `${where} ${i + 1}`; const row = record(item, at); const role = asRecord(row.target)?.to === "role" ? asRecord(row.target)?.role : undefined; - if (role === undefined) return [parseStep(row, at)]; + if (role === undefined) return stepWithoutLevelLabel(parseStep(row, at), at, dropped, refuse); dropped.push(`${at} (${String(row.do)} role ${String(role)}) no longer runs`); return []; }); @@ -1176,17 +1184,26 @@ function parseLabels(raw: unknown): Record { return Object.fromEntries(VERDICTS.map((verdict) => [verdict, row[verdict] === undefined ? VERDICT_LABELS[verdict] : text(row[verdict], `labels ${verdict}`, LABEL_MAX)])) as Record; } +function stepWithoutLevelLabel(step: Step, at: string, dropped: string[], refuse: boolean): Step[] { + const level = levelLabelOf(step); + if (level === undefined) return [step]; + if (refuse) throw new Error(`${at} label ${level} is one Triage takes back; name another.`); + const others = step.do === "labels" && step.target.from === "list" ? step.target.labels.filter((label) => label !== level) : []; + dropped.push(`${at} label ${level} is one Triage takes back, so the whole step is skipped${others.length > 0 ? `; its other labels (${others.join(", ")}) are not added either` : ""}`); + return []; +} + /** Packs written before verdicts stored an empty list of actions; one that is not empty is no longer read. */ -function parseActions(raw: unknown, dropped: string[]): PolicyActions { +function parseActions(raw: unknown, dropped: string[], refuse: boolean): PolicyActions { if (raw === undefined || (Array.isArray(raw) && raw.length === 0)) return { when: [...POLICY_EVENTS], pass: [], fail: [], unsure: [] }; if (Array.isArray(raw)) throw new Error("Actions must be { when, pass, fail, unsure }; a list of actions with yes, no, unsure and always branches is no longer read."); const row = record(raw, "Actions"); exactKeys(row, ["when", ...VERDICTS], "Actions"); return { when: parseEvents(row.when, "Actions when"), - pass: distinctSteps(parseList(row.pass, "Pass step", dropped)), - fail: distinctSteps(parseList(row.fail, "Fail step", dropped)), - unsure: distinctSteps(parseList(row.unsure, "Unsure step", dropped)), + pass: distinctSteps(parseList(row.pass, "Pass step", dropped, refuse)), + fail: distinctSteps(parseList(row.fail, "Fail step", dropped, refuse)), + unsure: distinctSteps(parseList(row.unsure, "Unsure step", dropped, refuse)), }; } @@ -1283,21 +1300,8 @@ function withDropped(read: T, dropped: string[]): T & { droppe return { ...read, ...(dropped.length > 0 && { dropped }) }; } -/** A labels step naming a label Triage takes back would flip it across runs, so it is left out and named, and the rest of the pack still loads. */ -function withoutLevelLabelSteps(actions: PolicyActions, dropped: string[]): PolicyActions { - const kept = { ...actions }; - for (const list of VERDICTS) { - kept[list] = actions[list].filter(function keepStep(step, i) { - const level = step.do === "labels" && step.target.from === "list" ? step.target.labels.find((label) => levelLabelRule(label) !== undefined) : undefined; - if (level !== undefined) dropped.push(`${STEP_WHERE[list]} ${i + 1} label ${level} is one Triage takes back; name another`); - return level === undefined; - }); - } - return kept; -} - -/** Reads a stored pack, throwing an Error whose message says why it is rejected. Version 1 reads as version 2. */ -export function readCheckPack(raw: unknown, expectedRepo?: string): CheckPack { +/** Reads a stored pack, throwing an Error whose message says why it is rejected. Version 1 reads as version 2. A policy file passes `refuse`, so a step naming a label Triage takes back is refused where a stored pack leaves it out and names it in `dropped`. */ +export function readCheckPack(raw: unknown, expectedRepo?: string, refuse = false): CheckPack { const stored = policyObject(raw); if (stored.schemaVersion !== LEGACY_SCHEMA_VERSION) currentVersion(stored); const dropped: string[] = []; @@ -1306,7 +1310,7 @@ export function readCheckPack(raw: unknown, expectedRepo?: string): CheckPack { const repo = readRepo(body, expectedRepo); const checks = readChecks(body.checks); const custom = parseCustom(body.custom); - const actions = withoutLevelLabelSteps(parseActions(body.actions, dropped), dropped); + const actions = parseActions(body.actions, dropped, refuse); const labels = parseLabels(body.labels); const groups = parseGroups(body.groups); return withDropped(refuseTriageLabels({ kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION, repo, checks, custom, actions, labels, ...(groups && { groups }) }), dropped); @@ -1335,7 +1339,7 @@ export function readSharedPolicy(raw: unknown): SharedPolicy { const checks = readChecks(body.checks); const dropped: string[] = []; const custom = parseCustom(body.custom); - const actions = parseActions(body.actions, dropped); + const actions = parseActions(body.actions, dropped, false); const labels = parseLabels(body.labels); return withDropped({ kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION, checks, custom, actions, labels }, dropped); } @@ -1357,7 +1361,7 @@ function parseOverrideActions(row: Partial const actions: Partial = {}; if (row?.when !== undefined) actions.when = parseEvents(row.when, "Actions when"); for (const verdict of VERDICTS) { - if (row?.[verdict] !== undefined) actions[verdict] = distinctSteps(parseList(row[verdict], STEP_WHERE[verdict], [])); + if (row?.[verdict] !== undefined) actions[verdict] = distinctSteps(parseList(row[verdict], STEP_WHERE[verdict], [], true)); } return actions; } @@ -1456,13 +1460,6 @@ export function refuseTriageLabels(pack: CheckPack): CheckPack { const level = levelLabelRule(lower); if (lower.startsWith("triage:") || lower.startsWith(RULE_LABEL_PREFIX) || verdictLabels.includes(lower) || (level !== undefined && level !== id)) throw new Error(`Rule ${id} label ${label} is one Triage takes back; name another.`); } - for (const list of VERDICTS) { - for (const [i, step] of pack.actions[list].entries()) { - if (step.do !== "labels" || step.target.from !== "list") continue; - const level = step.target.labels.find((label) => levelLabelRule(label) !== undefined); - if (level !== undefined) throw new Error(`${STEP_WHERE[list]} ${i + 1} label ${level} is one Triage takes back; name another.`); - } - } return pack; } diff --git a/packages/triage-contracts/src/policy-file.ts b/packages/triage-contracts/src/policy-file.ts index 8f54b9028..e56fcc933 100644 --- a/packages/triage-contracts/src/policy-file.ts +++ b/packages/triage-contracts/src/policy-file.ts @@ -113,7 +113,7 @@ function policyOf(file: Record, repo: string): { policy: Record const header = { kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION } as const; const { policy: body, workflow } = readFile(file); if (body.extends !== undefined) return { policy: { ...header, ...overrideOf({ ...body, ...header, repo }, repo) }, workflow }; - const { dropped, ...pack } = readCheckPack({ ...body, ...header, repo }, repo); + const { dropped, ...pack } = readCheckPack({ ...body, ...header, repo }, repo, true); if (dropped) throw new Error(`The policy file holds what no longer runs: ${refusalNames(dropped)}.`); return { policy: pack, workflow: { ...workflow, decide: workflow.decide ?? decideOf(pack.actions) } }; } diff --git a/packages/triage-contracts/src/policy-rules.test.ts b/packages/triage-contracts/src/policy-rules.test.ts index ec32f545d..d5776af9a 100644 --- a/packages/triage-contracts/src/policy-rules.test.ts +++ b/packages/triage-contracts/src/policy-rules.test.ts @@ -134,6 +134,8 @@ actions: expect(checkPolicyFile(`rules:\n${rule} label: docs\n`, REPO)).toEqual({ ok: false, problems: [{ message: "Rule docs label needs onFail: label.", line: 2 }] }); expect(checkPolicyFile(`rules:\n${rule} onFail: label\n label: "rule: docs"\n`, REPO)).toEqual({ ok: false, problems: [{ message: "Rule docs label rule: docs is one Triage takes back; name another." }] }); expect(checkPolicyFile(`rules:\n${rule} onFail: label\n label: "sensitive: medium"\n`, REPO)).toEqual({ ok: false, problems: [{ message: "Rule docs label sensitive: medium is one Triage takes back; name another." }] }); + const step = `rules:\n${rule}actions:\n pass: [{ do: comment, target: { body: Hi } }, { do: labels, target: { from: list, labels: ["Risk: High", needs-qa] } }]\n`; + expect(checkPolicyFile(step, REPO)).toEqual({ ok: false, problems: [{ message: "Pass step 2 label Risk: High is one Triage takes back; name another." }] }); }); test("sensitive-paths reads its groups and risk its levels, both ask unless the file says, survive export, and a bad group name, signal or count is refused", () => { From 502aa2d27007a1a43e2323ce8732c4706b652493 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 29 Oct 2026 09:07:11 -0700 Subject: [PATCH 3/3] fix(contracts): leave every level label out of a dropped step's other labels --- packages/triage-contracts/src/check-pack.test.ts | 2 +- packages/triage-contracts/src/check-pack.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/triage-contracts/src/check-pack.test.ts b/packages/triage-contracts/src/check-pack.test.ts index 3dc9d92fe..e754ec733 100644 --- a/packages/triage-contracts/src/check-pack.test.ts +++ b/packages/triage-contracts/src/check-pack.test.ts @@ -236,7 +236,7 @@ describe("check pack contract", () => { test("a labels step holding a level label is left out and named, and the rest of the pack loads", () => { const level = { do: "labels", target: { from: "list", labels: ["Risk: High"] } }; - const withOthers = { do: "labels", target: { from: "list", labels: ["needs-qa", "Risk: High"] } }; + const withOthers = { do: "labels", target: { from: "list", labels: ["needs-qa", "Risk: High", "sensitive: MEDIUM", "risk: high"] } }; const read = readCheckPack({ ...emptyPack(REPO), actions: { ...ACTIONS, pass: [COMMENT, COMMENT, level, withOthers] } }); expect(read.actions.pass).toEqual([COMMENT]); expect(read.dropped).toEqual([ diff --git a/packages/triage-contracts/src/check-pack.ts b/packages/triage-contracts/src/check-pack.ts index 8603a6c94..83fdde423 100644 --- a/packages/triage-contracts/src/check-pack.ts +++ b/packages/triage-contracts/src/check-pack.ts @@ -1188,7 +1188,7 @@ function stepWithoutLevelLabel(step: Step, at: string, dropped: string[], refuse const level = levelLabelOf(step); if (level === undefined) return [step]; if (refuse) throw new Error(`${at} label ${level} is one Triage takes back; name another.`); - const others = step.do === "labels" && step.target.from === "list" ? step.target.labels.filter((label) => label !== level) : []; + const others = step.do === "labels" && step.target.from === "list" ? step.target.labels.filter((label) => levelLabelRule(label) === undefined) : []; dropped.push(`${at} label ${level} is one Triage takes back, so the whole step is skipped${others.length > 0 ? `; its other labels (${others.join(", ")}) are not added either` : ""}`); return []; }