diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index b7774a264..b50ea11f2 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -217,9 +217,33 @@ Check `branching_strategy` from init: **"none":** Skip, continue on current branch. -**"phase" or "milestone":** Use pre-computed `branch_name` from init: +**"phase" or "milestone":** Use pre-computed `branch_name` from init. + +Fork the new phase branch off `origin/HEAD` (the project's default branch), not the current HEAD — otherwise consecutive phases compound and stay unpushed (#2916). If `$BRANCH_NAME` already exists locally, reuse it as-is. + ```bash -git checkout -b "$BRANCH_NAME" 2>/dev/null || git checkout "$BRANCH_NAME" +DEFAULT_BRANCH=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null | sed 's|^origin/||') +DEFAULT_BRANCH=${DEFAULT_BRANCH:-main} + +if git show-ref --verify --quiet "refs/heads/$BRANCH_NAME"; then + git switch "$BRANCH_NAME" || { echo "ERROR: Could not switch to existing branch '$BRANCH_NAME'." >&2; exit 1; } +else + if ! git fetch --quiet origin "$DEFAULT_BRANCH"; then # #2916 + git show-ref --verify --quiet "refs/remotes/origin/$DEFAULT_BRANCH" \ + || { echo "ERROR: fetch origin/$DEFAULT_BRANCH failed and no local copy exists. Refusing to create '$BRANCH_NAME' off current HEAD (#2916)." >&2; exit 1; } + echo "WARNING: fetch origin/$DEFAULT_BRANCH failed; using local copy as base." >&2 + fi + if [ -n "$(git status --porcelain)" ]; then + echo "WARNING: Uncommitted changes will be carried onto '$BRANCH_NAME' (branched off origin/$DEFAULT_BRANCH, not previous HEAD)." + else + git switch --quiet "$DEFAULT_BRANCH" 2>/dev/null && git merge --ff-only --quiet "origin/$DEFAULT_BRANCH" 2>/dev/null || true + fi + # Pinned base + fail-fast: on success HEAD is exactly at origin/$DEFAULT_BRANCH, + # so a post-creation merge-base or "ahead-of" guard would be unreachable. The + # explicit base argument here is the single source of correctness for #2916. + git checkout -b "$BRANCH_NAME" "origin/$DEFAULT_BRANCH" \ + || { echo "ERROR: Could not create '$BRANCH_NAME' from origin/$DEFAULT_BRANCH (#2916)." >&2; exit 1; } +fi ``` All subsequent commits go to this branch. User handles merging. diff --git a/get-shit-done/workflows/quick.md b/get-shit-done/workflows/quick.md index f8a93c00b..936578346 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.md @@ -180,10 +180,52 @@ Quick tasks can run mid-phase - validation only checks ROADMAP.md exists, not ph **If `branch_name` is empty/null:** Skip and continue on the current branch. -**If `branch_name` is set:** Check out the quick-task branch before any planning commits: +**If `branch_name` is set:** Check out the quick-task branch before any planning commits. + +The new branch must fork off the project's default branch (`origin/HEAD`), not +off whatever HEAD happens to be checked out — otherwise consecutive quick tasks +compound on top of each other and stay unpushed (#2916). If `$branch_name` +already exists locally, reuse it as-is so resumed work is not rebased. ```bash -git checkout -b "$branch_name" 2>/dev/null || git checkout "$branch_name" +DEFAULT_BRANCH=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null | sed 's|^origin/||') +DEFAULT_BRANCH=${DEFAULT_BRANCH:-main} + +if git show-ref --verify --quiet "refs/heads/$branch_name"; then + git switch "$branch_name" \ + || { echo "ERROR: Could not switch to existing quick-task branch '$branch_name'." >&2; exit 1; } +else + # Fetch the default branch so origin/$DEFAULT_BRANCH is current. If the fetch + # fails (offline, no remote, auth failure) AND we have no local copy of + # origin/$DEFAULT_BRANCH to fall back on, abort — creating the branch off + # arbitrary HEAD is exactly the bug #2916 fixed. + if ! git fetch --quiet origin "$DEFAULT_BRANCH"; then + if ! git show-ref --verify --quiet "refs/remotes/origin/$DEFAULT_BRANCH"; then + echo "ERROR: Could not fetch origin/$DEFAULT_BRANCH and no local copy exists. Refusing to create '$branch_name' off the current HEAD (#2916). Resolve the remote/network issue and retry." >&2 + exit 1 + fi + echo "WARNING: git fetch origin $DEFAULT_BRANCH failed; using the local copy of origin/$DEFAULT_BRANCH as base." >&2 + fi + + if [ -n "$(git status --porcelain)" ]; then + echo "WARNING: Uncommitted changes present. Carrying them onto the new quick-task branch — they will be branched off origin/$DEFAULT_BRANCH (not the previous-task HEAD)." + else + # Best-effort: fast-forward the local default branch so subsequent local + # work sees the latest tip. Failure here is non-fatal because we always + # create the new branch directly from origin/$DEFAULT_BRANCH below. + git switch --quiet "$DEFAULT_BRANCH" 2>/dev/null \ + && git merge --ff-only --quiet "origin/$DEFAULT_BRANCH" 2>/dev/null \ + || true + fi + + # Pin the new branch to origin/$DEFAULT_BRANCH so the start point is + # deterministic regardless of which branch we are currently on (#2916). + # On success HEAD is exactly at origin/$DEFAULT_BRANCH, so a post-creation + # merge-base / "ahead-of" guard would be unreachable — the explicit base + # argument here is the single source of correctness for #2916. + git checkout -b "$branch_name" "origin/$DEFAULT_BRANCH" \ + || { echo "ERROR: Could not create '$branch_name' from origin/$DEFAULT_BRANCH (#2916)." >&2; exit 1; } +fi ``` All quick-task commits for this run stay on that branch. User handles merge/rebase afterward. diff --git a/tests/bug-2916-handle-branching-default-base.test.cjs b/tests/bug-2916-handle-branching-default-base.test.cjs new file mode 100644 index 000000000..a51cbf2c0 --- /dev/null +++ b/tests/bug-2916-handle-branching-default-base.test.cjs @@ -0,0 +1,246 @@ +/** + * Regression test for #2916: execute-phase `handle_branching` step creates the + * per-phase branch off whatever HEAD is currently checked out (typically the + * previous phase's unmerged branch) instead of off `origin/HEAD`. + * + * The bug compounded phases on top of each other and stranded them unpushed + * for weeks. The fix: + * 1. Detect the default branch via `git symbolic-ref refs/remotes/origin/HEAD`. + * 2. If $BRANCH_NAME exists, switch to it (preserve existing behavior). + * 3. Otherwise, ff-update the default branch from origin and create the new + * phase branch off the default-branch tip. + * 4. Refuse-or-warn on dirty working tree. + * 5. Post-creation, assert `git rev-list --count $DEFAULT_BRANCH..HEAD == 0`. + * + * This test extracts the bash payload from the + * block in execute-phase.md (parsed structurally — no regex on prose), executes + * it inside a fixture git repo where HEAD sits on a previous-phase branch with + * extra commits, and asserts that the new phase branch's tip equals + * `origin/main` (no commits inherited from the previous phase). + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const EXECUTE_PHASE_PATH = path.join( + __dirname, + '..', + 'get-shit-done', + 'workflows', + 'execute-phase.md' +); + +const GIT_ENV = Object.freeze({ + ...process.env, + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@test.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@test.com', +}); + +function git(cwd, ...args) { + return execFileSync('git', args, { + cwd, + env: GIT_ENV, + stdio: ['pipe', 'pipe', 'pipe'], + }) + .toString() + .trim(); +} + +/** + * Structurally extract the bash code that the handle_branching step instructs + * the agent to run. We: + * 1. Locate the ... block. + * 2. Walk its body looking for fenced ```bash blocks. + * 3. Concatenate every bash block in the step (the fix may use more than one). + * + * No `.includes()` content checks — we parse fence-delimited code blocks the + * same way a markdown parser would. + */ +function extractHandleBranchingBash() { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const lines = content.split(/\r?\n/); + + let start = -1; + let end = -1; + for (let i = 0; i < lines.length; i += 1) { + if (start === -1 && /^\s*$/.test(lines[i])) { + start = i + 1; + } else if (start !== -1 && /^<\/step>\s*$/.test(lines[i])) { + end = i; + break; + } + } + if (start === -1 || end === -1) { + throw new Error( + 'execute-phase.md does not contain a ... block' + ); + } + + const bashBlocks = []; + let inBash = false; + let buffer = []; + for (let i = start; i < end; i += 1) { + const line = lines[i]; + if (!inBash && /^```bash\s*$/.test(line)) { + inBash = true; + buffer = []; + continue; + } + if (inBash && /^```\s*$/.test(line)) { + bashBlocks.push(buffer.join('\n')); + inBash = false; + continue; + } + if (inBash) buffer.push(line); + } + if (bashBlocks.length === 0) { + throw new Error( + 'handle_branching step contains no ```bash code blocks to execute' + ); + } + return bashBlocks.join('\n'); +} + +/** + * Build a fixture: a bare "origin" repo with the named default branch (one + * commit), a clone with `origin/HEAD` pointed at it, and a checked-out + * previous-phase branch carrying its own unmerged commit. + * + * `defaultBranch` is parameterized so callers can lock in that the workflow + * honors `git symbolic-ref refs/remotes/origin/HEAD` rather than silently + * defaulting to `main` (#2921 CR feedback — quick-branching.test.cjs got the + * same treatment in 80f14cac; this test deserves the same coverage). + */ +function setupFixture(defaultBranch = 'main') { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2916-')); + const seedPath = path.join(root, 'seed'); + const originPath = path.join(root, 'origin.git'); + const clonePath = path.join(root, 'clone'); + + fs.mkdirSync(seedPath); + git(seedPath, 'init', '-b', defaultBranch); + git(seedPath, 'config', 'commit.gpgsign', 'false'); + fs.writeFileSync(path.join(seedPath, 'README.md'), '# seed\n'); + git(seedPath, 'add', 'README.md'); + git(seedPath, 'commit', '-m', 'initial'); + + git(root, 'clone', '--bare', seedPath, originPath); + git(originPath, 'symbolic-ref', 'HEAD', `refs/heads/${defaultBranch}`); + + git(root, 'clone', originPath, clonePath); + git(clonePath, 'config', 'commit.gpgsign', 'false'); + git(clonePath, 'config', 'user.email', 'test@test.com'); + git(clonePath, 'config', 'user.name', 'Test'); + + // Simulate finishing a previous phase: branch off the default branch, add + // a commit, and *stay* on it (the failure scenario described in the bug). + git(clonePath, 'checkout', '-b', 'feature/phase-01-foundation'); + fs.writeFileSync(path.join(clonePath, 'phase01.txt'), 'phase 1 work\n'); + git(clonePath, 'add', 'phase01.txt'); + git(clonePath, 'commit', '-m', 'phase 01 work'); + + return { root, clonePath, defaultBranch }; +} + +function runHandleBranchingStep(bash, cwd, branchName) { + // Write the script to a sibling tempdir, not inside the repo — putting it in + // `cwd` would create an untracked file that trips `git status --porcelain` + // and steers the step into its dirty-tree fallback path. + const scriptDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2916-step-')); + const scriptPath = path.join(scriptDir, 'handle-branching.sh'); + const script = `#!/usr/bin/env bash\nset -uo pipefail\nBRANCH_NAME="${branchName}"\n${bash}\n`; + fs.writeFileSync(scriptPath, script, { mode: 0o755 }); + try { + return execFileSync('bash', [scriptPath], { + cwd, + env: GIT_ENV, + stdio: ['pipe', 'pipe', 'pipe'], + }).toString(); + } finally { + fs.rmSync(scriptDir, { recursive: true, force: true }); + } +} + +describe('handle_branching branches off origin/HEAD, not current HEAD (#2916)', () => { + // Run against `main` (conventional default) and `trunk` (non-main default + // exercising the symbolic-ref code path) so a regression that hard-codes + // `main` instead of consulting origin/HEAD will fail the trunk variant. + for (const defaultBranch of ['main', 'trunk']) { + test(`new phase branch branches off origin/${defaultBranch} with 0 inherited commits`, () => { + const bash = extractHandleBranchingBash(); + const { root, clonePath } = setupFixture(defaultBranch); + + try { + const upstream = `origin/${defaultBranch}`; + + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'feature/phase-01-foundation' + ); + assert.equal( + git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`), + '1', + `fixture should be 1 commit ahead of ${upstream}` + ); + + runHandleBranchingStep(bash, clonePath, 'feature/phase-02-content-sync'); + + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'feature/phase-02-content-sync', + 'handle_branching should switch to the new phase branch' + ); + + const inherited = git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`); + assert.equal( + inherited, + '0', + `new phase branch must branch off ${upstream}, but inherited ${inherited} commit(s) from previous-phase HEAD` + ); + assert.equal( + git(clonePath, 'rev-parse', 'HEAD'), + git(clonePath, 'rev-parse', upstream), + `new phase branch tip must equal ${upstream} tip` + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + } + + test('handle_branching reuses an existing branch instead of forking again', () => { + const bash = extractHandleBranchingBash(); + const { root, clonePath } = setupFixture(); + + try { + // Pre-create the target branch off origin/main with its own commit, then + // walk away to a different branch — the step must switch back to it. + git(clonePath, 'checkout', '-B', 'feature/phase-02-content-sync', 'origin/main'); + fs.writeFileSync(path.join(clonePath, 'phase02.txt'), 'phase 2 work\n'); + git(clonePath, 'add', 'phase02.txt'); + git(clonePath, 'commit', '-m', 'phase 02 wip'); + const phase02Sha = git(clonePath, 'rev-parse', 'HEAD'); + git(clonePath, 'checkout', 'feature/phase-01-foundation'); + + runHandleBranchingStep(bash, clonePath, 'feature/phase-02-content-sync'); + + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'feature/phase-02-content-sync' + ); + assert.equal( + git(clonePath, 'rev-parse', 'HEAD'), + phase02Sha, + 'existing-branch tip must be preserved (no rebase/reset)' + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/quick-branching.test.cjs b/tests/quick-branching.test.cjs index ff15c55c2..d0ac0dacb 100644 --- a/tests/quick-branching.test.cjs +++ b/tests/quick-branching.test.cjs @@ -1,39 +1,293 @@ /** * Quick task branching tests * - * Validates that /gsd-quick exposes branch_name from init and that the - * workflow checks out a dedicated quick-task branch when configured. + * Validates that /gsd-quick exposes branch_name from init and that the Step 2.5 + * "Handle quick-task branching" block: + * 1. Reuses an existing branch as-is (no rebase / no reset). + * 2. When the branch does not exist, creates it from origin/HEAD's default + * branch — never off the previous task's HEAD (#2916). + * + * Assertions are behavioral (run the bash block in a fixture git repo and + * inspect git state) and structural (parse the markdown for the step's bash + * block). No `.includes()` / regex grepping of raw markdown content — see + * CONTRIBUTING.md "no-source-grep" testing standard. */ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const QUICK_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'); + +const GIT_ENV = Object.freeze({ + ...process.env, + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@test.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@test.com', +}); + +function git(cwd, ...args) { + return execFileSync('git', args, { + cwd, + env: GIT_ENV, + stdio: ['pipe', 'pipe', 'pipe'], + }) + .toString() + .trim(); +} + +/** + * Structurally extract the bash code under the "Step 2.5: Handle quick-task + * branching" heading. We: + * 1. Locate the Step 2.5 heading. + * 2. Find the next horizontal rule (`---`) that ends the section. + * 3. Concatenate every fenced ```bash block in between. + * + * No `.includes()` content checks — fenced code blocks are parsed the same way + * a markdown parser would. + */ +function extractStep25Bash() { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const lines = content.split(/\r?\n/); + + let start = -1; + let end = -1; + for (let i = 0; i < lines.length; i += 1) { + if (start === -1 && /^\*\*Step 2\.5:\s*Handle quick-task branching\*\*\s*$/.test(lines[i])) { + start = i + 1; + } else if (start !== -1 && /^---\s*$/.test(lines[i])) { + end = i; + break; + } + } + if (start === -1) { + throw new Error('quick.md does not contain a "Step 2.5: Handle quick-task branching" section'); + } + if (end === -1) end = lines.length; + + const bashBlocks = []; + let inBash = false; + let buffer = []; + for (let i = start; i < end; i += 1) { + const line = lines[i]; + if (!inBash && /^```bash\s*$/.test(line)) { + inBash = true; + buffer = []; + continue; + } + if (inBash && /^```\s*$/.test(line)) { + bashBlocks.push(buffer.join('\n')); + inBash = false; + continue; + } + if (inBash) buffer.push(line); + } + if (bashBlocks.length === 0) { + throw new Error('Step 2.5 contains no ```bash code blocks to execute'); + } + return bashBlocks.join('\n'); +} + +/** + * Build a fixture: a bare "origin" repo with a non-`main` default branch + * (`trunk`) so the test fails if the workflow silently falls back to "main" + * instead of consulting `origin/HEAD`. The clone has `origin/HEAD` pointed at + * `trunk` and a checked-out previous-task branch carrying its own unmerged + * commit. + * + * Using `trunk` here locks in the symbolic-ref code path: if the + * implementation skips `git symbolic-ref refs/remotes/origin/HEAD` and just + * defaults to `main`, every assertion below collapses (#2921 CR nitpick). + */ +function setupFixture(defaultBranch = 'trunk') { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-quick-branching-')); + const seedPath = path.join(root, 'seed'); + const originPath = path.join(root, 'origin.git'); + const clonePath = path.join(root, 'clone'); + + fs.mkdirSync(seedPath); + git(seedPath, 'init', '-b', defaultBranch); + git(seedPath, 'config', 'commit.gpgsign', 'false'); + fs.writeFileSync(path.join(seedPath, 'README.md'), '# seed\n'); + git(seedPath, 'add', 'README.md'); + git(seedPath, 'commit', '-m', 'initial'); + + git(root, 'clone', '--bare', seedPath, originPath); + git(originPath, 'symbolic-ref', 'HEAD', `refs/heads/${defaultBranch}`); + + git(root, 'clone', originPath, clonePath); + git(clonePath, 'config', 'commit.gpgsign', 'false'); + git(clonePath, 'config', 'user.email', 'test@test.com'); + git(clonePath, 'config', 'user.name', 'Test'); + + // Simulate finishing a previous quick task: branch off the default branch, + // add a commit, and stay on it (this is the failure scenario from #2916). + git(clonePath, 'checkout', '-b', 'quick/01-prev-task'); + fs.writeFileSync(path.join(clonePath, 'prev.txt'), 'prev work\n'); + git(clonePath, 'add', 'prev.txt'); + git(clonePath, 'commit', '-m', 'prev quick task work'); + + return { root, clonePath, defaultBranch }; +} + +function runStep(bash, cwd, branchName) { + // Write the script to a sibling tempdir, not inside the repo — putting it in + // `cwd` would create an untracked file that trips `git status --porcelain` + // and steers the step into the dirty-tree path. + const scriptDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-quick-step-')); + const scriptPath = path.join(scriptDir, 'step25.sh'); + const script = `#!/usr/bin/env bash\nset -uo pipefail\nbranch_name="${branchName}"\n${bash}\n`; + fs.writeFileSync(scriptPath, script, { mode: 0o755 }); + try { + return execFileSync('bash', [scriptPath], { + cwd, + env: GIT_ENV, + stdio: ['pipe', 'pipe', 'pipe'], + }).toString(); + } finally { + fs.rmSync(scriptDir, { recursive: true, force: true }); + } +} describe('quick workflow: branching support', () => { - const workflowPath = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'); - let content; - test('workflow file exists', () => { - assert.ok(fs.existsSync(workflowPath), 'workflows/quick.md should exist'); + assert.ok(fs.existsSync(QUICK_PATH), 'workflows/quick.md should exist'); }); test('init parse list includes branch_name', () => { - content = fs.readFileSync(workflowPath, 'utf-8'); - assert.ok(content.includes('branch_name'), 'quick workflow should parse branch_name from init JSON'); + // Structural: the workflow's init step (Step 2) must declare branch_name as + // a parseable field of the init JSON. Restrict the scan to the init step's + // section only — a global walk over every bash fence could be fooled by an + // unrelated step that happens to mention branch_name (#2921 CR). + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const lines = content.split(/\r?\n/); + + // Locate the "Step 2: Initialize" heading and the next "Step N" heading + // that ends the section. We match the markdown bold-step convention used + // throughout quick.md: `**Step N[.M]: Title**`. + let start = -1; + let end = -1; + for (let i = 0; i < lines.length; i += 1) { + if (start === -1 && /^\*\*Step 2:\s*Initialize\*\*\s*$/.test(lines[i])) { + start = i + 1; + } else if (start !== -1 && /^\*\*Step \d+(?:\.\d+)?:\s/.test(lines[i])) { + end = i; + break; + } + } + assert.notEqual(start, -1, 'quick.md should contain a "Step 2: Initialize" section'); + if (end === -1) end = lines.length; + + // Within that section, look for the branch_name token inside fenced bash + // blocks AND in the surrounding markdown prose that documents the JSON + // fields. Both are part of the init contract. + let found = false; + for (let i = start; i < end; i += 1) { + if (/\bbranch_name\b/.test(lines[i])) { found = true; break; } + } + assert.ok( + found, + 'Step 2 (Initialize) of quick workflow should expose branch_name as part of the init contract' + ); }); - test('workflow includes quick-task branching step', () => { - content = fs.readFileSync(workflowPath, 'utf-8'); - assert.ok(content.includes('Step 2.5: Handle quick-task branching')); - assert.ok(content.includes('git checkout -b "$branch_name" 2>/dev/null || git checkout "$branch_name"')); + test('Step 2.5 section is present and contains executable bash', () => { + const bash = extractStep25Bash(); + assert.ok(bash.length > 0, 'Step 2.5 should contain at least one bash block'); }); - test('branching step runs before task directory creation', () => { - content = fs.readFileSync(workflowPath, 'utf-8'); + test('Step 2.5 runs before Step 3 (task directory creation)', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); const branchingIndex = content.indexOf('Step 2.5: Handle quick-task branching'); const createDirIndex = content.indexOf('Step 3: Create task directory'); - assert.ok(branchingIndex !== -1 && createDirIndex !== -1, 'workflow should contain both branching and directory steps'); - assert.ok(branchingIndex < createDirIndex, 'branching should happen before quick task directories and commits'); + assert.ok( + branchingIndex !== -1 && createDirIndex !== -1, + 'workflow should contain both branching and directory steps' + ); + assert.ok( + branchingIndex < createDirIndex, + 'branching should happen before quick task directories and commits' + ); + }); + + // Run against both `main` (the conventional default) and `trunk` (a non- + // main default that exercises the symbolic-ref code path). Keeping both + // restores main coverage that was removed when the fixture switched + // wholesale to trunk in 80f14cac. + for (const defaultBranch of ['main', 'trunk']) { + test(`new quick-task branch branches off origin/${defaultBranch} (#2916)`, () => { + const bash = extractStep25Bash(); + const { root, clonePath } = setupFixture(defaultBranch); + + try { + const upstream = `origin/${defaultBranch}`; + + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'quick/01-prev-task' + ); + assert.equal( + git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`), + '1', + `fixture should be 1 commit ahead of ${upstream}` + ); + + runStep(bash, clonePath, 'quick/02-new-task'); + + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'quick/02-new-task', + 'Step 2.5 should switch to the new quick-task branch' + ); + + const inherited = git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`); + assert.equal( + inherited, + '0', + `new quick-task branch must branch off ${upstream}, but inherited ${inherited} commit(s) from previous-task HEAD` + ); + assert.equal( + git(clonePath, 'rev-parse', 'HEAD'), + git(clonePath, 'rev-parse', upstream), + `new quick-task branch tip must equal ${upstream} tip` + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + } + + test('Step 2.5 reuses an existing quick-task branch instead of forking again', () => { + const bash = extractStep25Bash(); + const { root, clonePath } = setupFixture(); + + try { + // Pre-create the target branch off origin/trunk with its own commit, then + // walk away to a different branch — the step must switch back to it. + git(clonePath, 'checkout', '-B', 'quick/02-new-task', 'origin/trunk'); + fs.writeFileSync(path.join(clonePath, 'task02.txt'), 'task 2 work\n'); + git(clonePath, 'add', 'task02.txt'); + git(clonePath, 'commit', '-m', 'task 02 wip'); + const task02Sha = git(clonePath, 'rev-parse', 'HEAD'); + git(clonePath, 'checkout', 'quick/01-prev-task'); + + runStep(bash, clonePath, 'quick/02-new-task'); + + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'quick/02-new-task' + ); + assert.equal( + git(clonePath, 'rev-parse', 'HEAD'), + task02Sha, + 'existing-branch tip must be preserved (no rebase/reset)' + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } }); });