From 617a3eef725ffa43aeaa344bb767e9c0260216d2 Mon Sep 17 00:00:00 2001 From: Jeffery Date: Thu, 17 Sep 2026 17:06:07 +0800 Subject: [PATCH] =?UTF-8?q?fix(pr-watch):=20=E8=A9=A6=E8=B7=91=E7=9A=84?= =?UTF-8?q?=E9=A0=90=E5=91=8A=E8=88=87=E5=AF=A6=E8=B7=91=E7=9A=84=E5=AE=88?= =?UTF-8?q?=E9=96=80=E5=B0=8D=E9=BD=8A=EF=BC=8C=E4=B8=A6=E8=AA=AA=E5=87=BA?= =?UTF-8?q?=E8=B7=AF=E5=BE=91=E8=A2=AB=E4=BD=94=E4=BD=8F=E9=80=99=E4=BB=B6?= =?UTF-8?q?=E4=BA=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit review 抓到一處落差：試跑判「終止且工作樹乾淨」就預告 git worktree remove， 但實跑多判一件事——那條路徑上的東西是不是真的一棵工作樹。別的 clone 在同一條路徑上 留下目錄時（路徑由 owner/repo/分支名 推導，不含本機 clone 的位置），試跑會預告一行 實跑必然拒絕的指令，而回報裡完全看不出原因。 判斷收進 lib 的 inspectWorktree，由它直接給出 reason（missing／foreign／dirty／ removable）：試跑與實跑、自動與手動四條路徑從此擋在同一個判斷上，worktree-remove 裡 那份重算的副本也跟著刪掉。回報多一個「是工作樹」欄位，手動出口的 NOT_A_WORKTREE 不再是使用者第一次聽到這件事。 順帶兩件同源的修正： - 「是不是工作樹」改認 .git 為**檔案**。獨立 clone 的 .git 是目錄，先前會被當成工作樹， 然後在 git worktree remove 那一步炸出一句原始錯誤。 - 兩支腳本的 --dry-run 請求預告收進 pr-threads 的 plannedRequests，並補上 pr-watch 先前漏掉的那句說明（逐則 reaction 與逐個 review 的行內留言事前列不完）。預告與實際 發出的請求分開寫，加一個端點就會有一邊忘了改。 - PR 讀不到 head 分支時給出 PULL_HEAD_MISSING，而不是拿空字串去推導一條路徑。 議題 #41 Co-Authored-By: Claude Opus 5 (1M context) --- scripts/lib.js | 51 ++++++++++++++++++++++---------- scripts/pr-comments.js | 21 ++++++------- scripts/pr-threads.js | 23 +++++++++++++++ scripts/pr-watch.js | 57 +++++++++++++++++++++++------------- scripts/worktree-remove.js | 13 ++------ test/pr-watch.test.js | 35 +++++++++++++++++++++- test/worktree-remove.test.js | 12 ++++++++ 7 files changed, 152 insertions(+), 60 deletions(-) diff --git a/scripts/lib.js b/scripts/lib.js index 74d3964..ca2fb8b 100644 --- a/scripts/lib.js +++ b/scripts/lib.js @@ -13,7 +13,7 @@ */ import { execFileSync } from 'node:child_process'; import { createHash } from 'node:crypto'; -import { accessSync, constants, existsSync, readFileSync } from 'node:fs'; +import { accessSync, constants, existsSync, readFileSync, statSync } from 'node:fs'; import { homedir } from 'node:os'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -447,22 +447,22 @@ export function openGitRepo(path) { } /** - * 看一棵工作樹現在是什麼狀況。 + * 看一棵工作樹現在是什麼狀況，並直接說出「能不能清掉、不能的話卡在哪」。 * - * 清理前要先知道三件事：它還在不在、裡面有沒有沒提交的東西、以及那條路徑上的東西 - * 到底是不是一棵工作樹（別的 clone 也可能在同一條路徑上留下目錄——路徑由 - * owner/repo/分支名 推導，不含本機 clone 的位置）。 + * 判斷寫在這裡而不是各呼叫端：試跑與實跑、自動與手動都要擋在同一個地方， + * 兩份判斷遲早會分岔成「試跑說清得掉、實跑卻拒絕」。 + * + * 那條路徑上的東西不一定是工作樹：路徑由 owner/repo/分支名 推導，不含本機 clone 的 + * 位置，所以別的 clone 也可能在同一條路徑上留下東西。 * * @param {string} worktree 推導出的工作樹路徑 - * @returns {{path: string, exists: boolean, worktree: boolean, dirty: boolean, files: string[]}} + * @returns {{path: string, exists: boolean, isWorktree: boolean, dirty: boolean, + * files: string[], reason: 'missing'|'foreign'|'dirty'|'removable'}} */ export function inspectWorktree(worktree) { - const 空的 = { path: worktree, exists: false, worktree: false, dirty: false, files: [] }; - if (!existsSync(worktree)) return 空的; - // 工作樹的 .git 是一個檔案（指回主 repo），不是目錄；沒有它就不是 git 認得的工作樹 - if (!existsSync(join(worktree, '.git'))) { - return { ...空的, exists: true }; - } + const 空的 = { path: worktree, exists: false, isWorktree: false, dirty: false, files: [] }; + if (!existsSync(worktree)) return { ...空的, reason: 'missing' }; + if (!linkedWorktree(worktree)) return { ...空的, exists: true, reason: 'foreign' }; const files = runGit(['status', '--porcelain'], { cwd: worktree }) .split('\n') @@ -472,7 +472,27 @@ export function inspectWorktree(worktree) { // 切太多會讓檔名少一個字（README.md 變成 EADME.md），人照著去找會找不到。 .map((line) => line.replace(/^\s*\S{1,2}\s+/, '')); - return { path: worktree, exists: true, worktree: true, dirty: files.length > 0, files }; + return { + path: worktree, + exists: true, + isWorktree: true, + dirty: files.length > 0, + files, + reason: files.length > 0 ? 'dirty' : 'removable', + }; +} + +/** + * 這條路徑是不是一棵「連結出去的」工作樹。 + * 認的是 `.git` 為**檔案**（裡面一行 gitdir 指回主 repo）——獨立 clone 的 `.git` 是目錄， + * 對它下 `git worktree remove` 只會得到一句 git 的原始錯誤，而那不是使用者要的答案。 + */ +function linkedWorktree(worktree) { + try { + return statSync(join(worktree, '.git')).isFile(); + } catch { + return false; + } } /** @@ -487,12 +507,11 @@ export function inspectWorktree(worktree) { * * @param {string} worktree 推導出的工作樹路徑 * @returns {{removed: boolean, reason: 'removed'|'missing'|'dirty'|'foreign', files: string[], path: string}} + * reason 由 inspectWorktree 給，兩支腳本與試跑、實跑都擋在同一個判斷上 */ export function removeWorktree(worktree) { const state = inspectWorktree(worktree); - if (!state.exists) return { ...state, removed: false, reason: 'missing' }; - if (!state.worktree) return { ...state, removed: false, reason: 'foreign' }; - if (state.dirty) return { ...state, removed: false, reason: 'dirty' }; + if (state.reason !== 'removable') return { ...state, removed: false }; // 在工作樹自己裡面執行：它的 .git 指得回主 repo，呼叫端因此不必知道主 clone 在哪 runGit(['worktree', 'remove', worktree], { cwd: worktree }); diff --git a/scripts/pr-comments.js b/scripts/pr-comments.js index 53994bd..f327a58 100644 --- a/scripts/pr-comments.js +++ b/scripts/pr-comments.js @@ -21,7 +21,13 @@ import { preflight, resolveLogin, } from './lib.js'; -import { fetchPull, readPullComments, unhandledCount } from './pr-threads.js'; +import { + COMMENT_REQUEST_NOTE, + fetchPull, + plannedRequests, + readPullComments, + unhandledCount, +} from './pr-threads.js'; main(async () => { const flags = parseFlags(process.argv.slice(2), { @@ -31,23 +37,14 @@ main(async () => { }); const repo = parseRepo(flags.repo); const index = parseIndex(flags.index); - const pullPath = `/repos/${repo}/pulls/${index}`; if (flags['dry-run']) { return { dryRun: true, repo, index, - requests: [ - { method: 'GET', path: '/user' }, - { method: 'GET', path: pullPath }, - { method: 'GET', path: `/repos/${repo}/issues/${index}/comments` }, - { method: 'GET', path: `${pullPath}/reviews` }, - { method: 'GET', path: `/repos/${repo}/issues/${index}/timeline` }, - ], - note: - '每則一般留言還會各查一次 reaction、每個 review 還會各查一次它的行內留言；' + - '次數取決於留言數，事前無法列舉。', + requests: plannedRequests(repo, index), + note: COMMENT_REQUEST_NOTE, }; } diff --git a/scripts/pr-threads.js b/scripts/pr-threads.js index 2d4adb2..b7b96a6 100644 --- a/scripts/pr-threads.js +++ b/scripts/pr-threads.js @@ -38,6 +38,29 @@ export async function readPullComments(login, repo, index, me) { ]; } +/** + * 讀這三類留言會發出哪些請求。兩支腳本的 `--dry-run` 都印它——預告與實際發出的請求 + * 分開寫，加一個端點就會有一邊忘了改，而預告錯了等於沒有預告。 + * @returns {{method: string, path: string}[]} + */ +export function plannedRequests(repo, index) { + const pullPath = `/repos/${repo}/pulls/${index}`; + return [ + { method: 'GET', path: '/user' }, + { method: 'GET', path: pullPath }, + { method: 'GET', path: `/repos/${repo}/issues/${index}/comments` }, + { method: 'GET', path: `${pullPath}/reviews` }, + { method: 'GET', path: `/repos/${repo}/issues/${index}/timeline` }, + ]; +} + +/** + * `plannedRequests` 列不完的那部分。與 readPullComments 同進退——說明的是它發出的請求。 + */ +export const COMMENT_REQUEST_NOTE = + '每則一般留言還會各查一次 reaction、每個 review 還會各查一次它的行內留言；' + + '次數取決於留言數，事前無法列舉。'; + /** 還沒被處理的則數。`/sdlc-fix` 要做的量，也是 `pr-watch` 的建議動作的依據。 */ export function unhandledCount(留言) { return 留言.filter((comment) => !comment.已處理).length; diff --git a/scripts/pr-watch.js b/scripts/pr-watch.js index 07e0619..eba0f9d 100644 --- a/scripts/pr-watch.js +++ b/scripts/pr-watch.js @@ -24,6 +24,7 @@ * node scripts/pr-watch.js --repo owner/name --index 46 [--host <網址>] [--dry-run] */ import { + ScriptError, expectOk, giteaRequest, inspectWorktree, @@ -36,7 +37,13 @@ import { resolveLogin, worktreePath, } from './lib.js'; -import { fetchPull, readPullComments, unhandledCount } from './pr-threads.js'; +import { + COMMENT_REQUEST_NOTE, + fetchPull, + plannedRequests, + readPullComments, + unhandledCount, +} from './pr-threads.js'; main(async () => { const flags = parseFlags(process.argv.slice(2), { @@ -58,7 +65,14 @@ main(async () => { const 未處理留言數 = unhandledCount(await readPullComments(login, repo, index, me)); // 工作樹由 PR 自己的 head 分支推導，不必另外給——同一顆工作包算出來的永遠是同一條路徑 - const branch = pull.head?.ref ?? ''; + const branch = pull.head?.ref; + if (!branch) { + throw new ScriptError( + 'PULL_HEAD_MISSING', + `PR #${index} 讀不到 head 分支（來源分支可能已經被刪掉），推導不出工作樹在哪；` + + '請改用 worktree-remove --branch 指名要清哪一棵', + ); + } const worktree = worktreePath(repo, branch); const terminal = state === 'merged' || state === 'closed'; @@ -66,6 +80,8 @@ main(async () => { const 清理 = terminal && !dryRun ? removeWorktree(worktree) : null; const 工作樹 = 清理 ?? inspectWorktree(worktree); const cleaned = 清理?.removed === true; + // 試跑要預告的那一行，條件與實跑完全同一個：reason 由 lib 算，兩邊不各判一次 + const 清得掉 = 工作樹.reason === 'removable'; const 報告 = { repo, @@ -77,14 +93,17 @@ main(async () => { 未處理留言數, 工作樹: { 路徑: worktree, - // 清掉之後這兩個欄位講的是清理之前的狀況：cleaned 已經說了現在還在不在 + // 清掉之後這幾個欄位講的是清理之前的狀況：cleaned 已經說了現在還在不在 存在: cleaned ? false : 工作樹.exists, + // 路徑上有東西卻不是工作樹（多半是別的 clone 留下的）時，清理不會發生也不該 + // 靜靜跳過——手動出口會給出 NOT_A_WORKTREE，這個欄位是它的前情提要 + 是工作樹: 工作樹.isWorktree, 有未提交變更: 工作樹.dirty, 檔案: 工作樹.files, }, terminal, cleaned, - suggestedAction: suggest({ terminal, cleaned, dryRun, 未處理留言數, 工作樹 }), + suggestedAction: suggest({ terminal, cleaned, 清得掉, 未處理留言數, 工作樹 }), }; if (!dryRun) return 報告; @@ -92,15 +111,10 @@ main(async () => { return { dryRun: true, ...報告, - requests: [ - { method: 'GET', path: '/user' }, - { method: 'GET', path: `/repos/${repo}/pulls/${index}` }, - { method: 'GET', path: `/repos/${repo}/issues/${index}/comments` }, - { method: 'GET', path: `/repos/${repo}/pulls/${index}/reviews` }, - { method: 'GET', path: `/repos/${repo}/issues/${index}/timeline` }, - ], + requests: plannedRequests(repo, index), + note: COMMENT_REQUEST_NOTE, // 讀取是冪等的，試跑照樣發；會改變東西的只有這一行，所以只有它被留到這裡 - commands: terminal && 工作樹.exists && !工作樹.dirty ? [`git worktree remove ${worktree}`] : [], + commands: terminal && 清得掉 ? [`git worktree remove ${worktree}`] : [], }; }); @@ -119,14 +133,15 @@ function stateOf(pull) { /** * 下一步該做什麼，固定四個值。 * - * 順序有意義：有東西擋住清理時要先講那件事，因為它需要人動手；未處理的留言其次； - * 都沒有就是真的沒事。試跑時「該清而還沒清」講的是 cleanup——它是這次不做的那件事。 + * 終止的 PR 只問清理這件事：有沒提交的東西卡著就 blocked-dirty（要人自己處理）， + * 清掉了或本來就不在就沒事了，其餘都還有一棵樹等著清——試跑不動手，路徑上是別的 + * clone 留下的東西也一樣，兩種都落在 cleanup，由手動出口給出確切的原因。 + * + * 還沒終止的 PR 只問留言：有沒處理完的就建議去跑 /sdlc-fix，但只是建議。 */ -function suggest({ terminal, cleaned, dryRun, 未處理留言數, 工作樹 }) { - if (terminal) { - if (工作樹.exists && 工作樹.dirty) return 'blocked-dirty'; - if (dryRun && 工作樹.exists && 工作樹.worktree) return 'cleanup'; - return cleaned || !工作樹.exists ? 'nothing-to-do' : 'cleanup'; - } - return 未處理留言數 > 0 ? 'run-sdlc-fix' : 'nothing-to-do'; +function suggest({ terminal, cleaned, 清得掉, 未處理留言數, 工作樹 }) { + if (!terminal) return 未處理留言數 > 0 ? 'run-sdlc-fix' : 'nothing-to-do'; + if (工作樹.reason === 'dirty') return 'blocked-dirty'; + if (cleaned || 工作樹.reason === 'missing') return 'nothing-to-do'; + return 清得掉 || 工作樹.reason === 'foreign' ? 'cleanup' : 'nothing-to-do'; } diff --git a/scripts/worktree-remove.js b/scripts/worktree-remove.js index cc2b59c..7d42ca5 100644 --- a/scripts/worktree-remove.js +++ b/scripts/worktree-remove.js @@ -39,14 +39,14 @@ main(async () => { // 試跑與實跑走同一條守門：試跑印得出漂亮的計畫、實跑卻被擋下來，是最難查的那種落差 if (flags['dry-run']) { const state = inspectWorktree(worktree); - checkRemovable({ ...state, reason: reasonOf(state) }, worktree); + checkRemovable(state, worktree); return { dryRun: true, repo, branch, worktree, - 已經不在: !state.exists, - commands: state.exists ? [`git worktree remove ${worktree}`] : [], + 已經不在: state.reason === 'missing', + commands: state.reason === 'removable' ? [`git worktree remove ${worktree}`] : [], }; } @@ -64,13 +64,6 @@ main(async () => { }); -/** inspectWorktree 的結果換算成 removeWorktree 用的同一組原因，讓試跑與實跑擋在同一處 */ -function reasonOf(state) { - if (!state.exists) return 'missing'; - if (!state.worktree) return 'foreign'; - return state.dirty ? 'dirty' : 'removed'; -} - /** 擋下來的兩種情況各有各的下一步，錯誤碼要分得開 */ function checkRemovable(result, worktree) { if (result.reason === 'dirty') { diff --git a/test/pr-watch.test.js b/test/pr-watch.test.js index 0c78f72..6832689 100644 --- a/test/pr-watch.test.js +++ b/test/pr-watch.test.js @@ -223,6 +223,33 @@ test('工作樹早就不在時不當成失敗，也不說自己清了', async (t assert.equal(json.data.suggestedAction, 'nothing-to-do'); }); +test('路徑上是別的 clone 留下的東西時，說出來而不是靜靜跳過', async (t) => { + const { repo, run, worktree } = await withScene(t, { state: 'closed', merged: true }); + repo.git('worktree', 'remove', worktree); + mkdirSync(join(worktree, '.git'), { recursive: true }); + + const { code, json } = await run(); + + assert.equal(code, 0); + assert.equal(json.data.工作樹.是工作樹, false, '.git 是目錄的是獨立 clone，不是工作樹'); + assert.equal(json.data.cleaned, false); + assert.equal(json.data.suggestedAction, 'cleanup', '要人動手，而手動出口會說出確切的原因'); + assert.equal(existsSync(join(worktree, '.git')), true, '不是我們建的東西就不碰'); +}); + +test('清不掉的路徑，--dry-run 不會預告一行實跑會拒絕的指令', async (t) => { + // 試跑印得出漂亮的計畫、實跑卻被擋下來，是最難查的那種落差 + const { repo, run, worktree } = await withScene(t, { state: 'closed', merged: true }); + repo.git('worktree', 'remove', worktree); + mkdirSync(worktree, { recursive: true }); + writeFileSync(join(worktree, '別人的東西.txt'), 'x\n'); + + const { json } = await run(['--dry-run']); + + assert.deepEqual(json.data.commands, []); + assert.equal(json.data.工作樹.是工作樹, false); +}); + // ── 回報內容 ─────────────────────────────────────────────────────── test('回報內容含 PR 狀態、未處理留言數與工作樹現況', async (t) => { @@ -234,7 +261,13 @@ test('回報內容含 PR 狀態、未處理留言數與工作樹現況', async ( assert.equal(json.data.url, `https://gitea.jsc.idv.tw/${REPO}/pulls/${INDEX}`); assert.equal(json.data.branch, BRANCH, '工作樹是從 PR 的 head 分支推導的，要說出用的是哪一支'); assert.equal(json.data.未處理留言數, 1); - assert.deepEqual(json.data.工作樹, { 路徑: worktree, 存在: true, 有未提交變更: false, 檔案: [] }); + assert.deepEqual(json.data.工作樹, { + 路徑: worktree, + 存在: true, + 是工作樹: true, + 有未提交變更: false, + 檔案: [], + }); }); test('建議動作是固定的列舉值，呼叫端才能程式化判斷', async (t) => { diff --git a/test/worktree-remove.test.js b/test/worktree-remove.test.js index 45def73..433f693 100644 --- a/test/worktree-remove.test.js +++ b/test/worktree-remove.test.js @@ -152,6 +152,18 @@ test('--repo 格式不是 owner/name 時失敗', async (t) => { assert.equal(json.error.code, 'BAD_REPO'); }); +test('路徑上是一個獨立的 clone 時也擋下：對它下 worktree remove 只會得到一句 git 的原始錯誤', async (t) => { + const { repo, home, branch, worktree } = await withWorktree(t); + repo.git('worktree', 'remove', worktree); + mkdirSync(join(worktree, '.git'), { recursive: true }); + + const { code, json } = await run(home, ['--repo', REPO, '--branch', branch]); + + assert.equal(code, 1); + assert.equal(json.error.code, 'NOT_A_WORKTREE'); + assert.equal(existsSync(join(worktree, '.git')), true); +}); + test('推導出來的路徑上是別人的東西時不碰它', async (t) => { const { repo, home, branch, worktree } = await withWorktree(t); repo.git('worktree', 'remove', worktree);