diff --git a/docs/autopilot/README.md b/docs/autopilot/README.md index 6b5f53a1e0..d8d85079c7 100644 --- a/docs/autopilot/README.md +++ b/docs/autopilot/README.md @@ -137,6 +137,20 @@ close リンクで見つからなければ head ブランチ `topic/autopilot-` で閉じた EPIC、(C) 人手で閉じた issue が Project に +取り残される。daemon はポーリングのたびに `applyClosedReconcile` で **GitHub 上で closed な issue** +（`project.listClosedIssueNumbers`）のうち Project Status が終端（Close / Done）でないものを +**Status=Close + AI Status クリア**へ整合する。closed という事実だけを根拠にするので **EPIC も対象**。 +判定は `phases.js` の `selectClosedToReconcile`（純粋関数・終端は `TERMINAL_STATUSES`）。実行中 +（run が所有する）item は触らない。冪等。 + --- ## implement→review の自動ディスパッチ（自己レビュー） diff --git a/tools/autopilot/src/daemon.js b/tools/autopilot/src/daemon.js index 6906eaedfc..4364680900 100644 --- a/tools/autopilot/src/daemon.js +++ b/tools/autopilot/src/daemon.js @@ -25,6 +25,7 @@ const { hitlDesireFromResult, selectMergeCandidates, mergeProgressionIntents, + selectClosedToReconcile, selectPrSyncCandidates, labelActions, draftAction, @@ -245,6 +246,7 @@ function applyMergeProgression(items, cfg, state, log, deps = {}) { const hasMerged = deps.hasMergedPullRequest || project.hasMergedPullRequest; const applyIntents = deps.applyIntents || project.applyIntents; const findItemId = deps.findItemId || project.findItemId; + const closeIssue = deps.closeIssue || project.closeIssue; // merge 後の面正規化（Issue の HITL ラベルを落とす等）。テストは no-op を注入できる。 const syncFaces = deps.syncFaces || ((item, intents) => syncFacesAfterIntents(item, intents, cfg, log)); const ctx = { projectId: cfg.projectId, fields: cfg.fields }; @@ -263,6 +265,12 @@ function applyMergeProgression(items, cfg, state, log, deps = {}) { try { const applied = applyIntents(ctx, itemId, intents, token); log(`#${item.issue}: PR merged → ${applied.join(', ')}`); + // Fix A（#843）: 非デフォルト base 宛て PR（EPIC サブ）では GitHub の `Closes #N` 自動 + // close が効かないため、Project を Close へ進めたら GitHub issue も明示的に閉じる（冪等）。 + try { closeIssue(cfg.repo, item.issue, token); } + catch (e) { log(`#${item.issue}: gh issue close failed: ${e.message}`); } + // 反映済みを in-memory item にも映す → 同 tick の closed-reconcile が二重処理しない（冪等） + item.status = 'Close'; // merge は前進シグナル → 人間の番は解除。🙋 ラベルを両面から落とす（force 同期・#813）。 syncFaces({ ...item, hitlLabel: false }, intents); } catch (e) { @@ -271,6 +279,46 @@ function applyMergeProgression(items, cfg, state, log, deps = {}) { } } +/** + * Fix B（#843）: 「GitHub issue が closed ⇄ Project Status=Close」の整合パス。 + * merge-progression は leaf の連携 PR merge しか見ないため、(A) 非デフォルト base 宛て PR で手動 close + * した leaf、(B) 統合 PR の `Closes #` で閉じた EPIC、(C) 人手で閉じた issue が取り残される。 + * ここは closed という事実だけを根拠に整合する（EPIC も対象）。判定は phases.js の純粋関数 + * （selectClosedToReconcile）、I/O は project.js。実行中の item は触らない。1 件の失敗は他を止めない。 + * deps は injection 可能（テスト用）。 + */ +function applyClosedReconcile(items, cfg, state, log, deps = {}) { + const token = deps.token || project.botToken(); + const listClosed = deps.listClosedIssueNumbers || project.listClosedIssueNumbers; + const applyIntents = deps.applyIntents || project.applyIntents; + const findItemId = deps.findItemId || project.findItemId; + const syncFaces = deps.syncFaces || ((item, intents) => syncFacesAfterIntents(item, intents, cfg, log)); + let closedSet; + try { + closedSet = listClosed(cfg.repo, token); + } catch (e) { + log(`closed issue list failed: ${e.message}`); + return; + } + const ctx = { projectId: cfg.projectId, fields: cfg.fields }; + const intents = [ + { field: 'Status', value: 'Close' }, + { field: 'AI Status', value: null }, + ]; + for (const item of selectClosedToReconcile(items, closedSet)) { + if (state.running.has(item.issue)) continue; // live phase が所有中は触らない + const itemId = item.itemId || findItemId(cfg.owner, cfg.project, item.issue, token); + try { + const applied = applyIntents(ctx, itemId, intents, token); + log(`#${item.issue}: closed issue → ${applied.join(', ')}`); + // closed は終端シグナル → 人間の番は解除。🙋 ラベルを落とす（force 同期）。 + syncFaces({ ...item, status: 'Close', hitlLabel: false }, intents); + } catch (e) { + log(`#${item.issue}: closed reconcile failed: ${e.message}`); + } + } +} + /** * DoD 引き継ぎ生成（#821）: Status=DoD の leaf について、連携 PR に headful 検証手順の引き継ぎ * コメント（`autopilot:dod-handoff` マーカー付き）を **1 回だけ** 投稿する。コンテナ内 daemon は @@ -407,8 +455,10 @@ async function tick(cfg, state, log) { // fire-and-forget（running で重複防止） dispatch(item, cfg, state, log); } - // 人間が手動 merge した leaf を Close へ前進（自動 merge はしない） + // 人間が手動 merge した leaf を Close へ前進（自動 merge はしない）+ GitHub issue も close（#843 Fix A） applyMergeProgression(items, cfg, state, log); + // GitHub で closed な issue（EPIC・人手 close 含む）の Project Status を Close へ整合（#843 Fix B） + applyClosedReconcile(items, cfg, state, log); // In Progress + AI 作業中のまま止まった item を検知して Blocked へ（#816） detectStuck(items, cfg, state, log); // Status=DoD の leaf に headful 検証の引き継ぎコメントを 1 回だけ投稿（#821・冪等） @@ -549,6 +599,6 @@ async function main(opts = {}) { } module.exports = { - main, tick, runTickOnce, dispatch, applyMergeProgression, applyPrProjection, + main, tick, runTickOnce, dispatch, applyMergeProgression, applyClosedReconcile, applyPrProjection, applyDodHandoffs, detectStuck, markBlocked, }; diff --git a/tools/autopilot/src/phases.js b/tools/autopilot/src/phases.js index d31cc3c0ca..dfb11a02d5 100644 --- a/tools/autopilot/src/phases.js +++ b/tools/autopilot/src/phases.js @@ -180,6 +180,32 @@ function mergeProgressionIntents(item, prMerged) { ]; } +/** + * 終端 Status の集合。GitHub issue が close 済みでも、Project がこれらなら整合済みとみなす + * （Close は完了、Done は upstream 由来の完了。これ以外は GitHub と乖離なので reconcile 対象）。 + */ +const TERMINAL_STATUSES = new Set(['Close', 'Done']); + +/** + * GitHub で closed な issue のうち、Project Status がまだ終端（Close/Done）でない item を選ぶ + * （純粋関数・#843）。「GitHub issue 状態 → Project Close」の整合パス用。 + * + * merge-progression（{@link selectMergeCandidates}）は **leaf の連携 PR merge** だけを見るため、 + * (A) 非デフォルト base 宛て PR で GitHub の `Closes #N` が効かず手動 close した leaf、 + * (B) 統合 PR の `Closes #` で閉じた EPIC、(C) 人手で閉じた issue が Project に取り残される。 + * ここは **closed という事実だけ**を根拠に整合するので EPIC も対象に含める（子 PR merge とは別経路）。 + * 既に終端の item は除外（冪等）。実行中 item の除外は I/O 側（daemon）が行う。 + * @param {object[]} items 各 { issue, status, kind, ... } + * @param {Set|number[]} closedSet GitHub で closed な issue 番号の集合（配列も可） + * @returns {object[]} reconcile 対象（closed かつ非終端） + */ +function selectClosedToReconcile(items, closedSet) { + const closed = closedSet instanceof Set ? closedSet : new Set(closedSet || []); + return (items || []).filter( + (it) => it && closed.has(it.issue) && !TERMINAL_STATUSES.has(it.status), + ); +} + /** * PR の承認状態を導く（純粋関数・#815）。 * @@ -765,6 +791,8 @@ module.exports = { MERGE_CHECK_STATUSES, selectMergeCandidates, mergeProgressionIntents, + TERMINAL_STATUSES, + selectClosedToReconcile, computeReviewApproval, phaseForItem, isActionable, diff --git a/tools/autopilot/src/project.js b/tools/autopilot/src/project.js index cd6929a64a..b31b3fd293 100644 --- a/tools/autopilot/src/project.js +++ b/tools/autopilot/src/project.js @@ -435,6 +435,38 @@ function hasMergedPullRequest(repo, issueNumber, token, deps = {}) { return hasMergedHeadPr(JSON.parse(listOut)); } +/** + * GitHub で closed な issue 番号の集合を返す（#843）。`gh project item-list` の content には + * open/closed の state が無いため、Project とは別に issue 一覧から closed を引く。 + * @param {string} repo `owner/name` + * @param {string} token + * @param {{gh?:Function}} [deps] gh 実行を差し替え可能（テスト用） + * @returns {Set} closed な issue 番号の集合 + */ +function listClosedIssueNumbers(repo, token, deps = {}) { + const ghFn = deps.gh || gh; + const out = ghFn( + ['issue', 'list', '--repo', repo, '--state', 'closed', '--limit', '1000', '--json', 'number'], + { token }, + ); + const arr = JSON.parse(out); + return new Set((Array.isArray(arr) ? arr : []).map((i) => i.number)); +} + +/** + * Issue を close する（#843・冪等）。非デフォルト base 宛て PR では GitHub の `Closes #N` 自動 close が + * 効かないため、merge-progression が leaf を Close へ前進させたとき GitHub issue も明示的に閉じる。 + * 既に closed なら `gh issue close` は no-op で成功する（冪等）。 + * @param {string} repo `owner/name` + * @param {number} issueNumber + * @param {string} token + * @param {{gh?:Function}} [deps] gh 実行を差し替え可能（テスト用） + */ +function closeIssue(repo, issueNumber, token, deps = {}) { + const ghFn = deps.gh || gh; + ghFn(['issue', 'close', String(issueNumber), '--repo', repo], { token }); +} + /** applyResult が返す意図配列を Project に反映する */ function applyIntents(ctx, itemId, intents, token) { const applied = []; @@ -450,5 +482,5 @@ module.exports = { botLogin, findPrForIssue, selectClosingPr, selectHeadPr, hasMergedHeadPr, getPrReviewState, getReviewContext, hasMergedPullRequest, REPO_ROOT, getPrInfo, getIssueLabels, getIssueBody, editLabels, setPrDraft, upsertStickyComment, listIssueComments, - postIssueComment, + postIssueComment, listClosedIssueNumbers, closeIssue, }; diff --git a/tools/autopilot/test/daemon.test.js b/tools/autopilot/test/daemon.test.js index b910e373c8..20aa40d9b7 100644 --- a/tools/autopilot/test/daemon.test.js +++ b/tools/autopilot/test/daemon.test.js @@ -2,7 +2,8 @@ const { test } = require('node:test'); const assert = require('node:assert'); const { - applyMergeProgression, applyPrProjection, applyDodHandoffs, runTickOnce, detectStuck, markBlocked, + applyMergeProgression, applyClosedReconcile, applyPrProjection, applyDodHandoffs, runTickOnce, + detectStuck, markBlocked, } = require('../src/daemon'); const { HITL_LABEL, AUTOPILOT_LABEL } = require('../src/phases'); @@ -65,6 +66,120 @@ test('applyMergeProgression: closes leaf with merged PR; skips not-merged/EPIC/t assert.equal(m['AI Status'], null); }); +test('applyMergeProgression: also closes the GitHub issue (Fix A, #843)', () => { + // 非デフォルト base 宛て PR では GitHub の `Closes #N` 自動 close が効かないので、 + // leaf を Close へ前進させたら GitHub issue も明示 close する（冪等）。 + const items = [ + { issue: 1, itemId: 'i1', status: 'Review', kind: 'Issue' }, // merged -> Close + gh close + { issue: 2, itemId: 'i2', status: 'Review', kind: 'Issue' }, // not merged -> no close + ]; + const mergedMap = { 1: true, 2: false }; + const closed = []; + const state = { running: new Map() }; + applyMergeProgression(items, makeCfg(), state, () => {}, { + token: 't', + hasMergedPullRequest: (repo, issue) => mergedMap[issue], + applyIntents: (ctx, itemId, intents) => intents.map((i) => `${i.field}=${i.value}`), + findItemId: () => 'x', + closeIssue: (repo, issue) => closed.push(issue), + syncFaces: () => {}, + }); + assert.deepEqual(closed, [1]); +}); + +test('applyMergeProgression: a failing gh close does not abort the loop (#843)', () => { + const items = [ + { issue: 1, itemId: 'i1', status: 'Review', kind: 'Issue' }, + { issue: 2, itemId: 'i2', status: 'Review', kind: 'Issue' }, + ]; + const applied = []; + const state = { running: new Map() }; + applyMergeProgression(items, makeCfg(), state, () => {}, { + token: 't', + hasMergedPullRequest: () => true, + applyIntents: (ctx, itemId) => { applied.push(itemId); return []; }, + findItemId: () => 'x', + closeIssue: (repo, issue) => { if (issue === 1) throw new Error('boom'); }, + syncFaces: () => {}, + }); + assert.deepEqual(applied, ['i1', 'i2']); +}); + +test('applyClosedReconcile: closed non-terminal items -> Status=Close + clear AI Status (incl. EPIC)', () => { + const items = [ + { issue: 738, itemId: 'e1', status: 'Review', kind: 'EPIC' }, // closed EPIC -> reconcile + { issue: 839, itemId: 'i1', status: 'Review', kind: 'Issue' }, // closed leaf -> reconcile + { issue: 900, itemId: 'i2', status: 'Close', kind: 'Issue' }, // closed but terminal -> skip + { issue: 901, itemId: 'i3', status: 'In Progress', kind: 'Issue' }, // open on GitHub -> skip + ]; + const applied = []; + const faces = []; + const state = { running: new Map() }; + applyClosedReconcile(items, makeCfg(), state, () => {}, { + token: 't', + listClosedIssueNumbers: () => new Set([738, 839, 900]), + applyIntents: (ctx, itemId, intents) => { applied.push({ itemId, intents }); return []; }, + findItemId: () => 'x', + syncFaces: (item) => faces.push(item.issue), + }); + assert.deepEqual(applied.map((a) => a.itemId).sort(), ['e1', 'i1']); + for (const a of applied) { + const m = Object.fromEntries(a.intents.map((i) => [i.field, i.value])); + assert.equal(m.Status, 'Close'); + assert.equal(m['AI Status'], null); + } + assert.deepEqual(faces.sort((x, y) => x - y), [738, 839]); +}); + +test('applyClosedReconcile: skips running items (does not fight a live phase)', () => { + const items = [{ issue: 839, itemId: 'i1', status: 'Review', kind: 'Issue' }]; + const applied = []; + const state = { running: new Map([[839, { phase: 'review' }]]) }; + applyClosedReconcile(items, makeCfg(), state, () => {}, { + token: 't', + listClosedIssueNumbers: () => new Set([839]), + applyIntents: (ctx, itemId) => { applied.push(itemId); return []; }, + findItemId: () => 'x', + syncFaces: () => {}, + }); + assert.deepEqual(applied, []); +}); + +test('applyClosedReconcile: a failing listClosedIssueNumbers is a no-op (does not throw)', () => { + const items = [{ issue: 839, itemId: 'i1', status: 'Review', kind: 'Issue' }]; + const applied = []; + const state = { running: new Map() }; + assert.doesNotThrow(() => applyClosedReconcile(items, makeCfg(), state, () => {}, { + token: 't', + listClosedIssueNumbers: () => { throw new Error('rate limit'); }, + applyIntents: (ctx, itemId) => { applied.push(itemId); return []; }, + findItemId: () => 'x', + syncFaces: () => {}, + })); + assert.deepEqual(applied, []); +}); + +test('applyClosedReconcile: one failing item does not block others', () => { + const items = [ + { issue: 1, itemId: 'i1', status: 'Review', kind: 'Issue' }, // apply throws + { issue: 2, itemId: 'i2', status: 'Review', kind: 'Issue' }, // ok + ]; + const applied = []; + const state = { running: new Map() }; + applyClosedReconcile(items, makeCfg(), state, () => {}, { + token: 't', + listClosedIssueNumbers: () => new Set([1, 2]), + applyIntents: (ctx, itemId) => { + if (itemId === 'i1') throw new Error('boom'); + applied.push(itemId); + return []; + }, + findItemId: () => 'x', + syncFaces: () => {}, + }); + assert.deepEqual(applied, ['i2']); +}); + test('applyMergeProgression: skips items currently running (does not fight a live phase)', () => { const items = [{ issue: 1, itemId: 'i1', status: 'Review', kind: 'Issue' }]; const applied = []; diff --git a/tools/autopilot/test/phases.test.js b/tools/autopilot/test/phases.test.js index 50e21b0bf5..7b53d3771f 100644 --- a/tools/autopilot/test/phases.test.js +++ b/tools/autopilot/test/phases.test.js @@ -12,6 +12,8 @@ const { computeReviewApproval, mergeProgressionIntents, selectMergeCandidates, + selectClosedToReconcile, + TERMINAL_STATUSES, phaseForItem, isActionable, isStuckCandidate, @@ -215,6 +217,39 @@ test('mergeProgressionIntents: already at target Status -> no intents (idempoten assert.deepEqual(mergeProgressionIntents({ status: 'Close', kind: 'Issue' }, true), []); }); +test('selectClosedToReconcile: GitHub-closed items not yet at a terminal Status (incl. EPIC)', () => { + const items = [ + { issue: 1, status: 'Review', kind: 'Issue' }, // closed + non-terminal -> reconcile + { issue: 2, status: 'Close', kind: 'Issue' }, // closed but already terminal -> skip (idempotent) + { issue: 3, status: 'Done', kind: 'Issue' }, // closed but terminal -> skip + { issue: 4, status: 'Review', kind: 'EPIC' }, // closed EPIC + non-terminal -> reconcile (EPIC included) + { issue: 5, status: 'In Progress', kind: 'Issue' }, // open on GitHub -> skip + { issue: 6, status: undefined, kind: 'Issue' }, // closed, no Status -> reconcile + ]; + const closedSet = new Set([1, 2, 3, 4, 6]); + assert.deepEqual( + selectClosedToReconcile(items, closedSet).map((i) => i.issue), + [1, 4, 6], + ); +}); + +test('selectClosedToReconcile: accepts an array (not only a Set) as closedSet', () => { + const items = [{ issue: 7, status: 'DoD', kind: 'Issue' }]; + assert.deepEqual(selectClosedToReconcile(items, [7]).map((i) => i.issue), [7]); +}); + +test('selectClosedToReconcile: empty/missing inputs -> empty array', () => { + assert.deepEqual(selectClosedToReconcile([], new Set([1])), []); + assert.deepEqual(selectClosedToReconcile(null, null), []); + assert.deepEqual(selectClosedToReconcile([{ issue: 1, status: 'Review' }], new Set()), []); +}); + +test('TERMINAL_STATUSES: Close and Done are terminal', () => { + assert.ok(TERMINAL_STATUSES.has('Close')); + assert.ok(TERMINAL_STATUSES.has('Done')); + assert.ok(!TERMINAL_STATUSES.has('Review')); +}); + test('applyResult: done sets Status/Size/Kind, clears AI Status, no HITL field (#813)', () => { const intents = applyResult({ issue: 1, phase: 'triage', signal: 'done', summary: 's', diff --git a/tools/autopilot/test/project.test.js b/tools/autopilot/test/project.test.js index 38355db588..8d15b930c1 100644 --- a/tools/autopilot/test/project.test.js +++ b/tools/autopilot/test/project.test.js @@ -8,6 +8,8 @@ const { hasMergedHeadPr, findPrForIssue, hasMergedPullRequest, + listClosedIssueNumbers, + closeIssue, } = require('../src/project'); const { HITL_LABEL, AUTOPILOT_LABEL, autopilotHeadBranch } = require('../src/phases'); @@ -202,3 +204,27 @@ test('hasMergedPullRequest: false when neither close link nor head branch is mer }; assert.equal(hasMergedPullRequest('o/r', 5, 'tok', { gh: fakeGh }), false); }); + +test('listClosedIssueNumbers: returns a Set of closed issue numbers (#843)', () => { + let captured; + const fakeGh = (args) => { + captured = args; + return JSON.stringify([{ number: 738 }, { number: 839 }, { number: 840 }]); + }; + const set = listClosedIssueNumbers('smalruby/smalruby3-editor', 'tok', { gh: fakeGh }); + assert.ok(set instanceof Set); + assert.deepEqual([...set].sort((a, b) => a - b), [738, 839, 840]); + assert.ok(captured.includes('--state') && captured.includes('closed')); + assert.ok(captured.includes('--repo') && captured.includes('smalruby/smalruby3-editor')); +}); + +test('listClosedIssueNumbers: empty/non-array output -> empty Set', () => { + assert.deepEqual([...listClosedIssueNumbers('o/r', 't', { gh: () => '[]' })], []); + assert.deepEqual([...listClosedIssueNumbers('o/r', 't', { gh: () => 'null' })], []); +}); + +test('closeIssue: runs `gh issue close` for the given issue (#843)', () => { + let captured; + closeIssue('o/r', 839, 'tok', { gh: (args) => { captured = args; return ''; } }); + assert.deepEqual(captured, ['issue', 'close', '839', '--repo', 'o/r']); +});