fix: accept flag-first syntax for phase remove (#3416)

* fix(phase): parse remove --force regardless of position

* chore(changeset): add fragment for phase-remove flag fix

* fix(phase): fail when phase.remove target is missing

.changeset/3409-phase-remove-force-position.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3416
+---
+**`phase remove --force <phase>` now works correctly across both CLI and SDK query paths (#3409)** — flag parsing no longer treats `--force` as the phase id when the flag appears before the positional argument, preventing false success responses and unintended `STATE.md` phase-count drift.

get-shit-done/bin/lib/phase-command-router.cjs

@@ -66,7 +66,25 @@ function routePhaseCommand({ phase, args, cwd, raw, error }) {
         }
         phase.cmdPhaseInsert(cwd, args[2], args.slice(3).join(' '), raw);
       },
-      remove: () => phase.cmdPhaseRemove(cwd, args[2], { force: args.includes('--force') }, raw),
+      remove: () => {
+        const removeArgs = args.slice(2).filter(token => token !== '--raw');
+        let forceFlag = false;
+        const positional = [];
+        for (const token of removeArgs) {
+          if (token === '--force') {
+            forceFlag = true;
+            continue;
+          }
+          if (token.startsWith('--')) {
+            error(`phase remove does not support ${token}`);
+          }
+          positional.push(token);
+        }
+        if (positional.length > 1) {
+          error('phase remove accepts exactly one phase number');
+        }
+        phase.cmdPhaseRemove(cwd, positional[0], { force: forceFlag }, raw);
+      },
       complete: () => phase.cmdPhaseComplete(cwd, args[2], raw),
     },
   });

sdk/src/query/phase-lifecycle.test.ts

@@ -972,6 +972,22 @@ describe('phaseRemove', () => {
     expect(data.directory_deleted).toBeTruthy();
   });
 
+  it('bug-3409: accepts --force before phase id', async () => {
+    const { phaseRemove } = await import('./phase-lifecycle.js');
+    const phasesDir = join(tmpDir, '.planning', 'phases');
+    await setupTestProject(tmpDir, {
+      roadmap: ROADMAP_FOR_REMOVE,
+      state: STATE_FOR_REMOVE,
+      phases: ['05-auth', '06-dashboard', '07-api'],
+    });
+    await writeFile(join(phasesDir, '06-dashboard', '06-01-SUMMARY.md'), 'summary', 'utf-8');
+
+    const result = await phaseRemove(['--force', '6'], tmpDir);
+    const data = result.data as Record<string, unknown>;
+    expect(data.removed).toBe('6');
+    expect(data.directory_deleted).toBeTruthy();
+  });
+
   it('throws GSDError when ROADMAP.md is missing', async () => {
     const { phaseRemove } = await import('./phase-lifecycle.js');
     // Set up without ROADMAP.md
@@ -994,6 +1010,19 @@ describe('phaseRemove', () => {
     await expect(phaseRemove([], tmpDir)).rejects.toThrow('phase number required');
   });
 
+  it('throws GSDError when target phase does not exist and does not mutate STATE.md', async () => {
+    const { phaseRemove } = await import('./phase-lifecycle.js');
+    await setupTestProject(tmpDir, {
+      roadmap: ROADMAP_FOR_REMOVE,
+      state: STATE_FOR_REMOVE,
+      phases: ['05-auth', '06-dashboard', '07-api'],
+    });
+
+    await expect(phaseRemove(['99'], tmpDir)).rejects.toThrow('Phase 99 not found');
+    const stateContent = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
+    expect(stateContent).toMatch(/total_phases:\s*7/);
+  });
+
   it('updates ROADMAP.md by removing phase section and renumbering', async () => {
     const { phaseRemove } = await import('./phase-lifecycle.js');
     await setupTestProject(tmpDir, {

sdk/src/query/phase-lifecycle.ts

@@ -842,7 +842,24 @@ async function updateRoadmapAfterPhaseRemoval(
  * @returns QueryResult with { removed, directory_deleted, renamed_directories, renamed_files, roadmap_updated, state_updated }
  */
 export const phaseRemove: QueryHandler = async (args, projectDir, workstream) => {
-  const targetPhase = args[0];
+  let force = false;
+  const positional: string[] = [];
+  for (const token of args) {
+    if (token === '--force') {
+      force = true;
+      continue;
+    }
+    if (token.startsWith('--')) {
+      throw new GSDError(`phase remove does not support ${token}`, ErrorClassification.Validation);
+    }
+    positional.push(token);
+  }
+
+  if (positional.length > 1) {
+    throw new GSDError('phase remove accepts exactly one phase number', ErrorClassification.Validation);
+  }
+
+  const targetPhase = positional[0];
   if (!targetPhase) {
     throw new GSDError('phase number required for phase remove', ErrorClassification.Validation);
   }
@@ -857,15 +874,17 @@ export const phaseRemove: QueryHandler = async (args, projectDir, workstream) =>
 
   const normalized = normalizePhaseName(targetPhase);
   const isDecimal = targetPhase.includes('.');
-  const force = args[1] === '--force';
 
   // Find target directory
   const entries = await readdir(phasesDir, { withFileTypes: true });
   const dirs = entries.filter(e => e.isDirectory()).map(e => e.name);
   const targetDir = dirs.find(d => phaseTokenMatches(d, normalized)) ?? null;
+  if (!targetDir) {
+    throw new GSDError(`Phase ${targetPhase} not found`, ErrorClassification.Validation);
+  }
 
   // Guard against removing executed work
-  if (targetDir && !force) {
+  if (!force) {
     const files = await readdir(join(phasesDir, targetDir));
     const summaries = files.filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md');
     if (summaries.length > 0) {
@@ -877,9 +896,7 @@ export const phaseRemove: QueryHandler = async (args, projectDir, workstream) =>
   }
 
   // Delete directory
-  if (targetDir) {
-    await rm(join(phasesDir, targetDir), { recursive: true, force: true });
-  }
+  await rm(join(phasesDir, targetDir), { recursive: true, force: true });
 
   // Renumber subsequent phases on disk
   let renamedDirs: Array<{ from: string; to: string }> = [];

tests/phase.test.cjs

@@ -1491,6 +1491,30 @@ describe('phase remove command', () => {
     assert.ok(forceResult.success, `Force remove failed: ${forceResult.error}`);
   });
 
+  test('bug-3409: supports --force before phase id', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'ROADMAP.md'),
+      `# Roadmap\n### Phase 1: A\n**Goal:** A\n### Phase 2: B\n**Goal:** B\n`
+    );
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      `# State\n\n**Current Phase:** 1\n**Total Phases:** 2\n`
+    );
+    fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-a'), { recursive: true });
+    fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-b'), { recursive: true });
+
+    const result = runGsdTools('phase remove --force 2', tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const output = JSON.parse(result.output);
+    assert.strictEqual(output.removed, '2');
+    assert.strictEqual(output.directory_deleted, '02-b');
+    assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', 'phases', '02-b')));
+
+    const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
+    assert.ok(state.includes('**Total Phases:** 1'), 'total phases should be decremented after real removal');
+  });
+
   test('removes decimal phase and renumbers siblings', () => {
     fs.writeFileSync(
       path.join(tmpDir, '.planning', 'ROADMAP.md'),