feat: add current-branch isolation mode#210
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XQUYcRLiDWw6MRDAj5WQSc
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XQUYcRLiDWw6MRDAj5WQSc
Add bound_branch to legacyProjection's runKeys allowlist in the 0.3.9 differential contract test, alongside the existing isolation entry — both are active-runtime-only projection fields not part of the frozen 0.3.9 bash contract, so the differential comparison should exclude them the same way.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @1919chichi, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThe release adds persisted Git branch binding for ChangesCurrent-branch isolation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1919chichi
marked this pull request as ready for review
July 18, 2026 15:31
1919chichi
marked this pull request as draft
July 18, 2026 15:31
Centralize binding resolution in resolveBranchBinding (typed reads, lazy git spawns, optional heal) and fix the drift-protection gaps found in review: - guard --apply re-reads the run context before the transition write so it no longer clobbers a bound_branch healed during the same run - the PreToolUse hook enforces the binding for a sole active change even when no current-change selection file exists - selections record the branch they were made on again, so pre-binding (isolation null) changes flag branch switches as stale like master did - resolving the current change is read-only: the hook never heals (and can no longer throw on a failed heal write); heal happens on select/check/guard, which announce the lazily bound branch - select refuses a drifted change instead of writing a selection that the next command immediately reports as stale, and no longer prints an empty "(branch: )" suffix for unbound changes - switching isolation between workspace modes re-points the binding to the current branch; repeating the same mode keeps the sticky binding Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document that the binding records the branch of the directory the command runs in (run set/check/guard inside the worktree for worktree mode), that switching workspace modes re-binds while repeating the same mode keeps the binding, and that select refuses on drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merge the within-branch compatibility fix into the Changed entries, drop the sidecar implementation detail, and describe the final drift semantics (single-change write guard coverage, select refusal, mode switch rebinding) from the user's perspective. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1919chichi
marked this pull request as ready for review
July 19, 2026 02:07
There was a problem hiding this comment.
Sorry @1919chichi, your pull request is larger than the review limit of 150000 diff characters
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
domains/comet-classic/classic-current-change.ts (1)
147-150: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle potential file read errors in
resolveBranchBinding.Both of these read-only paths invoke
resolveBranchBindingwithout catching potential file system or parsing errors. If.comet.yamlis deleted or corrupted immediately after the initial validation (e.g., a TOCTOU race), the unhandled exception will crash the PreToolUse hook rather than cleanly blocking the operation using the established error-swallowing pattern.
domains/comet-classic/classic-current-change.ts#L147-L150: Wrap this call in atry/catchblock and return{ status: 'stale', reason: ... }on error.domains/comet-classic/classic-hook-guard.ts#L324-L327: Wrap this call in atry/catchblock and return{ blockedResult: blockedStaleSelection(relativePath, ...) }on error.🤖 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-current-change.ts` around lines 147 - 150, Wrap the resolveBranchBinding call in domains/comet-classic/classic-current-change.ts:147-150 with try/catch and return { status: 'stale', reason: ... } on errors; likewise wrap the call in domains/comet-classic/classic-hook-guard.ts:324-327 and return { blockedResult: blockedStaleSelection(relativePath, ...) }. Preserve the existing error-swallowing behavior and use each function’s established stale-selection messaging.
🤖 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.
Nitpick comments:
In `@domains/comet-classic/classic-current-change.ts`:
- Around line 147-150: Wrap the resolveBranchBinding call in
domains/comet-classic/classic-current-change.ts:147-150 with try/catch and
return { status: 'stale', reason: ... } on errors; likewise wrap the call in
domains/comet-classic/classic-hook-guard.ts:324-327 and return { blockedResult:
blockedStaleSelection(relativePath, ...) }. Preserve the existing
error-swallowing behavior and use each function’s established stale-selection
messaging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71b86af6-ad06-4297-acae-a8034d673527
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (44)
CHANGELOG.mdREADME-zh.mdREADME.mdapp/commands/status.tsassets/manifest.jsonassets/skills-zh/comet-archive/SKILL.mdassets/skills-zh/comet-build/SKILL.mdassets/skills-zh/comet-hotfix/SKILL.mdassets/skills-zh/comet-tweak/SKILL.mdassets/skills-zh/comet-verify/SKILL.mdassets/skills-zh/comet/SKILL.mdassets/skills-zh/comet/reference/comet-yaml-fields.mdassets/skills/comet-archive/SKILL.mdassets/skills/comet-build/SKILL.mdassets/skills/comet-hotfix/SKILL.mdassets/skills/comet-tweak/SKILL.mdassets/skills/comet-verify/SKILL.mdassets/skills/comet/SKILL.mdassets/skills/comet/reference/comet-yaml-fields.mdassets/skills/comet/scripts/comet-runtime.mjsdocs/superpowers/plans/2026-07-18-current-branch-isolation.mddocs/superpowers/specs/2026-07-18-current-branch-isolation-design.mddomains/comet-classic/classic-branch-binding.tsdomains/comet-classic/classic-current-change.tsdomains/comet-classic/classic-guard.tsdomains/comet-classic/classic-hook-guard.tsdomains/comet-classic/classic-state-command.tsdomains/comet-classic/classic-state-events.tsdomains/comet-classic/classic-state.tsdomains/comet-classic/classic-transitions.tsdomains/skill/find.tspackage.jsontest/app/cli-help.test.tstest/app/status.test.tstest/domains/comet-classic/classic-branch-binding.test.tstest/domains/comet-classic/classic-contract.test.tstest/domains/comet-classic/classic-current-change.test.tstest/domains/comet-classic/classic-guard.test.tstest/domains/comet-classic/classic-hook-guard.test.tstest/domains/comet-classic/classic-state.test.tstest/domains/comet-classic/comet-scripts-guard.test.tstest/domains/comet-classic/comet-scripts.test.tstest/domains/skill/skills.test.tstest/domains/skill/workflow-optimization-contract.test.ts
Contributor
|
classic-validate-command.ts中没有校验bound_branch只能是string或者null，虽然git分支命名上我们并不限制用户的名字，但是yaml校验通常数据类型都是string，比如叫123的分支名，会序列化为字符串 "123" 完成之后，运行 pnpm build:classic-runtime 更新生成的 comet-runtime.mjs |
- reject non-string, non-null bound_branch values in comet validate - add regression tests for numeric, array, mapping, and quoted values - rebuild comet-runtime.mjs bundle Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0143C9RxpGHLN7f5asmmhfiC
Contributor
Author
我在最新的提交中解决了这个问题,有空可以再review下 |
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #190
Summary
This PR implements first-class current-branch isolation for the Classic Comet workflow.
The design is based on #190. The issue asks for a mode where the user has already selected the correct integration branch and Comet must preserve that invariant: do not create a new branch, do not create a worktree, do not switch branches, and warn before continuing if the current branch changes unexpectedly.
Design Intent
This is not implemented as a loose
isolation: currentflag alone. The design treats current-branch isolation as a branch-bound workflow mode:Key Design Decisions
1. Persist the branch binding in
.comet.yamlBefore this change, the current-change sidecar could record the active branch and
state selectcould overwrite that local value. That made branch drift structurally hard to detect: after an accidental checkout, selecting the change again could make the wrong branch look correct.This PR moves the binding into the committed Classic state as
bound_branch:.comet/current-change.jsononly tracks which change is selected.openspec/changes/<name>/.comet.yamlstoresbound_branchas the durable branch binding.bound_branchis machine-owned and cannot be set directly withcomet state set <name> bound_branch ....bound_branchis optional for compatibility and does not bumpclassic_migration.2. Bind once, then block accidental drift
When the user chooses
isolation: current, Comet captures the live Git branch as the initialbound_branch. Re-running the normal select/check flow does not overwrite that value.If the workflow is later resumed from another branch, Comet reports the change as blocked and asks for an explicit decision: switch back to the bound branch, or intentionally rebind.
Detached HEAD is handled deliberately:
3. Add explicit audited rebinding
This PR adds
comet state rebind <change-name>for the rare case where the user intentionally wants the current-branch workflow to move to a different branch.The command:
bound_branchto the current Git branch,rebindevent in.comet/state-events.jsonl.The CLI command performs the state mutation; the workflow skills remain responsible for asking the user before invoking it.
4. Use one branch-binding verdict everywhere
The branch-binding logic is centralized in
domains/comet-classic/classic-branch-binding.tsso the same rules are used by:comet state check,That avoids each entry point inventing slightly different drift semantics.
5. Make hotfix/tweak workspace isolation explicit
The design also removes the hidden hotfix/tweak default of
isolation: current.Hotfix and tweak now start with
isolation: nulland pause for an explicit user decision among:This addresses the issue's current-branch use case without making current-branch work an invisible default. It also keeps
worktreeavailable for preset workflows.6. Split archive branch handling by isolation mode
Archive behavior now matches the selected workspace model:
branch/worktreekeep the existing feature-branch finishing flow.currentskips feature-branch merge/PR/keep choices and instead asks whether to push the current branch or keep it local.This avoids presenting branch-management choices that do not fit a workflow intentionally running on the user's existing branch.
7. Surface the selected isolation in status
comet statusnow reports the selected isolation mode and bound branch in human output, and--jsonexposes the same data. This directly covers the visibility requirement from issue #190.What Changed
bound_branchto Classic state serialization, validation, status projection, and runtime fixtures.comet state rebind <change-name>with audit-event recording.comet statustext and JSON output for isolation/bound-branch visibility.0.4.0-beta.6changelog.Compatibility
bound_branchremain readable because the field is optional.classic_migrationremains unchanged.isolation: currentchanges without a binding can be lazily bound on a real Git branch during state checks/guards.branchandworktreeworkflows keep their existing branch-management behavior, with the same bound-branch safety checks applied where appropriate.Validation
pnpm format:checkpnpm lintpnpm buildpnpm testSummary by CodeRabbit
New Features
isolation: current | branch | worktree, including persistedbound_branch.comet state rebindto explicitly refresh a binding when needed.comet statusto surface bound-branch details in both text and JSON.Bug Fixes
HEADis detached.Documentation
.comet.yamlfield references, and workflow guides to reflect the new isolation/binding behavior and prompts.