From 2e4d1489ae0549dbe5e96a5f8bef7c5f304223cf Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Thu, 26 Mar 2026 00:34:09 +0100 Subject: [PATCH] fix: guard writeIntegrationBranch against workflow-template branches writeIntegrationBranch already rejects slice branches (SLICE_BRANCH_RE) and quick-task branches (QUICK_BRANCH_RE), but has no guard for the 8 workflow- template branches (gsd/hotfix/*, gsd/bugfix/*, gsd/spike/*, etc.). When a user runs `/gsd start hotfix` during an active milestone, the ephemeral hotfix branch gets recorded as the integration target and the milestone later merges to the wrong branch. Add WORKFLOW_BRANCH_RE (/^gsd\/(?!M\d)[\w-]+\//) that matches all gsd// branches while excluding milestone slice branches (gsd/M001/S01). The negative lookahead ensures milestone branches starting with 'M' followed by a digit are not affected. Same root cause as gsd/quick/* (#1293, PR #1342). Closes #2498 --- src/resources/extensions/gsd/git-service.ts | 13 +++++ .../extensions/gsd/tests/git-service.test.ts | 49 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/resources/extensions/gsd/git-service.ts b/src/resources/extensions/gsd/git-service.ts index 9f17574e5..69851c418 100644 --- a/src/resources/extensions/gsd/git-service.ts +++ b/src/resources/extensions/gsd/git-service.ts @@ -246,6 +246,15 @@ export function readIntegrationBranch(basePath: string, milestoneId: string): st /** Regex matching GSD quick-task branches: gsd/quick/- */ export const QUICK_BRANCH_RE = /^gsd\/quick\//; +/** + * Matches all GSD workflow-template branches: gsd//. + * + * Template IDs are lowercase alphanumeric with hyphens (e.g. hotfix, bugfix, + * small-feature, dep-upgrade). The negative lookahead excludes milestone + * branches (gsd/M001/... or gsd/M001-abc123/...) which use SLICE_BRANCH_RE. + */ +export const WORKFLOW_BRANCH_RE = /^gsd\/(?!M\d)[\w-]+\//; + export function writeIntegrationBranch( basePath: string, milestoneId: string, @@ -257,6 +266,10 @@ export function writeIntegrationBranch( // to their origin branch on completion. Recording one as the integration // target causes milestone merges to land on the wrong branch (#1293). if (QUICK_BRANCH_RE.test(branch)) return; + // Don't record workflow-template branches (hotfix, bugfix, spike, etc.) — + // same root cause as quick-task branches (#2498). All templates create + // gsd// branches that are ephemeral. + if (WORKFLOW_BRANCH_RE.test(branch)) return; // Validate if (!VALID_BRANCH_NAME.test(branch)) return; // Skip if already recorded with the same branch (idempotent across restarts). diff --git a/src/resources/extensions/gsd/tests/git-service.test.ts b/src/resources/extensions/gsd/tests/git-service.test.ts index cd7168ada..3e4b3ffda 100644 --- a/src/resources/extensions/gsd/tests/git-service.test.ts +++ b/src/resources/extensions/gsd/tests/git-service.test.ts @@ -868,6 +868,55 @@ describe('git-service', async () => { rmSync(repo, { recursive: true, force: true }); }); + // ─── writeIntegrationBranch: rejects workflow-template branches (#2498) ─ + + test('Integration branch: rejects workflow-template branches', () => { + const repo = initBranchTestRepo(); + + // All 8 registered workflow templates should be rejected + writeIntegrationBranch(repo, "M001", "gsd/hotfix/fix-login"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "hotfix branch is not recorded"); + + writeIntegrationBranch(repo, "M001", "gsd/bugfix/null-pointer"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "bugfix branch is not recorded"); + + writeIntegrationBranch(repo, "M001", "gsd/small-feature/add-button"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "small-feature branch is not recorded"); + + writeIntegrationBranch(repo, "M001", "gsd/refactor/rename-module"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "refactor branch is not recorded"); + + writeIntegrationBranch(repo, "M001", "gsd/spike/evaluate-lib"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "spike branch is not recorded"); + + writeIntegrationBranch(repo, "M001", "gsd/security-audit/owasp-scan"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "security-audit branch is not recorded"); + + writeIntegrationBranch(repo, "M001", "gsd/dep-upgrade/bump-react"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "dep-upgrade branch is not recorded"); + + writeIntegrationBranch(repo, "M001", "gsd/full-project/new-app"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), null, "full-project branch is not recorded"); + + rmSync(repo, { recursive: true, force: true }); + }); + + // ─── writeIntegrationBranch: still records legitimate branches ──────── + + test('Integration branch: records non-ephemeral gsd branches', () => { + const repo = initBranchTestRepo(); + + // A normal feature branch should still be recorded + writeIntegrationBranch(repo, "M001", "feature/new-thing"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M001"), "feature/new-thing", "normal branches are recorded"); + + // The main branch should be recorded + writeIntegrationBranch(repo, "M002", "main"); + assert.deepStrictEqual(readIntegrationBranch(repo, "M002"), "main", "main branch is recorded"); + + rmSync(repo, { recursive: true, force: true }); + }); + // ─── writeIntegrationBranch: rejects invalid branch names ───────────── test('Integration branch: rejects invalid names', () => {