Skip to content

feat: add current-branch isolation mode#203

Closed
1919chichi wants to merge 26 commits into
rpamis:masterfrom
1919chichi:codex/current-branch-isolation
Closed

feat: add current-branch isolation mode#203
1919chichi wants to merge 26 commits into
rpamis:masterfrom
1919chichi:codex/current-branch-isolation

Conversation

@1919chichi

@1919chichi 1919chichi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

✨ Summary

Closes #190

Adds an isolation: current mode to the Classic workflow state machine (alongside the existing branch/worktree), so users can continue on the current branch without creating a new branch or worktree.

  • /comet-build offers current as a third isolation option; /comet-hotfix and /comet-tweak now require an explicit branch/current choice instead of silently defaulting to a new branch.
  • /comet-verify and /comet-archive document the current-branch handoff and archive-commit path (push/no-push decision, no finishing-a-development-branch skill invocation) consistently in English and Chinese Skills.
  • comet state/comet guard accept and validate the new isolation: current value; hotfix/tweak presets reject worktree.
  • Fixes found during self-review before opening this PR: a crash when a preset workflow entered build phase before isolation was chosen, a validation gap that let worktree slip through for hotfix/tweak, a detached-HEAD display bug and redundant I/O in comet state current, and a dropped decision-point sentence in comet-verify's branch/worktree finishing step.

🎯 Scope

  • CLI commands (init, status, doctor, update)
  • Core installer / platform detection
  • Comet skills (assets/skills/, assets/skills-zh/)
  • Comet shell scripts (assets/skills/comet/scripts/)
  • Tests / CI
  • Documentation / changelog
  • Other: Classic workflow state machine (domains/comet-classic/)

🧪 Testing

  • pnpm build
  • pnpm lint
  • pnpm run lint:architecture
  • pnpm format:check
  • pnpm test
  • pnpm test -- test/domains/comet-classic/comet-scripts.test.ts
  • Not run:

✅ Checklist

  • PR title follows Conventional Commits, for example fix: handle project-scope init
  • User-facing behavior is documented in README.md, README-zh.md, or CONTRIBUTING.md
  • CHANGELOG.md is updated when behavior changes
  • Skill changes were made in Chinese first when applicable, then synced to English
  • New scripts are included in assets/manifest.json and relevant tests
  • Shell scripts remain portable across macOS, Linux, and Windows Git Bash
  • No unrelated generated files or local artifacts are included

👀 Notes for Reviewers

