Skip to content

Commit 928dcbc

Browse files
committed
fix(ci): a pm:blocked card parked on a PR is released only once that PR merged
The full-run card refresh resolved every `Blocked-by:` target through issues.get and kept `{state, closed_at}`. The issues API answers a PR number `closed` whether the PR merged or was closed unmerged, so an abandoned PR released the card as though its fix had landed on main. The card step now carries `pull_request.merged_at`, and the verdict holds a closed PR with no `merged_at` and names it on the summary row. The self-tests pin merged (released, body and comment channel), closed unmerged (held) and open (held), plus the card step's projection. triage-duties.md names the line triage writes when it parks a card: a line-initial `Blocked-by: #N` in the card body, an issue or PR number, beside the label; the full-run card takes it in a comment because its body is rewritten on every red. The line is paid in the same file by folding the two reporting-discipline bullets and dropping their restated clause. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WV4cxBhj4jCJgyxHY42p5
1 parent 2a3dec2 commit 928dcbc

3 files changed

Lines changed: 101 additions & 20 deletions

File tree

‎.claude/skills/pm-dispatch/references/triage-duties.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
- 行首 `Ruling-ref:` 引本例外裁决原话;⛔ 不改 `domain:*`、不定 p0(归 main-red)、不碰人工地板。
3434
- 分诊每轮读上轮以来新贴的自分诊记录,逐条回复确认或改判;复审前席位定级有效。
3535
- 分类动作每张三选一,外加一个修复通道;裁定写方向、给六态之一。
36+
- 停 `pm:blocked` 同笔在卡正文写行首 `Blocked-by: #N`(issue/PR 号;full-run 卡写评论),散文不算。
3637
- 方向在 ADR/本卡裁决/不可重裁规则:贴 `check-prior-rulings.mjs` 的 `Prior rulings read:` 行,或不点。
3738
- `pm:queue` = 有具名落点或复现的具体缺陷,无可问之事;⛔ 工具/门禁修复不由此进。
3839
- `tooling` 入 `pm:queue` 仅凭三门之一:首行带 `Unblocks: #N`(open 产品卡)或点名所护的已发布面;
@@ -97,6 +98,5 @@
9798
- 轮次报告设呈裁清单专节(卡、推荐、所引权威),⛔ 无代裁清单、无追认表。
9899
- 回翻条款:实施中发现契约终究要动 ⇒ dev 停手,卡回决策箱,⛔ 永不静默重裁。
99100
- 请示纪律:方向性授权覆盖整条执行链一并做完,⛔ 不逐项回问。
100-
- 中间状态与限流/重试类运维细节只进座位贴和触发器,⛔ 不弹给维护者。
101-
- 汇报只在里程碑、真阻塞、被问到时。
101+
- 汇报只在里程碑、真阻塞、被问到时;中间状态与限流/重试类细节只进座位贴和触发器。
102102
- 人工地板项(发版、天花板、契约扩大)仍需明确字句,但攒批不逐项弹。

