diff --git a/docs/internals/config-reference.md b/docs/internals/config-reference.md index 04d2b48..d65fae3 100644 --- a/docs/internals/config-reference.md +++ b/docs/internals/config-reference.md @@ -72,7 +72,7 @@ All fields are optional. Defaults are applied when omitted. |---|---|---|---| | `root` | string | `../.repokernel-worktrees` | Root directory for managed worktrees. May be absolute or relative to the main checkout. | | `branchPrefix` | string | `rk/` | Prefix for managed branches. Epic branches: `epic/`. Sprint branches: `sprint//`. | -| `baseBranch` | string | `main` | Branch used as the base when creating a new epic worktree. | +| `baseBranch` | string | `main` | Branch used as the base when creating a new epic worktree. Must be a legal Git branch name and must sit outside the epic and sprint branch namespaces. | | `autoAcquire` | boolean | `true` | `rk run` creates or reuses the epic worktree automatically. | | `branchPattern` | string \| omitted | omitted | Compatibility shorthand. Without `{sprintId}`, applies to epic branches only. With `{sprintId}`, applies to sprint branches only. See below. | | `epicBranchPattern` | string \| omitted | omitted | Explicit epic branch template. Cannot contain `{sprintId}`. | @@ -115,6 +115,8 @@ Prefer explicit `epicBranchPattern` + `sprintBranchPattern` for team-specific na RepoKernel also renders representative epic and sprint refs at config load and validates the final Git ref strings. This catches unsafe `branchPrefix` values, dot-prefixed path components, `.lock` components, accidental double slashes after token substitution, and epic/sprint ref collisions such as `feature/E-001` plus `feature/E-001/S-001`. +`baseBranch` is validated against the same ref rules and is additionally rejected when it falls inside the namespace a branch pattern generates into — everything up to the pattern's first `{epicId}` or `{sprintId}` token. With the default patterns that rules out `rk/epic/...` and `rk/sprint/...` while leaving a base that merely shares `branchPrefix`, such as `release/current` under prefix `release/`, accepted. RepoKernel creates and deletes branches in those namespaces, so a base inside one makes work merged only into a worktree branch look merged into the project's trunk. + **Examples:** ```yaml diff --git a/docs/internals/specs/config.md b/docs/internals/specs/config.md index 2a91766..cb9d393 100644 --- a/docs/internals/specs/config.md +++ b/docs/internals/specs/config.md @@ -73,7 +73,7 @@ All configured `paths` values must be repo-relative. Absolute paths, NUL bytes, |---|---|---|---| | `root` | string | `../.repokernel-worktrees` | Root directory for managed worktrees. May be absolute or relative to the control checkout. | | `branchPrefix` | string | `rk/` | Prefix for managed branches. Epic branches use `epic/`; sprint branches use `sprint//`. | -| `baseBranch` | string | `main` | Base branch used when creating a new epic worktree branch. | +| `baseBranch` | string | `main` | Base branch used when creating a new epic worktree branch. Rejected at config load when it is not a legal Git branch name, or when it falls inside the epic or sprint branch namespace. | | `autoAcquire` | boolean | `true` | `rk run` automatically creates/reuses the epic worktree. | | `branchPattern` | string | omitted | Shorthand branch template. Without `{sprintId}`, applies to epic branches; with `{sprintId}`, applies to sprint branches. Rendered refs are validated at config load. | | `epicBranchPattern` | string | omitted | Explicit epic branch template. Cannot contain `{sprintId}` and must render to a valid non-colliding Git ref. | diff --git a/packages/cli/test/worktree.test.ts b/packages/cli/test/worktree.test.ts index b8dd5b8..a2dd69b 100644 --- a/packages/cli/test/worktree.test.ts +++ b/packages/cli/test/worktree.test.ts @@ -218,3 +218,63 @@ describe('worktree naming — branchPattern', () => { expect(worktreeBranch(eid('E-001'), config)).toBe('feat-2026.q2/E-001_a'); }); }); + +describe('worktree config — baseBranch', () => { + it('accepts the default base branch', () => { + expect(CONFIG.worktrees.baseBranch).toBe('main'); + }); + + it('rejects a baseBranch inside the epic worktree branch namespace', () => { + expect(() => configWithWorktrees({ baseBranch: 'rk/epic/E-001' })).toThrow(); + }); + + it('rejects a baseBranch inside the sprint worktree branch namespace', () => { + expect(() => configWithWorktrees({ baseBranch: 'rk/sprint/E-001/S-001' })).toThrow(); + }); + + it('rejects a baseBranch in the generated namespace that is not an id', () => { + expect(() => configWithWorktrees({ baseBranch: 'rk/epic/legacy' })).toThrow(); + }); + + it('rejects a baseBranch inside a custom epic branch namespace', () => { + expect(() => + configWithWorktrees({ + epicBranchPattern: 'feature/epic/{epicId}', + sprintBranchPattern: 'feature/sprint/{epicId}/{sprintId}', + baseBranch: 'feature/epic/E-001', + }), + ).toThrow(); + }); + + it('rejects a baseBranch equal to a pattern that generates one fixed branch', () => { + expect(() => + configWithWorktrees({ epicBranchPattern: 'devel', baseBranch: 'devel' }), + ).toThrow(); + }); + + it('accepts a baseBranch under branchPrefix that no pattern generates', () => { + const config = configWithWorktrees({ branchPrefix: 'release/', baseBranch: 'release/current' }); + expect(config.worktrees.baseBranch).toBe('release/current'); + }); + + it('accepts a baseBranch that only shares a leading segment with the namespace', () => { + const config = configWithWorktrees({ baseBranch: 'rk-mainline' }); + expect(config.worktrees.baseBranch).toBe('rk-mainline'); + }); + + it('rejects a baseBranch that is not a legal git ref — leading dash', () => { + expect(() => configWithWorktrees({ baseBranch: '-x' })).toThrow(); + }); + + it('rejects a baseBranch that is not a legal git ref — whitespace', () => { + expect(() => configWithWorktrees({ baseBranch: 'my branch' })).toThrow(); + }); + + it('rejects a baseBranch that is not a legal git ref — `..` range syntax', () => { + expect(() => configWithWorktrees({ baseBranch: 'main..dev' })).toThrow(); + }); + + it('rejects a baseBranch that is not a legal git ref — trailing .lock', () => { + expect(() => configWithWorktrees({ baseBranch: 'main.lock' })).toThrow(); + }); +}); diff --git a/packages/core/src/config/schema.ts b/packages/core/src/config/schema.ts index fcd2aa2..9586897 100644 --- a/packages/core/src/config/schema.ts +++ b/packages/core/src/config/schema.ts @@ -195,6 +195,33 @@ function refsConflict(a: string, b: string): boolean { return a === b || a.startsWith(`${b}/`) || b.startsWith(`${a}/`); } +const ID_TOKEN_RE = /\{(?:epicId|sprintId)\}/; + +/** + * The branch namespace a worktree pattern generates into: everything up to its + * first id token, with `{branchPrefix}` substituted. + * + * `isPrefix` is false when the pattern carries no id token at all — it then + * names a single fixed branch rather than a namespace, so containment has to be + * tested as a ref conflict instead of a string prefix. + * + * Cutting at the first id token rather than matching rendered ids keeps the + * test on the namespace RepoKernel claims, so a hand-made `rk/epic/legacy` is + * caught alongside `rk/epic/E-001` while a base that merely shares the + * configured `branchPrefix` (`release/current` under prefix `release/`) is not. + */ +function generatedBranchNamespace( + pattern: string, + branchPrefix: string, +): { readonly value: string; readonly isPrefix: boolean } { + const idToken = ID_TOKEN_RE.exec(pattern); + const head = idToken === null ? pattern : pattern.slice(0, idToken.index); + return { + value: head.replace(/\{branchPrefix\}/g, branchPrefix), + isPrefix: idToken !== null, + }; +} + /** * Validate a `worktrees.branchPattern` template string. * @@ -282,6 +309,14 @@ export const WorktreesSchema = z }); } + if (!isValidGitBranchRef(value.baseBranch)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['baseBranch'], + message: `baseBranch \`${value.baseBranch}\` is not a valid git branch name`, + }); + } + const epicPattern = epicBranchPatternFor(value); const sprintPattern = sprintBranchPatternFor(value); if (hasToken(epicPattern, 'sprintId')) { @@ -301,6 +336,27 @@ export const WorktreesSchema = z if (!hasOnlyCurrentTokens(epicPattern) || !hasOnlyCurrentTokens(sprintPattern)) return; + // A base inside the worktree branch namespace makes every worktree branch + // merged into it look merged into trunk, so cleanup deletes work that never + // reached the real base. + for (const [pattern, label] of [ + [epicPattern, 'epic'], + [sprintPattern, 'sprint'], + ] as const) { + const namespace = generatedBranchNamespace(pattern, value.branchPrefix); + const collides = namespace.isPrefix + ? value.baseBranch.startsWith(namespace.value) + : refsConflict(value.baseBranch, namespace.value); + if (collides) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['baseBranch'], + message: `baseBranch \`${value.baseBranch}\` sits inside the ${label} worktree branch namespace \`${namespace.value}\` — RepoKernel creates and deletes branches there, so the base must live outside it`, + }); + break; + } + } + const sampleCtx = { branchPrefix: value.branchPrefix, epicId: 'E-001', sprintId: 'S-001' }; const epicRef = renderBranchPattern(epicPattern, sampleCtx); const sprintRef = renderBranchPattern(sprintPattern, sampleCtx);