Skip to content

fix: reject --base combined with --scope working-tree - #128

Open
behruznassre wants to merge 1 commit into
sendbird:mainfrom
behruznassre:fix/base-ignores-working-tree-scope
Open

behruznassre wants to merge 1 commit into
sendbird:mainfrom
behruznassre:fix/base-ignores-working-tree-scope

Conversation

@behruznassre

Copy link
Copy Markdown
Contributor

Summary

  • resolveReviewTarget() validates --scope before it looks at --base, so an unknown scope is rejected whether or not --base is given
  • --base <ref> with --scope working-tree is refused with a message that says which flag to drop, instead of silently returning a branch review
  • --base with auto or branch scope, and --scope working-tree on its own, behave as before
  • the review and adversarial-review skills now say not to combine the two flags

Root cause

The if (baseRef) early return came first, so the scope check never ran when --base was present. A working-tree review always compares against HEAD, so there is no meaningful way to honor both flags. Picking one silently is how staged work went unreviewed.

Validation

  • the issue's reproduction on main (19e5651) returns { mode: 'branch', baseRef: 'main' } for both scope: "working-tree" and scope: "bogus"
  • new resolveReviewTarget rows in tests/git.test.mjs: the two rejection rows fail on main and pass with the fix; the branch-scope and working-tree-only rows pass on both
  • companion path: node scripts/claude-companion.mjs review --base main --scope working-tree (and adversarial-review) now exits 1 with the message before any job is created
  • npm run check: unit 518 passed, integration 45 passed, Codex E2E 23 passed; lint, typecheck, version-sync and changelog OK

Fixes #126

🤖 Generated with Claude Code

resolveReviewTarget() returned a branch review for any --base before it
read --scope, so `--base main --scope working-tree` reviewed main...HEAD
and silently skipped the staged and unstaged changes it was asked to
review. An unknown scope passed with --base also skipped validation.

Validate the scope first, then refuse the base/working-tree pair with a
message that says which flag to drop. --base with auto or branch scope
is unchanged. The review skills now say not to combine the two.

Fixes sendbird#126

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

bug: --base silently overrides --scope working-tree, so staged and unstaged changes are not reviewed

1 participant