Skip to content

Commit 9a2f3df

Browse files
os-litantclaude
andauthored
fix(pm): split the governed-merges attribution column three ways (#12658)
A PR-less mainline entry — the loudest line the sweep prints — rendered "merged_by UNAVAILABLE — every channel failed; see the attribution note below" when zero channels had been tried: `main()` skips an entry with no PR number (there is no pull request to query), and the column was picked on `entry.attribution` alone. The note it pointed at is never produced for such an entry either, since `summariseAttributionFailures` groups only entries carrying `attributionError`. The column now reads: resolved · UNAVAILABLE, every channel failed (an `attributionError` is present — the only case that may point at the note) · NOT LOOKED UP, with the reason read off the entry rather than assumed. Report-only: no judgment moves. `attributionFailed` and the INCOMPLETE exit are still driven by real channel failures alone, and the direct-push warning on a PR-less entry is unchanged. Claude-Session: https://claude.ai/code/session_01MnijPVVDakqK2J335JoJtq Co-authored-by: Claude <noreply@anthropic.com>
1 parent ab6dd32 commit 9a2f3df

1 file changed

Lines changed: 93 additions & 5 deletions

File tree

scripts/pm/check-governed-merges.mjs

Lines changed: 93 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,10 @@
416416
* verbatim and extend the audit to read the enqueue actor from the issue
417417
* timeline (`added_to_merge_queue`) — never remap silently. A mainline commit
418418
* whose subject names NO PR is listed as its own loud entry (a direct push to
419-
* `main` is more anomalous than any PR merge, not less).
419+
* `main` is more anomalous than any PR merge, not less). Such an entry has no
420+
* pull request to query, so its attribution column reads NOT LOOKED UP, never
421+
* "every channel failed" — the three-way column at `attributionCell` (#12645)
422+
* carries that distinction and the reason it is not cosmetic.
420423
*
421424
* ### The attribution channel chain (#9619, measured on the PM container)
422425
*
@@ -1430,6 +1433,58 @@ export function summariseAttributionFailures(entries) {
14301433

14311434
// ── rendering ───────────────────────────────────────────────────────────────
14321435

1436+
/**
1437+
* The attribution column, THREE ways (#12645) — because "nothing was found"
1438+
* and "nothing was looked at" are different facts, and this report keeps them
1439+
* apart everywhere else (#4690: an unaudited repo is not a clean repo, a
1440+
* window whose boundary is unproven is not an empty window).
1441+
*
1442+
* The column used to be picked on `entry.attribution` alone, so an entry with
1443+
* no attribution rendered "every channel failed; see the attribution note
1444+
* below" — and the ONE entry shape that can reach that branch without a single
1445+
* channel having been tried is the loudest line the sweep prints: a mainline
1446+
* commit whose subject names no PR (`main()` skips it: `if (entry.pr == null)
1447+
* continue` — there is no pull request to query). Measured 2026-08-27 on a
1448+
* constructed sweep, that line claimed every channel failed four lines under a
1449+
* printed `0 API lookup(s)`, and referred the reader to a note
1450+
* `summariseAttributionFailures` never produces for it (that function groups
1451+
* only entries carrying `attributionError`, and this one carries none). A
1452+
* false claim on the most anomalous entry in the list is exactly the line a
1453+
* reader learns to discount.
1454+
*
1455+
* 1. resolved — a channel answered; it names which one.
1456+
* 2. UNAVAILABLE — `attributionError` is present: channels WERE tried and
1457+
* all failed. ⚠️ This is the only branch that may point
1458+
* at the attribution note, because it is the only one
1459+
* `summariseAttributionFailures` writes a line for.
1460+
* 3. NOT LOOKED UP — no reading was attempted. The reason is READ off the
1461+
* entry, never assumed: absent PR number is the case
1462+
* `main()` produces, and an entry that has a PR number
1463+
* yet reached here gets the honest residual instead of
1464+
* being told it has no PR number — asserting an untried
1465+
* channel and asserting an absent PR number are the same
1466+
* defect wearing different words.
1467+
*
1468+
* ⛔ Report-only. This changes no judgment: `attributionFailed` (and with it
1469+
* the INCOMPLETE exit) is still set only by a real channel failure, and a
1470+
* PR-less entry is still its own loud entry — a direct push to `main` is more
1471+
* anomalous than any PR merge, not less. Pure, so `--self-test` asserts on the
1472+
* words.
1473+
*/
1474+
export function attributionCell(entry) {
1475+
if (entry.attribution) {
1476+
return (
1477+
`merged_by ${entry.attribution.mergedBy ?? '(none)'} @ ${entry.attribution.mergedAt ?? '(unknown)'} ` +
1478+
`(via ${entry.attributionChannel ?? 'unknown channel'})`
1479+
);
1480+
}
1481+
if (entry.attributionError) return `merged_by UNAVAILABLE — every channel failed; see the attribution note below`;
1482+
if (entry.pr == null) {
1483+
return `merged_by NOT LOOKED UP — no PR number in the subject, so there is no pull request to query (not a channel failure)`;
1484+
}
1485+
return `merged_by NOT LOOKED UP — no attribution reading was recorded for this entry (not a channel failure)`;
1486+
}
1487+
14331488
/**
14341489
* The window, in the words the operator reads — pure, and the half of route A
14351490
* the #12633 ruling names explicitly: the back-off has to be SAID, or a
@@ -1517,9 +1572,7 @@ export function renderReport({ window, repos, scanned, entries, lookups }) {
15171572

15181573
const lines = entries.map((e) => {
15191574
const surfaces = e.surfaces.map((s) => `${s.glob} ×${s.files.length}`).join(', ');
1520-
const who = e.attribution
1521-
? `merged_by ${e.attribution.mergedBy ?? '(none)'} @ ${e.attribution.mergedAt ?? '(unknown)'} (via ${e.attributionChannel ?? 'unknown channel'})`
1522-
: `merged_by UNAVAILABLE — every channel failed; see the attribution note below`;
1575+
const who = attributionCell(e);
15231576
const prName = e.pr != null ? `PR #${e.pr}` : '⚠️ NO PR NUMBER IN SUBJECT — direct push to main? investigate';
15241577
const files = e.surfaces.flatMap((s) => s.files.slice(0, 6)).slice(0, 8);
15251578
return ` • ${e.repoSlug ? `${e.repoSlug} ` : ''}${prName}${e.subject}\n commit ${e.sha.slice(0, 9)} @ ${e.date}; ${who}\n surfaces: ${surfaces}\n${files.map((f) => ` - ${f}`).join('\n')}`;
@@ -2196,6 +2249,41 @@ async function selfTest() {
21962249
assert('a-resolved-column-carries-the-account-is-not-a-principal-caveat', resolvedReport.includes('names an ACCOUNT, not a principal'), resolvedReport);
21972250
assert('the-caveat-is-absent-when-nothing-resolved', !unresolvedReport.includes('names an ACCOUNT, not a principal'));
21982251

2252+
// ── the attribution column's THIRD case (#12645) ──────────────────────────
2253+
// The two fixtures above are cases 1 and 2; the PR-less mainline entry —
2254+
// the loudest line the sweep prints — is case 3, and it used to render
2255+
// case 2's words with zero channels tried. All three are pinned as a set,
2256+
// because the defect was a two-way split covering three facts.
2257+
const notLookedUp = renderReport({ window: dateWindowFor('2026-08-13T00:00:00Z'), repos: allAudited, scanned: 3, entries: [noPr], lookups: 0 });
2258+
assert('a-pr-less-entry-is-NOT-LOOKED-UP-not-a-failed-lookup', notLookedUp.includes('merged_by NOT LOOKED UP') && !notLookedUp.includes('every channel failed'), notLookedUp);
2259+
assert('and-it-says-WHY-nothing-was-queried', notLookedUp.includes('no PR number in the subject') && notLookedUp.includes('not a channel failure'), notLookedUp);
2260+
// The dangling pointer half of the defect: it named a note that this very
2261+
// report never prints for it, because the note groups attributionError only.
2262+
assert('a-not-looked-up-entry-points-at-no-attribution-note', !notLookedUp.includes('attribution note below'), notLookedUp);
2263+
assert('and-the-report-prints-none-for-it', summariseAttributionFailures([noPr]).length === 0, JSON.stringify(summariseAttributionFailures([noPr])));
2264+
assert('the-note-pointer-belongs-to-the-every-channel-failed-case-alone', unresolvedReport.includes('attribution note below'), unresolvedReport);
2265+
// Report-only: the loud entry stays loud, and a case-3 column is still not
2266+
// a resolved one (no ACCOUNT-not-a-principal caveat, nothing to prompt on).
2267+
assert('the-third-case-does-not-soften-the-direct-push-warning', notLookedUp.includes('NO PR NUMBER IN SUBJECT — direct push to main? investigate'), notLookedUp);
2268+
assert('and-carries-no-resolved-column-caveat', !notLookedUp.includes('names an ACCOUNT, not a principal'));
2269+
// The cell function itself, all three classes plus the residual.
2270+
assert('cell-case-1-resolved-names-its-channel',
2271+
attributionCell({ attribution: { mergedBy: 'os-steve', mergedAt: '2026-08-18T09:00:00Z' }, attributionChannel: 'anonymous', pr: 5188 })
2272+
=== 'merged_by os-steve @ 2026-08-18T09:00:00Z (via anonymous)');
2273+
assert('cell-case-2-every-channel-failed-needs-an-attributionError',
2274+
attributionCell({ pr: 101, attributionError: 'anonymous REST: HTTP 403' }).startsWith('merged_by UNAVAILABLE — every channel failed'));
2275+
assert('cell-case-3-no-pr-number-is-nothing-to-query', attributionCell({ pr: null }).startsWith('merged_by NOT LOOKED UP — no PR number in the subject'));
2276+
// ⛔ An entry that HAS a PR number must never be told it has none: asserting
2277+
// an untried channel and asserting an absent PR number are the same defect.
2278+
const residual = attributionCell({ pr: 4242 });
2279+
assert('cell-residual-never-invents-a-missing-pr-number', residual.startsWith('merged_by NOT LOOKED UP') && !residual.includes('no PR number'), residual);
2280+
assert('and-the-residual-is-not-a-channel-failure-either', !residual.includes('every channel failed') && !residual.includes('attribution note below'), residual);
2281+
// An attributionError never outranks a real reading, and a resolved entry
2282+
// is never demoted by a stale PR-less shape.
2283+
assert('a-resolved-reading-outranks-a-stale-error',
2284+
attributionCell({ attribution: { mergedBy: 'x', mergedAt: 'y' }, attributionChannel: 'env-token', pr: null, attributionError: 'HTTP 401' })
2285+
=== 'merged_by x @ y (via env-token)');
2286+
21992287
// ── the --test pre-arm predicate (#9550) ──────────────────────────────────
22002288
const governedCase = testVerdict(['AGENTS.md']);
22012289
assert('--test-on-the-#9527-file-list-answers-GOVERNED', governedCase.governed === true && governedCase.hitPaths.join() === 'AGENTS.md', JSON.stringify(governedCase));
@@ -2505,7 +2593,7 @@ async function selfTest() {
25052593
for (const failure of failures) console.error(` • ${failure}`);
25062594
process.exit(1);
25072595
}
2508-
console.log(`✓ check-governed-merges --self-test: ${checked} assertions (the unified governed predicate + near misses, subject→PR spellings, window parsing, the #12633 landing window — the QS-7 regression pin in both directions, the topological close beyond the budget, the unproven-boundary EDGE, the listed-or-INCOMPLETE invariant over every fixture, the escalating floors, per-repo --since-ref resolution and its named fallback, and the window words — the replay fixtures, the four-repo resolution incl. absent/wrong-origin/relocated checkouts, the attribution channel chain + its proxy-transport re-arm plan and its one named fallback line, the --test pre-arm predicate, the generated-artifact provenance exception — the four ruled cases against the generator's own splice, byte-exactness, fail-closed inputs, the untouched mixed-diff rule, single-file-not-a-class, the #11084 generator co-edit fence in both directions, and its render words — the #11705 generator-owned rows inside skills/** (a genuine generated file passes, the same path hand-edited does not, a path no generator declares is hand-authored content, per-row fences, and the enumeration read from the real generator), the exit table, and the report wording pins).\n ${liveNote}`);
2596+
console.log(`✓ check-governed-merges --self-test: ${checked} assertions (the unified governed predicate + near misses, subject→PR spellings, window parsing, the #12633 landing window — the QS-7 regression pin in both directions, the topological close beyond the budget, the unproven-boundary EDGE, the listed-or-INCOMPLETE invariant over every fixture, the escalating floors, per-repo --since-ref resolution and its named fallback, and the window words — the replay fixtures, the four-repo resolution incl. absent/wrong-origin/relocated checkouts, the attribution channel chain + its proxy-transport re-arm plan and its one named fallback line, the three-way attribution column (resolved · every-channel-failed · NOT LOOKED UP, and the note pointer that belongs to the middle one alone), the --test pre-arm predicate, the generated-artifact provenance exception — the four ruled cases against the generator's own splice, byte-exactness, fail-closed inputs, the untouched mixed-diff rule, single-file-not-a-class, the #11084 generator co-edit fence in both directions, and its render words — the #11705 generator-owned rows inside skills/** (a genuine generated file passes, the same path hand-edited does not, a path no generator declares is hand-authored content, per-row fences, and the enumeration read from the real generator), the exit table, and the report wording pins).\n ${liveNote}`);
25092597
}
25102598

25112599
/** The exit code `--test` would return for a path list — pinned without spawning. */

0 commit comments

Comments
 (0)