fix: reject a worktrees baseBranch inside the managed branch namespace - #52
Conversation
worktrees.baseBranch was validated only as a non-empty string, so it could name any ref — including one RepoKernel itself creates and later deletes. Basing worktrees on an epic branch makes every sprint branch merged into that epic register as merged into the project's base, so branch cleanup removes work that never reached trunk. Validate it the way branchPrefix already is: reject illegal Git branch names, and reject a base that falls inside the namespace an epic or sprint branch pattern generates into. That namespace is the pattern up to its first id token, so a hand-made branch there is caught alongside a rendered one, while a base that merely shares branchPrefix stays accepted.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05e656fc68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const collides = namespace.isPrefix | ||
| ? value.baseBranch.startsWith(namespace.value) | ||
| : refsConflict(value.baseBranch, namespace.value); |
There was a problem hiding this comment.
When an otherwise-valid pattern begins with {epicId} or {sprintId}, generatedBranchNamespace returns an empty prefix, so baseBranch.startsWith('') rejects every possible base branch. For example, epicBranchPattern: "{epicId}/epic", sprintBranchPattern: "{epicId}/sprint/{sprintId}", and baseBranch: "main" can no longer load even though the generated E-<number>/... refs cannot collide with main; handle the empty-prefix case using the rendered ID shape rather than treating it as the entire branch space.
Useful? React with 👍 / 👎.
1 participant
worktrees.baseBranchwas validated only asz.string().min(1), so it could name any ref — including one RepoKernel creates and later deletes. Point it at an epic branch and every sprint branch merged into that epic registers as merged into the project's base, sork worktree sweep --applyforce-deletes work that never reached trunk.Two checks added to
WorktreesSchema, both hard errors at config load, matching howbranchPrefixis already validated:git worktree add -b <branch> <path> <baseRef>, where a leading-is argument injection.{epicId}/{sprintId}token. That catches a hand-maderk/epic/legacyalongside a renderedrk/epic/E-001, while leaving a base that merely sharesbranchPrefix(release/currentunder prefixrelease/) accepted, so the arrangement feat: sweep merged worktree branches that outlived their worktree #51 documents as supported keeps working.Test plan
packages/cli/test/worktree.test.ts, next to the existing branch-pattern testspnpm typecheckclean; core 561 pass, cli 1621 pass — includingnever sweeps the base branch when the base sits under the prefix