Pull request overview

Adds a CLI --branch=NAME override to explicitly control the feature branch/worktree name (instead of deriving it from the plan filename), improving robustness for generic plan filenames and spec-driven layouts.

Changes:

• Introduces --branch flag and threads the override through plan execution (normal + worktree + continuation paths).
• Updates git service APIs (CreateBranchForPlan, CreateWorktreeForPlan, internal preparePlanBranch) to accept and use the override.
• Updates docs and extends git service tests with override scenarios.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

	wtPlanFile := resolveWorktreePlanFile(req.PlanFile, req.GitSvc.Root())

	// commit plan file on the feature branch (inside worktree), not on the default branch
	if planNeedsCommit {
		if commitErr := wtGitSvc.CommitPlanFile(req.PlanFile, req.GitSvc.Root()); commitErr != nil {
			return fmt.Errorf("commit plan in worktree: %w", commitErr)
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

couple of things to address before merge:

1. --branch is silently dropped in --plan mode

at cmd/ralphex/main.go:292-301, the runPlanMode branch builds executePlanRequest without BranchOverride: o.Branch. The non-plan path at line 314 sets it correctly. So ralphex --plan "..." --branch=foo ignores the override entirely, which is exactly the generated/generic-filename case the flag is supposed to help with. Fix is one line, but worth a test that locks the wiring.

2. CommitPlanFile commit message ignores the override

pkg/git/service.go:330 still does branchName := plan.ExtractBranchName(planFile) to build the commit subject. With --branch=my-feature on a 2026-04-30-some-name.md plan, the worktree is on my-feature but the auto-commit reads add plan: some-name. Copilot flagged this too. Easiest fix: have CommitPlanFile use the current branch from git rather than re-deriving from the filename, then this whole class of bug goes away.

3. branch-name derivation is duplicated in 4 places

preparePlanBranch:189, CreateWorktreeForPlan early at :280, runWithWorktree logger at cmd/ralphex/main.go:650, and the broken CommitPlanFile:330. Each one repeats if branchOverride != "" { use it } else { ExtractBranchName(planFile) }. Since most call sites are inside Service, makes sense as a method on *Service, something like s.effectiveBranchName(planFile, override), then all four sites delegate to it. That alone would have prevented #2 and removes the risk of forgetting a site for #1.

4. new override subtests are misplaced

at pkg/git/service_test.go:1261 and :1281. Both live inside TestService_RemoveWorktree (starts at 1200) but exercise CreateWorktreeForPlan and CreateBranchForPlan. Should move to TestService_CreateWorktreeForPlan (line 845) and TestService_CreateBranchForPlan (line 186) respectively. Tests-as-documentation gets weaker when the subtest name doesn't match its parent.

5. gofmt drift

gofmt -d cmd/ralphex/main.go shows a struct alignment diff on the BranchOverride field, the name is longer than the existing fields and the rest of the struct wants realignment. Golangci-lint doesn't catch it because gofmt isn't enabled in formatters: in .golangci.yml. make fmt fixes it. Small but a follow-up format pass will show a noisy diff if left.

Notes that aren't blockers

• Copilot's path-traversal flag on service.go:282 is a false positive on impact. filepath.Join calls Clean which collapses .., then git worktree add -b rejects the bad ref before any directory is created (verified with git check-ref-format --branch '../../outside' exit 128). Cleanup never reaches an attacker-chosen path because WtCleanup is registered after successful creation. You may still want a defense-in-depth sanitizeBranchName for clearer error messages, but it's not a security issue.
• PR body says "Closes part of #306", just confirming you don't intend the merge to auto-close #306, since the issue has more aspects (header parsing, MovePlanToCompleted skip, sibling spec files) that this PR correctly leaves for follow-up. "Related to #306" might be safer phrasing.
• Consider also a test for --branch=existing-feature-branch (does it switch or error?) and one for the runWithWorktree → CommitPlanFile path with override set, which would have caught #2.

thx for working through these.

1, 2, 3, 5 all good. 2's currentBranch() approach is cleaner than what I suggested, sidesteps the threading entirely.

minor on 4: the CreateBranchForPlan override subtest is in the right place (TestService_CreateBranchForPlan:427), but the CreateWorktreeForPlan one ended up in TestService_CommitPlanFile:1217 instead of TestService_CreateWorktreeForPlan - the body only exercises CreateWorktreeForPlan, no commit verification. One-line move would finish it.

also right call reverting f5cd336, the path-traversal validation wasn't needed.

lgtm with the test-placement nit, fine to merge as-is.