‎scripts/ci/scheduled-full-run-card.mjs‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,9 @@ export async function fileCardStep({ github, context, core, now = () => new Date
105105
// The DECISION is the module's, driven offline by its self-test
106106
// against a mutable board (one red run files one card; the next
107107
// refreshes it and files no second; a refresh of a card whose
108-
// every blocker is a closed LOCAL card moves `pm:blocked` to
109-
// `pm:queue`). What is left here is thin callbacks over the API.
108+
// every blocker is a closed LOCAL card, a PR one merged, moves
109+
// `pm:blocked` to `pm:queue`). What is left here is thin callbacks
110+
// over the API.
110111
const outcome = await mod.fileOrRefreshCard({
111112
identity,
112113
body,
@@ -138,7 +139,9 @@ export async function fileCardStep({ github, context, core, now = () => new Date
138139
getIssue: async ({ repo: target, number }) => {
139140
const [targetOwner, targetRepo] = target.split('/');
140141
const res = await github.rest.issues.get({ owner: targetOwner, repo: targetRepo, issue_number: number });
141-
return { state: res.data.state, closed_at: res.data.closed_at };
142+
// A PR number answers `closed` merged or not; `merged_at` tells them apart.
143+
const pr = res.data.pull_request ? { pull_request: { merged_at: res.data.pull_request.merged_at ?? null } } : {};
144+
return { state: res.data.state, closed_at: res.data.closed_at, ...pr };
142145
},
143146
// The swap: an additive POST and ONE remove by name.
144147
addLabels: async ({ number, labels }) => {
@@ -362,6 +365,43 @@ const SELF_TEST_CASES = [
362365
{ output: ['number', '430'] },
363366
],
364367
},
368+
{
369+
label: 'a pm:blocked card whose blocker is a PR closed unmerged is held: the PR shape reaches the judgement, no label write',
370+
run: watchedRun(),
371+
api: {
372+
'actions.listWorkflowRuns': async () => previousGreenRun,
373+
'repos.compareCommitsWithBasehead': async () => compareOf(0),
374+
'issues.listForRepo': async () => ({
375+
data: [{
376+
number: 440,
377+
title: mod.cardIdentity('CI').title,
378+
body: 'old\n\nBlocked-by: #601\n',
379+
state: 'open',
380+
labels: [{ name: 'bug' }, { name: 'pm:blocked' }],
381+
assignees: [],
382+
}],
383+
}),
384+
'issues.listComments': async () => ({ data: [] }),
385+
'issues.get': async () => ({
386+
data: { state: 'closed', closed_at: '2026-10-26T20:00:00Z', pull_request: { merged_at: null } },
387+
}),
388+
'issues.update': async () => ({ data: {} }),
389+
},
390+
calls: [
391+
LOOKUP, COMPARE, SCAN,
392+
{ api: 'issues.listComments', params: { ...OWNER_REPO, issue_number: 440, per_page: 100, page: 1 } },
393+
{ api: 'issues.get', params: { ...OWNER_REPO, issue_number: 601 } },
394+
{ api: 'issues.update', params: { ...OWNER_REPO, issue_number: 440 } },
395+
],
396+
body: renderArgs({ compareUrl: COMPARE_URL }),
397+
reports: [
398+
{ info: `refreshed ${mod.cardIdentity('CI').marker} card #440` },
399+
{ info: 'block: held -- #601 closed unmerged (a PR that never landed)' },
400+
{ notice: 'CI is red on the scheduled full run — card #440 refreshed.' },
401+
{ output: ['action', 'refreshed'] },
402+
{ output: ['number', '440'] },
403+
],
404+
},
365405
];
366406

367407
// The roster is a LITERAL the table is checked against (see
@@ -375,9 +415,10 @@ const SELF_TEST_BATTERIES = Object.freeze({
375415
'a compare past the endpoint cap is marked truncated, and the watched workflow names the card': 1,
376416
'an open card is refreshed in place: one issues.update, no create, no label write': 1,
377417
'a pm:blocked card whose only blocker closed reaches every callback: comments, issue read, add then remove': 1,
418+
'a pm:blocked card whose blocker is a PR closed unmerged is held: the PR shape reaches the judgement, no label write': 1,
378419
});
379420
// Deleting a roster entry silences its floor as surely as zeroing it.
380-
const SELF_TEST_BATTERY_FLOOR = 7;
421+
const SELF_TEST_BATTERY_FLOOR = 8;
381422

382423
/** Doubles of what github-script hands the step, recording every call. */
383424
function doubles(caseDef) {

‎scripts/ci/scheduled-full-run.mjs‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -504,9 +504,9 @@ export async function fileOrRefreshCard({
504504
// THE RULE. On a refresh of a card carrying `pm:blocked`, every `Blocked-by:`
505505
// target the card names -- body AND comments, line-initial, read by the
506506
// patrol's own reader -- is resolved. When EVERY one is a LOCAL card that has
507-
// closed, no `Unlock-action:` line names another unlock, and the card has no
508-
// assignee, the refresh swaps `pm:blocked` for `pm:queue` and says so in the
509-
// refreshed body. Anything else changes no label:
507+
// closed (a PR: merged), no `Unlock-action:` line names another unlock, and the
508+
// card has no assignee, the refresh swaps `pm:blocked` for `pm:queue` and says
509+
// so in the refreshed body. Anything else changes no label:
510510
//
511511
// not pm:blocked nothing read at all
512512
// no target named H4's row (a block with no line), not this refresh's
@@ -520,6 +520,10 @@ export async function fileOrRefreshCard({
520520
// objectstack consumes objectui through the
521521
// `.objectui-sha` pin, so the next hourly red is the
522522
// SAME failure until the pin moves -- never released
523+
// a PR target CLOSED abandoned, not landed (#27846): the issues API answers
524+
// UNMERGED a PR number `closed` whether or not it merged, and
525+
// only `pull_request.merged_at` tells the two apart --
526+
// never released
523527
// an `Unlock-action:` the card names an unlock other than closure
524528
// line (any channel) (`references/state-machine.md`, `pm:blocked`), so
525529
// closure is not its exit -- never released. Any value
@@ -561,19 +565,22 @@ export const QUEUE_LABEL = 'pm:queue';
561565

562566
// The pure verdict over resolved targets. `resolutions` are
563567
// `blockerTargetsFor`'s targets, each carrying `local` and `state`
564-
// ('open' | 'closed' | 'unresolved') -- the shape H19 judges.
568+
// ('open' | 'closed' | 'unresolved') -- the shape H19 judges -- and, on a PR
569+
// that closed without merging, `unmerged: true`.
565570
// `unlockActions` are the card's `Unlock-action:` values, both channels.
566571
export function staleBlockVerdict({ issue, resolutions, unlockActions = [] }) {
567572
if (!labelNames(issue ?? {}).includes(BLOCKED_LABEL)) return { verdict: 'not-blocked' };
568573
const rows = resolutions ?? [];
569574
if (rows.length === 0) return { verdict: 'no-target' };
570575
const open = rows.filter((r) => r.state === 'open');
571576
const unread = rows.filter((r) => r.state !== 'open' && r.state !== 'closed');
577+
// A PR closed unmerged never landed its fix anywhere: it withholds.
578+
const unmerged = rows.filter((r) => r.state === 'closed' && r.unmerged === true);
572579
// Closed THERE is not consumable HERE: a closed cross-repo target withholds.
573580
const upstream = rows.filter((r) => r.state === 'closed' && r.local !== true);
574581
const declared = [...(unlockActions ?? [])];
575-
if (open.length > 0 || unread.length > 0 || upstream.length > 0 || declared.length > 0) {
576-
return { verdict: 'held', open, unread, upstream, unlockActions: declared };
582+
if (open.length > 0 || unread.length > 0 || unmerged.length > 0 || upstream.length > 0 || declared.length > 0) {
583+
return { verdict: 'held', open, unread, unmerged, upstream, unlockActions: declared };
577584
}
578585
const assignees = (issue?.assignees ?? []).map((a) => String(a?.login ?? a));
579586
if (assignees.length > 0) return { verdict: 'assigned', assignees };
@@ -611,10 +618,14 @@ export async function judgeCardBlock({ issue, ownerRepo, listComments, getIssue,
611618
try {
612619
const got = await getIssue({ repo: target.repo, number: target.number });
613620
const state = got?.state === 'closed' || got?.state === 'open' ? got.state : 'unresolved';
621+
// A PR answers in the issues API's PR shape; a closed one with no
622+
// `merged_at` was abandoned (a missing `merged_at` reads the same: held).
623+
const unmerged = state === 'closed' && got?.pull_request != null && !got.pull_request.merged_at;
614624
resolutions.push({
615625
...target,
616626
state,
617627
closedAt: got?.closed_at ?? null,
628+
...(unmerged ? { unmerged: true } : {}),
618629
detail: state === 'unresolved' ? `answered state '${got?.state}'` : null,
619630
});
620631
} catch (error) {
@@ -627,12 +638,15 @@ export async function judgeCardBlock({ issue, ownerRepo, listComments, getIssue,
627638

628639
const targetName = (r) => (r.local ? `#${r.number}` : r.key);
629640

630-
// What each target reads as on the summary row. A closed cross-repo target is
631-
// NAMED as the reason it holds, never shown as a bare `closed`.
632-
const targetReading = (r) =>
633-
r.state === 'closed' && r.local !== true
634-
? `${targetName(r)} closed upstream (not an unlock criterion for this consumer)`
635-
: `${targetName(r)} ${r.state}`;
641+
// What each target reads as on the summary row. A closed cross-repo target, and
642+
// a PR closed unmerged, are NAMED as the reason they hold, never a bare `closed`.
643+
const targetReading = (r) => {
644+
if (r.state === 'closed' && r.unmerged === true) return `${targetName(r)} closed unmerged (a PR that never landed)`;
645+
if (r.state === 'closed' && r.local !== true) {
646+
return `${targetName(r)} closed upstream (not an unlock criterion for this consumer)`;
647+
}
648+
return `${targetName(r)} ${r.state}`;
649+
};
636650

637651
// One line for the run log: the verdict and what it was read from.
638652
function summariseBlock(block) {
@@ -1726,7 +1740,7 @@ async function selfTest() {
17261740
// log of every call the refresh makes, in order.
17271741
const blockBoard = ({
17281742
labels = [...CARD_LABELS, BLOCKED_LABEL], assignees = [], comments = [], states = {}, cardBody = OLD_BODY,
1729-
commentPages = null, failComments = null, failAdd = null, failRemove = null,
1743+
commentPages = null, failComments = null, failAdd = null, failRemove = null, pulls = {},
17301744
} = {}) => {
17311745
const card = { number: 23885, title: OPEN_CI_TITLE, body: cardBody,
17321746
labels: labels.map((name) => ({ name })), assignees: assignees.map((login) => ({ login })) };
@@ -1751,7 +1765,9 @@ async function selfTest() {
17511765
calls.push(`getIssue ${repo}#${number}`);
17521766
const state = states[`${repo}#${number}`];
17531767
if (!state) throw notFound('Not Found');
1754-
return { state, closed_at: state === 'closed' ? '2026-10-16T13:20:00Z' : null };
1768+
// A number in `pulls` is a PR, answered in the issues API's PR shape.
1769+
const pr = Object.hasOwn(pulls, `${repo}#${number}`) ? { pull_request: { merged_at: pulls[`${repo}#${number}`] } } : {};
1770+
return { state, closed_at: state === 'closed' ? '2026-10-16T13:20:00Z' : null, ...pr };
17551771
},
17561772
addLabels: async ({ number, labels: add }) => {
17571773
calls.push(`addLabels #${number} ${add.join(',')}`);
@@ -1947,6 +1963,30 @@ async function selfTest() {
19471963
states: { 'objectstack-ai/objectstack#23908': 'closed' } });
19481964
if ((await b.refresh()).block !== 'expired') throw new Error('body channel: a body-borne line was not read');
19491965
});
1966+
// A park on a PR (#27846): triage names the PR itself, and the issues API
1967+
// answers it `closed` whether it merged or was abandoned. MERGED releases
1968+
// the card, in either channel; closed UNMERGED holds it and says why; OPEN
1969+
// holds it.
1970+
await check(async () => {
1971+
const PR = 'objectstack-ai/objectstack#24305';
1972+
for (const where of [{ cardBody: `${OLD_BODY}\nBlocked-by: #24305` }, { comments: ['- **Blocked-by: #24305.**'] }]) {
1973+
const b = blockBoard({ ...where, states: { [PR]: 'closed' }, pulls: { [PR]: '2026-10-16T13:20:00Z' } });
1974+
const outcome = await b.refresh();
1975+
if (outcome.block !== 'expired' || b.names() !== sorted([...CARD_LABELS, QUEUE_LABEL])) {
1976+
throw new Error(`merged PR: expected the card released, got '${outcome.block}' holding ${b.names()}`);
1977+
}
1978+
}
1979+
const abandoned = blockBoard({ cardBody: `${OLD_BODY}\nBlocked-by: #24305`,
1980+
states: { [PR]: 'closed' }, pulls: { [PR]: null } });
1981+
const held = await abandoned.refresh();
1982+
untouched(abandoned, held, 'held');
1983+
if (!held.blockSummary.includes('#24305 closed unmerged')) {
1984+
throw new Error(`unmerged PR: the summary does not name the abandoned PR as the reason (${held.blockSummary})`);
1985+
}
1986+
if (abandoned.card.body !== NEW_BODY) throw new Error('unmerged PR: a held block wrote a note into the body');
1987+
const open = blockBoard({ cardBody: `${OLD_BODY}\nBlocked-by: #24305`, states: { [PR]: 'open' }, pulls: { [PR]: null } });
1988+
untouched(open, await open.refresh(), 'held');
1989+
});
19501990
// A card naming only itself names no blocker.
19511991
await check(async () => {
19521992
const b = blockBoard({ comments: ['Blocked-by: #23885'], states: { 'objectstack-ai/objectstack#23885': 'open' } });

0 commit comments

Comments
 (0)