assets/skills/comet/scripts/comet-runtime.mjs is a generated bundle (via pnpm build from domains/comet-classic/*.ts); please review the TypeScript sources under domains/comet-classic/ as the source of truth and treat the bundle diff as generated output.

One pre-existing, unrelated test failure may show up locally in test/domains/bundle/bundle-command.test.ts — it is caused by a local global skill file on the reviewer's machine, not by this change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added isolation: current option so workflows can run against the current branch.
    • Introduced comet state rebind to confirm/rebind current-branch isolation with audit logging and safety checks.
    • Enhanced current-change status to surface isolation and branch details.
  • Bug Fixes
    • Hotfix/tweak (and others) now require an explicit isolation choice; workflows pause at decision points when bindings drift or HEAD is detached (no automatic switching/rebinding).
    • Archive flow now supports current-branch handling and records the chosen branch_action.
  • Documentation
    • Updated English/Chinese docs and skill guides for current isolation, drift handling, archive decisions, and branch_action.
  • Chores
    • Bumped release version to 0.4.0-beta.6.

1919chichi and others added 5 commits July 13, 2026 20:44
- resolveBuild() no longer throws when a preset (hotfix/tweak) enters
  build phase with isolation unset; it now routes to `.build.execute`
  the same way `full` routes to `full.build.configure`. Previously any
  hotfix/tweak guard call crashed the CLI right after init, since
  presets stopped defaulting isolation to 'branch'.
- Enforce PRESET_ALLOWED_ISOLATIONS (branch/current only) at the guard
  build check and at `state transition build-complete`, so hotfix/tweak
  can no longer silently pick up isolation: worktree.
- resolveCurrentChange()/currentChange() reuse the already-resolved
  Classic state and branch instead of re-reading the same .comet.yaml
  and re-spawning git; also fixes a detached-HEAD display bug where the
  fallback branch lookup printed the literal 'HEAD'.
- Restore the branch/worktree decision-point trigger sentence in
  comet-verify SKILL.md (en/zh) that was dropped when the current-mode
  variant was added.
- Update stale context-recovery text and test fixtures/assertions
  (classic-contract, classic-guard, skills.test.ts) to match the
  intentional removal of the preset isolation default.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @1919chichi, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Classic workflows add isolation: current, persist and enforce branch bindings, require explicit preset isolation choices, update verification and archive handling, synchronize English and Chinese guidance, and extend runtime, tests, and beta release metadata.

Changes

Classic current-branch isolation

Layer / File(s) Summary
Isolation state and enforcement
domains/comet-classic/*
Adds derived isolation modes, persisted bound_branch and branch_action fields, current-branch binding evaluation, drift checks, rebind handling, and guard enforcement across build, verify, and archive.
Current-change binding and resolution
domains/comet-classic/classic-current-change.ts, assets/skills/comet/reference/scripts.md, assets/skills-zh/comet/reference/scripts.md
Active-change resolution now carries Classic state and branch context, detects current-isolation drift, and requires re-selection after branch changes.
Preset build and isolation workflow contracts
domains/comet-classic/classic-resolver.ts, assets/skills*/comet-build/*, assets/skills*/comet-hotfix/*, assets/skills*/comet-tweak/*, assets/skills*/comet/reference/*
Preset workflows require explicit isolation selection, support current, and document binding, stale-selection, and decision-point behavior.
Verification and archive decisions
assets/skills*/comet-verify/*, assets/skills*/comet-archive/*, assets/skills/comet/rules/*
Verification pauses on binding drift; archive handling branches by isolation mode and records the completed branch_action.
Regression coverage and release updates
test/domains/comet-classic/*, test/domains/skill/*, README*, CHANGELOG.md, package.json, assets/manifest.json, .gitignore
Tests cover current isolation, drift, rebind, guards, state serialization, and workflow text; documentation and package metadata identify beta.6 and the new option.
Bundled runtime synchronization
assets/skills/comet/scripts/comet-runtime.mjs
The shipped bundle incorporates updated state persistence, YAML code, current-isolation logic, guards, commands, selection, and resume handling.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CometBuild
  participant ClassicState
  participant ClassicGuard
  participant CometVerify
  participant CometArchive
  User->>CometBuild: choose branch, worktree, or current
  CometBuild->>ClassicState: set isolation and bind current branch
  ClassicGuard->>ClassicState: check isolation and branch binding
  ClassicState-->>CometVerify: return valid or blocked state
  CometVerify->>User: request branch switch or confirmed rebind
  CometArchive->>ClassicState: record branch action after archive handling
Loading

Possibly related issues

Possibly related PRs

  • rpamis/comet#195 — Both changes modify Classic current-change selection and resolution behavior.

Suggested reviewers: benym

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and clearly matches the main change: adding a new current-branch isolation mode.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

README.md/README-zh.md's isolation field docs and hard-constraint bullet
still only listed branch/worktree after the current-branch isolation
feature was added.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@1919chichi 1919chichi changed the title feat(comet-classic): add current-branch isolation mode feat: add current-branch isolation mode Jul 13, 2026
@1919chichi

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@1919chichi

Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

@1919chichi

Copy link
Copy Markdown
Contributor Author

@greptileai review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@sourcery-ai

sourcery-ai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Sorry @1919chichi, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 11-13: Update the changelog entry for “Preset isolation decisions”
to remove the Chinese/English Skills synchronization wording and retain only the
user-visible behavior: the consistent current-branch handoff and archive commit
path for /comet-verify and /comet-archive.

In `@README-zh.md`:
- Line 539: Clarify the isolation documentation for preset workflows: in
README-zh.md at lines 539-539, add that hotfix / tweak only allow branch or
current; make the equivalent clarification in README.md at lines 579-579, while
retaining the global support statement for branch, worktree, and current.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 56a15a29-0614-401f-9e7c-00db3d779cb9

📥 Commits

Reviewing files that changed from the base of the PR and between 4dcf177 and 83c651d.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (37)
  • CHANGELOG.md
  • README-zh.md
  • README.md
  • assets/manifest.json
  • assets/skills-zh/comet-archive/SKILL.md
  • assets/skills-zh/comet-build/SKILL.md
  • assets/skills-zh/comet-hotfix/SKILL.md
  • assets/skills-zh/comet-tweak/SKILL.md
  • assets/skills-zh/comet-verify/SKILL.md
  • assets/skills-zh/comet/SKILL.md
  • assets/skills-zh/comet/reference/comet-yaml-fields.md
  • assets/skills-zh/comet/reference/scripts.md
  • assets/skills/comet-archive/SKILL.md
  • assets/skills/comet-build/SKILL.md
  • assets/skills/comet-hotfix/SKILL.md
  • assets/skills/comet-tweak/SKILL.md
  • assets/skills/comet-verify/SKILL.md
  • assets/skills/comet/SKILL.md
  • assets/skills/comet/reference/comet-yaml-fields.md
  • assets/skills/comet/reference/scripts.md
  • assets/skills/comet/rules/comet-phase-guard.md
  • assets/skills/comet/scripts/comet-runtime.mjs
  • domains/comet-classic/classic-current-change.ts
  • domains/comet-classic/classic-guard.ts
  • domains/comet-classic/classic-resolver.ts
  • domains/comet-classic/classic-state-command.ts
  • domains/comet-classic/classic-state.ts
  • domains/comet-classic/classic-validate-command.ts
  • package.json
  • test/app/cli-help.test.ts
  • test/domains/comet-classic/classic-contract.test.ts
  • test/domains/comet-classic/classic-guard.test.ts
  • test/domains/comet-classic/classic-hook-guard.test.ts
  • test/domains/comet-classic/classic-resolver.test.ts
  • test/domains/comet-classic/classic-state.test.ts
  • test/domains/comet-classic/comet-scripts.test.ts
  • test/domains/skill/skills.test.ts

Comment thread CHANGELOG.md Outdated
Comment thread README-zh.md

benym commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

请先解决Review中的问题。

目前仍有影响现有工作流语义的问题,建议修复后再合并。

1. current 的分支绑定会被标准入口流程静默覆盖

相关位置:

  • domains/comet-classic/classic-current-change.ts:107-116
  • assets/skills-zh/comet/SKILL.md:43-47
  • assets/skills-zh/comet-build/SKILL.md:15-22
  • assets/skills-zh/comet-verify/SKILL.md:19-26
  • assets/skills-zh/comet-archive/SKILL.md:20-27
  • 对应英文 Skill

selectCurrentChange() 每次都会把执行命令时的当前分支重新写入 .comet/current-change.json。与此同时,Comet 主入口以及 build、verify、archive 的恢复入口都会先无条件执行:

comet state select <change-name>

这会导致 current 模式的分支漂移检测被标准流程绕过。

例如:

  1. 在分支 A 选择 change,并设置 isolation: current
  2. 完成 build 后意外切换到分支 B
  3. 进入 /comet-verify
  4. verify 入口先执行 comet state select
  5. sidecar 中的绑定分支被直接覆盖为 B
  6. 后续 state check 和 guard 不再报告 stale selection

结果是工作流可以在没有用户确认的情况下继续运行在错误分支上。这与 issue #190 中“当前分支意外变化时必须警告并要求确认”的要求不一致。

另外,绑定分支目前只存在于仓库本地的 .comet/current-change.json,没有持久化到 change 状态。sidecar 被清理、丢失,或者在另一个 checkout 中恢复时,isolation: current 本身无法说明应该固定在哪个分支。

建议:

  • 将 current 模式的绑定分支持久化到 .comet.yaml,例如增加独立的 isolation_ref / bound_branch 字段;或者
  • comet state select 在 change 已经处于 isolation: current 且绑定分支与当前分支不一致时拒绝覆盖;
  • 如确实需要切换绑定,应提供明确的 rebind/confirm 操作,而不是让普通 select 隐式完成;
  • state current、hook guard 和 phase guard 应同时比较绑定分支与实际分支;
  • detached HEAD 应明确拒绝 current 绑定,或者记录 commit/ref 类型,避免 detached HEAD → 普通分支时绕过 stale 检测。

建议补充完整生命周期测试:

  • current 模式下 build → 切换分支 → verify 入口必须失败;
  • current 模式下 verify → 切换分支 → archive 入口必须失败;
  • sidecar 缺失后不能在其他分支自动恢复;
  • detached HEAD 绑定和后续分支切换;
  • 显式确认 rebind 后才允许继续。

2. hotfix/tweak 禁止 worktree 是不必要的兼容性回归

相关位置:

  • domains/comet-classic/classic-state.ts:15-24
  • domains/comet-classic/classic-state-command.ts:534-540
  • domains/comet-classic/classic-guard.ts:501-511
  • domains/comet-classic/classic-resolver.ts:31-37

PR 新增了 PRESET_ALLOWED_ISOLATIONS,并将 hotfix/tweak 限制为:

branch | current

这不是 issue #190 的要求。issue 的目标是让 current 与现有 branch/worktree 模式共存,而不是移除 preset workflow 原有的 worktree 能力。

此前运行时允许 hotfix/tweak 使用 isolation: worktree,复杂 preset 任务转入 /comet-build 后也可以选择 worktree。升级后,已有记录了 isolation: worktree 的 hotfix/tweak change 会被 guard 和 transition 拒绝,无法正常完成。

建议保留显式选择,但让 hotfix/tweak 接受:

branch | worktree | current

如果产品上确实希望移除 preset workflow 的 worktree 支持,这应该作为单独的破坏性行为变更讨论,并提供迁移策略,不建议捆绑在 current 模式 PR 中。

当前实现还有一处相关矛盾:

  • classic-state-command.ts:1020 的恢复提示仍让用户选择 branch/worktree/current
  • 但 hotfix/tweak 随后会拒绝其中的 worktree

如果保留 preset 限制,需要让恢复提示根据 workflow 输出允许的选项;如果恢复 worktree 支持,这个问题会自然消失。

3. current 模式的最终 push 状态需要更明确

/comet-verify 当前让用户选择:

  1. 立即 push 当前分支
  2. 暂不 push

然后将 branch_status 写为 handled。但是 archive 阶段随后还会产生新的 archive commit。

这意味着即使用户在 verify 阶段选择了立即 push,最终 archive commit 仍可能只保留在本地,而状态中也无法区分用户此前选择的是 push 还是 keep-local。

建议:

  • 将 current 模式的最终 push/no-push 决策移动到 archive commit 之后;或者
  • 持久化具体动作,例如 branch_action: push | keep-local
  • archive 产生新 commit 后,根据该动作再次完成或确认最终 push。

4. 测试文件存在 ESLint 错误

test/domains/comet-classic/comet-scripts.test.ts:2075

JSON.stringify({ scripts: { build: 'node -e \"process.exit(0)\"' } }),

单引号字符串中的双引号不需要转义,额外对 PR 改动测试文件运行 ESLint 时会产生两个 no-useless-escape 错误。

建议改为:

JSON.stringify({ scripts: { build: 'node -e "process.exit(0)"' } }),

当前仓库的正式 pnpm lint 没有检查 test/,因此 CI 没有发现这个问题。

结论

Request changes。

建议至少先完成以下两项再合并:

  1. current 分支绑定不能被普通 state select 静默覆盖,并增加跨阶段切换分支、sidecar 丢失和 detached HEAD 的回归测试;
  2. 恢复 hotfix/tweak 的 worktree 兼容性,或者单独确认并设计这项破坏性变更及迁移方案。

其余 push 状态和测试文件 ESLint 问题也建议在本 PR 中一并处理。

1919chichi and others added 5 commits July 17, 2026 09:33
Reconciles the current-isolation / bound_branch drift-detection feature
branch with master's beta.5 release (Codex hook migration, standard
Superpowers artifact routing, verification repair loops, dashboard
Markdown preview, OpenSpec 1.5 compatibility). Branch handling after
verify follows master's design (finishing-a-development-branch runs in
comet-archive after the archive commit, per the
workflow-optimization-contract test suite); the isolation-choice and
bound_branch drift-detection additions from this branch are layered on
top, with branch_action now also recorded during archive's branch
handling step.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@1919chichi

Copy link
Copy Markdown
Contributor Author

感谢详细评审。分支已通过 f5a0f2b 追平 master,最新提交(已 push)针对四个问题分别做了设计上的调整,逐条说明如下。

1. current 绑定被 state select 静默覆盖

你发现的 bug 的根因:绑定分支之前只存在于 .comet/current-change.json 这个 sidecar 里,而它是由 comet state select 写入的。问题在于,所有标准入口(主入口、build/verify/archive 的 resume 流程)在做任何漂移检查之前,都会无条件先跑一遍 state select——这就导致"预期分支"这个比较基准,在比较发生前的一瞬间被重写成了"当前实际所在的分支"。检查因此永远不可能失败。

具体场景走一遍旧逻辑下的 bug:

  1. 在分支 A 上,change X 被设为 isolation: current。此时 X 的 sidecar 是 {branch: A}
  2. build 在 A 上完成。用户误切换到了分支 B
  3. /comet-verify 的 resume 入口先执行 comet state select X → sidecar 被重写为 {branch: B}
  4. 之后的 stale 检查拿 sidecar.branch(刚被写成的 B)去和当前分支(B)比较 → 相等 → 通过。verify 就这样在错误的分支上继续跑下去,没有任何警告。

新设计:把"真相来源"从 sidecar 挪到 change 自己的持久化状态里。在 .comet.yaml 里新增 bound_branch 字段,在用户选择 isolation: current 那一刻写入一次(此时如果 HEAD 是 detached 状态则直接拒绝),此后 state select 完全不会再碰它——select 只写 sidecar,代码里没有任何路径会去写 .comet.yaml

用同样的场景走一遍新逻辑:

  1. 在分支 A 上选择 isolation: currentX.comet.yaml 里写入 bound_branch: A
  2. 用户切到 B
  3. /comet-verify 入口依然会跑 state select(sidecar 照样被重写成 B——但现在这已经无关紧要,因为它不再是判断依据了),随后 resolveCurrentChange() 读取 .comet.yaml 里的 bound_branch: A,通过共享的 evaluateBranchBinding() 判定函数和当前实际分支 B 比较 → 判定为 drift → 入口返回 stale 并阻断。
  4. classic-guard.ts 里的 boundBranchMatches() 在 build-complete/verify/archive 三个关卡各自独立做了同样的比较,所以就算某条代码路径绕过了 resume 入口,guard 这一层依然会拦下来,不是单点检查。
  5. 如果用户确实想把这个 change 转移到 B 上继续做,必须显式执行 comet state rebind X——这个命令在 detached HEAD 下会被拒绝,成功执行后会把 bound_branch 更新为 B,并追加一条审计记录(event: 'rebind', from: A, to: B)——变成一次显式、有记录的操作,而不是 select 的隐式副作用。

补充的回归测试覆盖:build → 切分支 → verify 失败、verify → 切分支 → archive 失败、sidecar 被删除后状态仍可从 .comet.yaml 恢复、detached HEAD 下绑定/rebind 均被拒绝。

2. hotfix/tweak 失去 worktree 支持

这是一处非预期的回归,不是范围决策。ISOLATION_MODES 是一张 { value, allowedInPreset } 的表;本分支早期某个提交在加入 current 的同时,顺手把 worktreeallowedInPreset 也改成了 false,错误地把 preset workflow 收窄成了 branch | current。现已回滚,preset 恢复为 branch | worktree | current,符合 #190 实际的范围(在已有模式基础上新增 current,而不是移除某个已有模式)。

3. current 模式最终的 push 状态不明确

问题本质:/comet-verify 里的 push/不 push 选择,和 archive 产生的提交,是两个独立事件,但旧状态只记了一个 branch_status: handled——这个近似布尔值的字段没法区分"verify 时已经 push 过了,archive 又在上面加了一个只存在本地的提交"和"用户当时选择了不 push"这两种情况。新增了独立的 branch_action 字段(push | keep-local | merged-locally | pushed-pr),在 archive 阶段记录分支处理实际采取的动作,和笼统的 branch_status 解耦开。

4. 测试文件里的 ESLint no-useless-escape

test/domains/comet-classic/comet-scripts.test.ts:单引号字符串里的 'node -e \"process.exit(0)\"' 有多余转义,改成了 'node -e "process.exit(0)"'


test/domains/comet-classic/comet-scripts.test.ts 测试通过(196 通过 / 1 跳过——跳过的是一个已有的、Windows 平台专属的大小写不敏感路径测试,和这次改动无关,在 macOS 上不会被执行)。分支已经 push,麻烦帮忙 review。

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
assets/skills/comet-tweak/SKILL.md (1)

73-75: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore worktree isolation option.

The PR objective states that restricting tweak/hotfix workflows removed worktree support and this regression was reverted to restore all three isolation modes. However, the worktree option is still missing from the isolation decision points in this document.

  • assets/skills/comet-tweak/SKILL.md#L73-L75: add - C. use a worktree to the explicit choice list.
  • assets/skills/comet-tweak/SKILL.md#L155-L155: update the initial isolation choice to (`branch` / `worktree` / `current`).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@assets/skills/comet-tweak/SKILL.md` around lines 73 - 75, Restore worktree as
an isolation option in assets/skills/comet-tweak/SKILL.md: at lines 73-75 add
“C. use a worktree” to the explicit choice list, and at line 155 update the
initial isolation choice to include branch, worktree, and current.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.gitignore:
- Line 92: Remove the docs/superpowers/ entry from .gitignore so persisted
workflow artifacts are tracked and included in normal and archive commits.

In `@assets/skills-zh/comet/reference/comet-yaml-fields.md`:
- Line 49: Restore the complete isolation contract across all three documented
sites: in assets/skills-zh/comet/reference/comet-yaml-fields.md:49, document
worktree alongside branch and current for hotfix/tweak and require explicit
selection; in assets/skills-zh/comet-hotfix/SKILL.md:47-81 and
assets/skills/comet-hotfix/SKILL.md:47-81, remove the implicit current default
and present branch, worktree, and current as supported explicit isolation modes,
keeping the Chinese and English contracts aligned.

In `@assets/skills/comet-archive/SKILL.md`:
- Around line 122-129: Reverse the state-setting order in the archive operation
instructions so branch_action is persisted before branch_status, preventing an
interrupted sequence from leaving handled without an action. Apply this ordering
consistently at assets/skills/comet-archive/SKILL.md:122-129 and
assets/skills-zh/comet-archive/SKILL.md:122-129; no other workflow changes are
needed.

In `@domains/comet-classic/classic-state-command.ts`:
- Around line 1304-1315: Make the rebind flow in healBoundBranch’s surrounding
command failure-atomic by using the existing recoverable transaction/WAL
protocol to persist the state change and audit event as one operation,
preventing an unaudited binding if event appending fails or the process exits.
Implement the fix only in domains/comet-classic/classic-state-command.ts, then
regenerate assets/skills/comet/scripts/comet-runtime.mjs with pnpm
build:classic-runtime; do not hand-edit the generated bundle.

---

Outside diff comments:
In `@assets/skills/comet-tweak/SKILL.md`:
- Around line 73-75: Restore worktree as an isolation option in
assets/skills/comet-tweak/SKILL.md: at lines 73-75 add “C. use a worktree” to
the explicit choice list, and at line 155 update the initial isolation choice to
include branch, worktree, and current.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f651a193-65e3-4ae6-aaeb-dae0dc6cf1c7

📥 Commits

Reviewing files that changed from the base of the PR and between 83c651d and 69001fb.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (38)
  • .gitignore
  • CHANGELOG.md
  • README-zh.md
  • README.md
  • assets/manifest.json
  • assets/skills-zh/comet-archive/SKILL.md
  • assets/skills-zh/comet-build/SKILL.md
  • assets/skills-zh/comet-hotfix/SKILL.md
  • assets/skills-zh/comet-tweak/SKILL.md
  • assets/skills-zh/comet-verify/SKILL.md
  • assets/skills-zh/comet/SKILL.md
  • assets/skills-zh/comet/reference/comet-yaml-fields.md
  • assets/skills/comet-archive/SKILL.md
  • assets/skills/comet-build/SKILL.md
  • assets/skills/comet-hotfix/SKILL.md
  • assets/skills/comet-tweak/SKILL.md
  • assets/skills/comet-verify/SKILL.md
  • assets/skills/comet/SKILL.md
  • assets/skills/comet/reference/comet-yaml-fields.md
  • assets/skills/comet/rules/comet-phase-guard.md
  • assets/skills/comet/scripts/comet-runtime.mjs
  • domains/comet-classic/classic-branch-binding.ts
  • domains/comet-classic/classic-current-change.ts
  • domains/comet-classic/classic-guard.ts
  • domains/comet-classic/classic-state-command.ts
  • domains/comet-classic/classic-state-events.ts
  • domains/comet-classic/classic-state.ts
  • domains/comet-classic/classic-validate-command.ts
  • package.json
  • test/app/cli-help.test.ts
  • test/domains/comet-classic/classic-branch-binding.test.ts
  • test/domains/comet-classic/classic-contract.test.ts
  • test/domains/comet-classic/classic-current-change.test.ts
  • test/domains/comet-classic/classic-hook-guard.test.ts
  • test/domains/comet-classic/classic-resolver.test.ts
  • test/domains/comet-classic/classic-state.test.ts
  • test/domains/comet-classic/comet-scripts.test.ts
  • test/domains/skill/skills.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
  • test/domains/comet-classic/classic-state.test.ts
  • domains/comet-classic/classic-validate-command.ts
  • assets/manifest.json
  • test/domains/comet-classic/classic-resolver.test.ts
  • assets/skills-zh/comet-tweak/SKILL.md
  • assets/skills/comet/reference/comet-yaml-fields.md
  • assets/skills/comet-build/SKILL.md
  • domains/comet-classic/classic-current-change.ts

Comment thread .gitignore

# Superpowers docs (local only)
#docs/superpowers/
docs/superpowers/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not ignore persisted workflow artifacts.

Classic stores design documents and plans under docs/superpowers/. Ignoring the directory prevents newly generated artifacts from entering normal and archive commits, leaving committed state that references missing files.

Proposed fix
-docs/superpowers/
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
docs/superpowers/
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.gitignore at line 92, Remove the docs/superpowers/ entry from .gitignore so
persisted workflow artifacts are tracked and included in normal and archive
commits.

Comment thread assets/skills-zh/comet/reference/comet-yaml-fields.md Outdated
Comment on lines +122 to 129
Only after the selected operation succeeds (or the user explicitly keeps the branch), run:

```bash
comet state set <change-name> branch_status handled
comet state set <change-name> branch_action <value>
comet guard <change-name> archive
comet state clear-selection
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== assets/skills/comet-archive/SKILL.md ==\n'
nl -ba assets/skills/comet-archive/SKILL.md | sed -n '100,150p'

printf '\n== assets/skills-zh/comet-archive/SKILL.md ==\n'
nl -ba assets/skills-zh/comet-archive/SKILL.md | sed -n '100,150p'

printf '\n== search related state commands ==\n'
rg -n "branch_status handled|branch_action|clear-selection|guard <change-name> archive" assets/skills assets/skills-zh

Repository: rpamis/comet

Length of output: 234


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== assets/skills/comet-archive/SKILL.md relevant section ==\n'
nl -ba assets/skills/comet-archive/SKILL.md | sed -n '112,136p'

printf '\n== assets/skills-zh/comet-archive/SKILL.md relevant section ==\n'
nl -ba assets/skills-zh/comet-archive/SKILL.md | sed -n '112,136p'

printf '\n== repository-wide references ==\n'
rg -n "branch_status|branch_action|clear-selection|comet state set" .

Repository: rpamis/comet

Length of output: 251


🏁 Script executed:

#!/bin/bash
set -euo pipefail
nl -ba assets/skills/comet-archive/SKILL.md | sed -n '1,180p'

Repository: rpamis/comet

Length of output: 190


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== assets/skills/comet-archive/SKILL.md ==\n'
sed -n '112,136p' assets/skills/comet-archive/SKILL.md | cat -n

printf '\n== assets/skills-zh/comet-archive/SKILL.md ==\n'
sed -n '112,136p' assets/skills-zh/comet-archive/SKILL.md | cat -n

printf '\n== related references ==\n'
rg -n "branch_status handled|branch_action|clear-selection|comet state set" assets/skills assets/skills-zh

Repository: rpamis/comet

Length of output: 16595


Set branch_action before branch_status. The current order can leave branch_status: handled persisted without branch_action if the sequence is interrupted. Apply the same ordering in both assets/skills/comet-archive/SKILL.md and assets/skills-zh/comet-archive/SKILL.md, or make the transition atomic.

📍 Affects 2 files
  • assets/skills/comet-archive/SKILL.md#L122-L129 (this comment)
  • assets/skills-zh/comet-archive/SKILL.md#L122-L129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@assets/skills/comet-archive/SKILL.md` around lines 122 - 129, Reverse the
state-setting order in the archive operation instructions so branch_action is
persisted before branch_status, preventing an interrupted sequence from leaving
handled without an action. Apply this ordering consistently at
assets/skills/comet-archive/SKILL.md:122-129 and
assets/skills-zh/comet-archive/SKILL.md:122-129; no other workflow changes are
needed.

Comment on lines +1304 to +1315
const before = await readClassicState(directory);
if (!before.classic) fail('ERROR: Classic state projection is missing');
await healBoundBranch(directory, branch);
const after: ClassicState = { ...before.classic, boundBranch: branch };
await appendClassicStateEvent(directory, {
change: name,
event: 'rebind',
source: 'comet-state',
from: before.classic,
to: after,
effects: [{ field: 'boundBranch', from: boundBranch, to: branch }],
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make rebind state and audit persistence failure-atomic.

The binding is persisted before its audit event, so an append failure or process exit can leave an unaudited rebind.

  • domains/comet-classic/classic-state-command.ts#L1304-L1315: use a recoverable transaction/WAL protocol for the state and event-log writes.
  • assets/skills/comet/scripts/comet-runtime.mjs#L14208-L14219: regenerate the bundle after fixing the TypeScript source; do not hand-edit it.

As per coding guidelines, Classic logic belongs in domains/comet-classic/*.ts, followed by pnpm build:classic-runtime.

🧰 Tools
🪛 ast-grep (0.44.1)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

📍 Affects 2 files
  • domains/comet-classic/classic-state-command.ts#L1304-L1315 (this comment)
  • assets/skills/comet/scripts/comet-runtime.mjs#L14208-L14219
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@domains/comet-classic/classic-state-command.ts` around lines 1304 - 1315,
Make the rebind flow in healBoundBranch’s surrounding command failure-atomic by
using the existing recoverable transaction/WAL protocol to persist the state
change and audit event as one operation, preventing an unaudited binding if
event appending fails or the process exits. Implement the fix only in
domains/comet-classic/classic-state-command.ts, then regenerate
assets/skills/comet/scripts/comet-runtime.mjs with pnpm build:classic-runtime;
do not hand-edit the generated bundle.

Source: Coding guidelines

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@assets/skills/comet-tweak/SKILL.md`:
- Around line 71-84: Update the earlier isolation default in the tweak workflow
to use isolation: null, or explicitly block execution until the user selects
branch, worktree, or current. Ensure the instruction no longer defaults to
isolation: current, while preserving the mandatory decision point and subsequent
comet state set flow.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dec73d89-0a54-4954-ae17-e973896e4c88

📥 Commits

Reviewing files that changed from the base of the PR and between 69001fb and a13aa76.

📒 Files selected for processing (9)
  • assets/skills-zh/comet-archive/SKILL.md
  • assets/skills-zh/comet-hotfix/SKILL.md
  • assets/skills-zh/comet-tweak/SKILL.md
  • assets/skills-zh/comet/reference/comet-yaml-fields.md
  • assets/skills/comet-archive/SKILL.md
  • assets/skills/comet-hotfix/SKILL.md
  • assets/skills/comet-tweak/SKILL.md
  • assets/skills/comet/reference/comet-yaml-fields.md
  • test/domains/skill/skills.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • assets/skills-zh/comet/reference/comet-yaml-fields.md
  • assets/skills-zh/comet-tweak/SKILL.md
  • test/domains/skill/skills.test.ts
  • assets/skills/comet-archive/SKILL.md
  • assets/skills-zh/comet-archive/SKILL.md

Comment on lines +71 to +84
Before execution starts, the user must explicitly choose isolation. This is a user decision point and must pause per `comet/reference/decision-point.md` until the user explicitly chooses:

- A. create a branch (recommended default)
- B. use a worktree
- C. use the current branch

After the choice, immediately write:

```bash
comet state set <name> isolation <branch|worktree|current>
```

If `branch` is chosen, follow `/comet-build`'s branch rules to confirm the branch name and create it, then re-run `comet state select <change-name>` to bind the current change. If `worktree` is chosen, follow `/comet-build`'s worktree rules and re-run `comet state select <change-name>` inside the worktree. If `current` is chosen, create no new branch or worktree and do not force an extra re-selection when no workspace switch happened.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the contradictory isolation: current default.

This section correctly requires an explicit user choice, but the earlier instruction at Line 55 still says tweak defaults to isolation: current. An agent may follow that default and skip this mandatory decision, violating the new workflow contract. Rewrite the earlier default as isolation: null or otherwise state that execution cannot proceed until the user selects branch, worktree, or current.

🧰 Tools
🪛 LanguageTool

[style] ~83-~83: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ect inside the worktree. Ifcurrent` is chosen, create no new bran...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

🪛 SkillSpector (2.3.11)

[warning] 136: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.

Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.

(Excessive Agency (EA2))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@assets/skills/comet-tweak/SKILL.md` around lines 71 - 84, Update the earlier
isolation default in the tweak workflow to use isolation: null, or explicitly
block execution until the user selects branch, worktree, or current. Ensure the
instruction no longer defaults to isolation: current, while preserving the
mandatory decision point and subsequent comet state set flow.

@benym

benym commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

合并的时候有些问题,wechat.png被回退到了wechat.png,website的子模块指针也被回退了

另外还有一些

  • Tweak Skill 自相矛盾:写“默认 isolation: current”,后面又要求用户显式选择 branch/worktree/current。这会让 agent 跳过新增的决策点。中英文都存在
  • 中文 phase guard 明确增加了 current-branch handling,英文规则仍只写 branch handling,存在双语行为表述不完全同步
  • Issue 要求 comet status 显示 isolation 和当前绑定分支,但 ChangeStatus 没有 branch/boundBranch 字段,文本输出时也没有isolation显示

@1919chichi

Copy link
Copy Markdown
Contributor Author

合并的时候有些问题,wechat.png被回退到了wechat.png,website的子模块指针也被回退了

另外还有一些

  • Tweak Skill 自相矛盾:写“默认 isolation: current”,后面又要求用户显式选择 branch/worktree/current。这会让 agent 跳过新增的决策点。中英文都存在
  • 中文 phase guard 明确增加了 current-branch handling,英文规则仍只写 branch handling,存在双语行为表述不完全同步
  • Issue 要求 comet status 显示 isolation 和当前绑定分支,但 ChangeStatus 没有 branch/boundBranch 字段,文本输出时也没有isolation显示

那我先把这个pr关了吧, 我用主分支最新的代码重新设计
代码和skill中的细节问题我会根据近几次review的反馈的结果再好好打磨下

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: support current-branch mode with no new branch or worktree

2 participants