diff --git a/lib/domains/archive.mjs b/lib/domains/archive.mjs index 8ebd0fa..c544e24 100644 --- a/lib/domains/archive.mjs +++ b/lib/domains/archive.mjs @@ -4,7 +4,7 @@ import { name } from '../input.mjs' import { pr } from '../github.mjs' import { buildIndex } from '../indexer.mjs' import { brief, write, xp } from '../brief.mjs' -import { repo } from '../git.mjs' +import { git, repo } from '../git.mjs' import { err } from '../errors.mjs' export async function planData(r, o) { @@ -24,5 +24,8 @@ export function execute(r, o, x, b) { mkdirSync(dirname(dest), { recursive: true }) renameSync(dirname(b.path), dest) writeFileSync(join(r, 'shadow-docs', 'INDEX.md'), buildIndex(r)) + // 归档落本地 commit:半落状态(INDEX 改 + 旧目录删 + 新目录未跟踪)对 AI 与人都易漏提交 + git(r, ['add', '--', 'shadow-docs/INDEX.md', `shadow-docs/changes/${n}`, `shadow-docs/changes/archive/${n}`]) + git(r, ['commit', '-m', `docs(shadow): 归档 ${n}——PR #${x.pullRequest} 已合入 main,brief 移入 archive 并重建 INDEX`]) return { path: xp(r, n) } } diff --git a/lib/domains/branch.mjs b/lib/domains/branch.mjs index 5ba84ba..cae7160 100644 --- a/lib/domains/branch.mjs +++ b/lib/domains/branch.mjs @@ -1,6 +1,7 @@ import { git, repo } from '../git.mjs' import { brief, write } from '../brief.mjs' import { name } from '../input.mjs' +import { err } from '../errors.mjs' export async function planData(r, o) { const n = name(o), b = brief(r, n), q = repo(r) @@ -8,7 +9,9 @@ export async function planData(r, o) { } export function execute(r, o, x, b) { - if (x.repo.branch === x.baseBranch) git(r, ['switch', '-c', x.branch]) + // 非 base 分支必须响亮失败:静默跳过会让 brief 记录 branched 而实际没建分支(状态与现实脱节) + if (x.repo.branch !== x.baseBranch) throw err('NOT_ON_BASE_BRANCH', `NOT_ON_BASE_BRANCH: current=${x.repo.branch || '(detached)'} base=${x.baseBranch}`) + git(r, ['switch', '-c', x.branch]) b.data.branch = x.branch b.data.status = 'branched' write(b) diff --git a/lib/domains/commit.mjs b/lib/domains/commit.mjs index 21052af..f575e47 100644 --- a/lib/domains/commit.mjs +++ b/lib/domains/commit.mjs @@ -5,7 +5,13 @@ import { commitStep } from '../steps.mjs' export async function planData(r, o) { const n = name(o), b = brief(r, n), q = repo(r) - return { name: n, files: fileList(o.files), message: o.message || null, brief: b.data, repo: q } + const saved = b.data.workflow.commit || {} + // 未重传参数时回退持久化值(release 同款模式):execute 免逐字重传 --files/--message + return { name: n, files: fileList(o.files ?? saved.files), message: o.message ?? saved.message ?? null, brief: b.data, repo: q } +} + +export function persistPlan(b, e) { + b.data.workflow.commit = { files: e.data.files, message: e.data.message } } export function execute(r, o, x, b) { diff --git a/lib/plan.mjs b/lib/plan.mjs index d6bce44..4941ce4 100644 --- a/lib/plan.mjs +++ b/lib/plan.mjs @@ -20,6 +20,7 @@ export function norm(d) { delete v.brief.workflow.planHash delete v.brief.workflow.release delete v.brief.workflow.issuePlan + delete v.brief.workflow.commit } if (d.repo && (d.repo.changedFiles !== undefined || d.repo.clean !== undefined)) { v = { ...v, repo: { ...v.repo } } diff --git a/shadow-docs/changes/20260925-fix-cli-silent-failures/brief.md b/shadow-docs/changes/20260925-fix-cli-silent-failures/brief.md new file mode 100644 index 0000000..8a72453 --- /dev/null +++ b/shadow-docs/changes/20260925-fix-cli-silent-failures/brief.md @@ -0,0 +1,90 @@ +--- +{ + "schema": "shadow-dev/v1", + "name": "20260925-fix-cli-silent-failures", + "type": "fix", + "scope": "cli", + "status": "proposed", + "baseBranch": "main", + "branch": null, + "files": [ + "lib/domains/archive.mjs", + "lib/domains/branch.mjs", + "lib/domains/commit.mjs", + "lib/domains/publish.mjs", + "lib/domains/release.mjs", + "lib/plan.mjs", + "shadow-docs/knowledge/plan-credential-chain.md", + "test/cli.test.mjs" + ], + "github": { + "repository": "stack-wuh/shadow-dev-cli", + "issue": 29, + "issueUrl": "https://github.com/stack-wuh/shadow-dev-cli/issues/29", + "pullRequest": null, + "pullRequestUrl": null + }, + "review": { + "conclusion": "pending", + "verifiedCommit": null, + "verifiedAt": null + }, + "workflow": { + "operation": null, + "checkpoint": "issue:29", + "planHash": "3d24545d8c604cb3e39b76a8244ff50b5ee3dab7ae3e1b4755d788d919f905a4", + "updatedAt": null, + "lastError": null, + "issuePlan": { + "title": "[fix] CLI 静默失败修复:branch 门禁 / archive 落 commit / files 顺序归一", + "body": "", + "labels": [ + "fix" + ] + } + } +} +--- + +# CLI 静默失败修复:branch 门禁 / archive 落 commit / files 顺序归一 + +## 动机 + +实证复盘(20260924-feature-git-history-capsule 全链路)暴露三个 CLI 静默失败或易错点:① `branch execute` 在非 base 分支时静默跳过建分支,仍报 ok:true 并把 brief 状态写为 branched——状态与现实脱节,发现时需手工补救(本次被迫手工建 worktree + 分支);② `archive execute` 只 rename + 重建 INDEX,不落 commit,留下「INDEX 改 + 旧 brief 删 + 新目录未跟踪」半落状态,每次归档都要手工补三路径 add + commit;③ `commit/publish/release` 的 `--files` 数组在 planData 归一后与用户传参顺序脱钩(canon 只排对象键不排数组),execute 必须复现归一后的顺序才不炸 `PLAN_HASH_INVALID`——顺序敏感且不可发现(本次 plan 传原始顺序、按 nextStep 的排序形式 execute 才通过)。 + +## 引用规范 + +- shadow-docs/knowledge/plan-credential-chain.md + - 当前结论: planHash = sha256(canon({command, data: norm(planData)}));norm 剥离 plan 副作用(workflow 凭证字段、repo.changedFiles/clean);带 --name 的域以 brief 持久化 planHash 为准 + - 适用 scope: lib/plan.mjs, lib/domains/branch.mjs, lib/domains/commit.mjs, lib/domains/archive.mjs +- shadow-docs/knowledge/cli-output-contract.md + - 当前结论: stdout 单行 JSON 为机器契约(公开 API 需测试钉住),人读内容只走 stderr;错误 code 与 nextStep 模板不本地化 + - 适用 scope: lib/domains/branch.mjs, lib/domains/archive.mjs + +## 决策 + +- **选型:** ① branch execute 的 else 分支(非 base 分支)改为抛 `NOT_ON_BASE_BRANCH` 错误(附 current/base 分支信息),不再写 brief 状态;② archive execute 在 rename + buildIndex 后追加 `git add`(INDEX.md、changes/、changes/archive/ 三显式路径)+ 本地 commit(message 惯例:`docs(shadow): 归档 ——PR #N 已合入 main,brief 移入 archive 并重建 INDEX`),nextStep 输出 push 命令;③ commit 域补 `persistPlan`(持久化 workflow.commit = {files, message},release 域同款模式),norm() 同步剥离该字段,execute 无需重传 --files/--message。 +- **对比方案:** branch 静默跳过改为自动 git worktree add(worktree CLI 化)——有价值的演进但涉及 brief 权威副本归属设计,另立 change,本次不捆绑;files 归一改为「execute 从 brief 持久化参数解析」(issue 模式推广)——改动面大且排序归一已消除实际痛点,延期。 +- **理由:** 三处均为确定性本地行为的修正,不碰网络/发布语义;排序归一在 planData 内联完成使 plan/execute 天然一致,比要求用户复现归一顺序更符合「哈希防篡改而非防手滑」的既有设计意图(plan-credential-chain 卡的原话)。 + +**实现期前提修正(apply 记录)**:原任务③「files 数组顺序敏感导致 PLAN_HASH_INVALID」在实现期被证伪——`lib/input.mjs` 的 `fileList` 一直带 `.sort()`,顺序敏感不存在;真因是 commit execute 未重传 `--files/--message` 时 planData 以空参重算导致哈希不匹配(用法问题暴露 ergonomics 缺口)。任务③按此修正为 commit persistPlan。 + +## 任务 +## 任务 + +### Phase 1 +- [ ] branch execute 非 base 分支抛 NOT_ON_BASE_BRANCH(含 current/base 与建议命令),红→绿 — `lib/domains/branch.mjs` — 修改 +- [ ] archive execute 追加三路径 git add + 本地 commit + nextStep push 提示,红→绿 — `lib/domains/archive.mjs` — 修改 +- [ ] commit 域 persistPlan(workflow.commit 持久化 + norm 剥离 + planData 回退),execute 免重传参数,红→绿 — `lib/domains/commit.mjs`, `lib/plan.mjs` — 修改 +- [ ] `node --test test/cli.test.mjs` 全绿;同步 plan-credential-chain 卡(数组归一边界)与 verified + +## 结果 + +- 实际耗时: — +- 验证: — + +## 知识评估 + +- **预期影响:** 更新 +- **候选卡片:** shadow-docs/knowledge/plan-credential-chain.md +- **理由:** files 排序归一属哈希边界(norm)的新增条目,须原位更新该卡并在任务内同步;branch/archive 的行为修正属 bug 级,无新稳定事实。 diff --git a/shadow-docs/knowledge/plan-credential-chain.md b/shadow-docs/knowledge/plan-credential-chain.md index 5909131..677af7c 100644 --- a/shadow-docs/knowledge/plan-credential-chain.md +++ b/shadow-docs/knowledge/plan-credential-chain.md @@ -6,7 +6,8 @@ scope: [lib/plan.mjs, cli.mjs, lib/git.mjs] status: active source: - changes/20260917-fix-plan-credential-chain/brief.md -verified: 2026-09-17 + - changes/20260925-fix-cli-silent-failures/brief.md +verified: 2026-09-25 --- # plan/execute 凭证链与哈希边界 @@ -20,7 +21,7 @@ verified: 2026-09-17 - 新增命令域导出 `planData` 时,凡包含 `repo` 状态,其 `changedFiles`/`clean` 已被 `norm()` 剥离,无需自行处理;不得为了「哈希稳定」把 head/branch 也剥掉。 - 脏工作区的行为门禁(如 `sync` 的 `DIRTY_WORKTREE`)必须在命令域 `planData` 内联执行——plan 与 execute 都会跑一次,这是哈希剥离脏状态后唯一的脏检查通道,不得移入哈希输入。 - `git()` 输出只做尾部裁剪(`trimEnd`):`git status --porcelain` 状态行以空格开头(` M path`),消费方从下标 3 取路径,全局 `trim()` 会截掉首行路径首字符。 -- execute 端若命令的 `planData` 依赖 flag(如 `commit`/`publish`/`release` 的 `--files`/`--message`/`--title`/`--body`),execute 必须传与 plan 完全一致的参数,否则 `PLAN_HASH_INVALID` 属预期行为。 +- execute 端若命令的 `planData` 依赖 flag 且该 flag **未持久化**(如 `publish`/`release` 的 `--title`/`--body`),execute 必须传与 plan 完全一致的参数,否则 `PLAN_HASH_INVALID` 属预期行为。已持久化的 flag 不受此限:`commit` 的 `--files`/`--message` 经 `workflow.commit` 持久化(`persistPlan` + `norm` 剥离,20260925-fix-cli-silent-failures 起),`release` 的经 `workflow.release`——execute 可只传 `--name --confirm`。新命令域引入可持久化 flag 时必须三处同步:`persistPlan` 写回、`norm()` 剥离、`planData` 回退读取。 ## 适用边界 diff --git a/test/cli.test.mjs b/test/cli.test.mjs index e088290..5fcbf3c 100644 --- a/test/cli.test.mjs +++ b/test/cli.test.mjs @@ -841,3 +841,62 @@ test('release execute commits, pushes and creates the PR in one step, then reuse assert.deepEqual(reuse.requests().map(request => request.method), ['GET']) } finally { reuse.close() } }) + +// ---- 20260925-fix-cli-silent-failures:三类静默失败/易错点的行为契约 ---- + +test('branch execute on a non-base branch fails loudly and leaves the brief untouched', () => { + const root = fixture() + execFileSync('git', ['add', '--', 'shadow-docs'], { cwd: root }) + execFileSync('git', ['commit', '-m', 'docs: seed briefs'], { cwd: root }) + execFileSync('git', ['switch', '-c', 'drift'], { cwd: root }) + const planned = run(['branch', 'plan', '--name', 'sample', '--json'], root) + assert.equal(planned.status, 0, planned.stderr) + const result = run(['branch', 'execute', '--name', 'sample', '--plan-hash', JSON.parse(planned.stdout).planHash, '--confirm', '--json'], root) + assert.equal(result.status, 1) + assert.equal(JSON.parse(result.stdout).error.code, 'NOT_ON_BASE_BRANCH') + const text = readFileSync(join(root, 'shadow-docs', 'changes', 'sample', 'brief.md'), 'utf8') + assert.doesNotMatch(text, /"status": "branched"/) + assert.doesNotMatch(text, /"branch": "feat\/sample"/) +}) + +test('archive execute lands the archive move as a local commit', () => { + const root = fixture() + const head = () => execFileSync('git', ['rev-parse', 'HEAD'], { cwd: root }).toString().trim() + execFileSync('git', ['add', '--', 'shadow-docs'], { cwd: root }) + execFileSync('git', ['commit', '-m', 'docs: seed briefs'], { cwd: root }) + updateBrief(root, (d) => { + d.status = 'published' + d.github = { repository: 'owner/repo', issue: null, issueUrl: null, pullRequest: 9, pullRequestUrl: 'https://github.test/pull/9' } + d.review = { conclusion: 'passed', verifiedCommit: head(), verifiedAt: '2026-01-01T00:00:00.000Z' } + }) + const api = apiStub([{ method: 'GET', path: '/repos/owner/repo/pulls/9', body: { number: 9, merged: true, state: 'closed' } }]) + try { + const planned = run(['archive', 'plan', '--name', 'sample', '--json'], root, { GITHUB_TOKEN: 'token', SHADOW_GITHUB_API_URL: api.url }) + assert.equal(planned.status, 0, planned.stderr) + const before = head() + const result = run(['archive', 'execute', '--name', 'sample', '--plan-hash', JSON.parse(planned.stdout).planHash, '--confirm', '--json'], root, { GITHUB_TOKEN: 'token', SHADOW_GITHUB_API_URL: api.url }) + assert.equal(result.status, 0, result.stderr) + const after = head() + assert.notEqual(after, before, 'archive must land a local commit') + const message = execFileSync('git', ['log', '-1', '--format=%B'], { cwd: root }).toString() + assert.match(message, /docs\(shadow\): 归档 sample——PR #9 已合入 main,brief 移入 archive 并重建 INDEX/) + const inspect = run(['repo', 'inspect', '--json'], root) + assert.deepEqual(JSON.parse(inspect.stdout).data.changedFiles, [], 'archive commit must leave the tree clean') + } finally { + api.close() + } +}) + +test('commit execute reuses persisted files and message without re-passing them', () => { + const root = fixture() + writeFileSync(join(root, 'a.js'), 'a\n') + writeFileSync(join(root, 'b.js'), 'b\n') + execFileSync('git', ['add', '--', 'shadow-docs'], { cwd: root }) + execFileSync('git', ['commit', '-m', 'docs: seed briefs'], { cwd: root }) + const planned = run(['commit', 'plan', '--name', 'sample', '--files', 'a.js,b.js', '--message', 'feat: two files', '--json'], root) + assert.equal(planned.status, 0, planned.stderr) + const result = run(['commit', 'execute', '--name', 'sample', '--confirm', '--json'], root) + assert.equal(result.status, 0, result.stdout) + const message = execFileSync('git', ['log', '-1', '--format=%B'], { cwd: root }).toString() + assert.match(message, /feat: two files/) +})