From 2a1bd3a265657d4bf136cd3091f9049b5629ca5a Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Sun, 12 Apr 2026 13:28:49 +0200 Subject: [PATCH] fix(gsd): detach auto start from active turns --- src/resources/extensions/gsd/auto.ts | 66 ++++++++++++++ src/resources/extensions/gsd/auto/session.ts | 8 ++ .../extensions/gsd/commands/handlers/auto.ts | 46 +++------- .../gsd/commands/handlers/workflow.ts | 14 +-- src/resources/extensions/gsd/guided-flow.ts | 15 ++-- .../extensions/gsd/interrupted-session.ts | 1 + .../gsd/tests/start-auto-detached.test.ts | 90 +++++++++++++++++++ 7 files changed, 184 insertions(+), 56 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/start-auto-detached.test.ts diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index 1b8d4fd47..de4b4a350 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -165,6 +165,7 @@ import { reconcileMergeState, } from "./auto-recovery.js"; import { resolveDispatch, DISPATCH_RULES } from "./auto-dispatch.js"; +import { getErrorMessage } from "./error-utils.js"; import { initRegistry, convertDispatchRules } from "./rule-registry.js"; import { emitJournalEvent as _emitJournalEvent, type JournalEntry } from "./journal.js"; import { @@ -272,6 +273,53 @@ function restoreProjectRootEnv(): void { s.projectRootEnvCaptured = false; } +function captureMilestoneLockEnv(milestoneId: string | null): void { + if (!s.milestoneLockEnvCaptured) { + s.hadMilestoneLockEnv = Object.prototype.hasOwnProperty.call(process.env, "GSD_MILESTONE_LOCK"); + s.previousMilestoneLockEnv = process.env.GSD_MILESTONE_LOCK ?? null; + s.milestoneLockEnvCaptured = true; + } + + if (milestoneId) { + process.env.GSD_MILESTONE_LOCK = milestoneId; + } else { + delete process.env.GSD_MILESTONE_LOCK; + } +} + +function restoreMilestoneLockEnv(): void { + if (!s.milestoneLockEnvCaptured) return; + + if (s.hadMilestoneLockEnv && s.previousMilestoneLockEnv !== null) { + process.env.GSD_MILESTONE_LOCK = s.previousMilestoneLockEnv; + } else { + delete process.env.GSD_MILESTONE_LOCK; + } + + s.previousMilestoneLockEnv = null; + s.hadMilestoneLockEnv = false; + s.milestoneLockEnvCaptured = false; +} + +export function startAutoDetached( + ctx: ExtensionCommandContext, + pi: ExtensionAPI, + base: string, + verboseMode: boolean, + options?: { + step?: boolean; + interrupted?: InterruptedSessionAssessment; + milestoneLock?: string | null; + }, +): void { + void startAuto(ctx, pi, base, verboseMode, options).catch((err) => { + const message = getErrorMessage(err); + ctx.ui.notify(`Auto-start failed: ${message}`, "error"); + logWarning("engine", `auto start error: ${message}`, { file: "auto.ts" }); + debugLog("auto-start-failed", { error: message }); + }); +} + export function shouldUseWorktreeIsolation(): boolean { const prefs = loadEffectiveGSDPreferences()?.preferences?.git; if (prefs?.isolation === "worktree") return true; @@ -574,6 +622,7 @@ function handleLostSessionLock( s.paused = false; clearUnitTimeout(); restoreProjectRootEnv(); + restoreMilestoneLockEnv(); deregisterSigtermHandler(); clearCmuxSidebar(loadEffectiveGSDPreferences()?.preferences); const base = lockBase(); @@ -610,6 +659,7 @@ function cleanupAfterLoopExit(ctx: ExtensionContext): void { s.active = false; clearUnitTimeout(); restoreProjectRootEnv(); + restoreMilestoneLockEnv(); // Clear crash lock and release session lock so the next `/gsd next` does // not see a stale lock with the current PID and treat it as a "remote" @@ -880,6 +930,7 @@ export async function stopAuto( ctx?.ui.setWidget("gsd-progress", undefined); ctx?.ui.setFooter(undefined); restoreProjectRootEnv(); + restoreMilestoneLockEnv(); // Reset all session state in one call s.reset(); @@ -933,6 +984,7 @@ export async function pauseAuto( activeEngineId: s.activeEngineId, activeRunDir: s.activeRunDir, autoStartTime: s.autoStartTime, + milestoneLock: s.sessionMilestoneLock ?? undefined, }; const runtimeDir = join(gsdRoot(s.originalBasePath || s.basePath), "runtime"); mkdirSync(runtimeDir, { recursive: true }); @@ -971,6 +1023,7 @@ export async function pauseAuto( s.active = false; s.paused = true; restoreProjectRootEnv(); + restoreMilestoneLockEnv(); s.pendingVerificationRetry = null; s.verificationRetryCount.clear(); ctx?.ui.setStatus("gsd-auto", "paused"); @@ -1154,6 +1207,7 @@ export async function startAuto( options?: { step?: boolean; interrupted?: InterruptedSessionAssessment; + milestoneLock?: string | null; }, ): Promise { if (s.active) { @@ -1163,6 +1217,12 @@ export async function startAuto( const requestedStepMode = options?.step ?? false; const interruptedAssessment = options?.interrupted ?? null; + if (options?.milestoneLock !== undefined) { + s.sessionMilestoneLock = options.milestoneLock ?? null; + } + if (s.sessionMilestoneLock) { + captureMilestoneLockEnv(s.sessionMilestoneLock); + } // Escape stale worktree cwd from a previous milestone (#608). base = escapeStaleWorktree(base); @@ -1194,6 +1254,7 @@ export async function startAuto( s.originalBasePath = meta.originalBasePath || base; s.stepMode = meta.stepMode ?? requestedStepMode; s.autoStartTime = meta.autoStartTime || Date.now(); + s.sessionMilestoneLock = meta.milestoneLock ?? null; s.paused = true; try { unlinkSync(pausedPath); } catch (e) { logWarning("session", `pause file cleanup failed: ${e instanceof Error ? e.message : String(e)}`, { file: "auto.ts" }); } ctx.ui.notify( @@ -1228,6 +1289,7 @@ export async function startAuto( s.pausedUnitType = meta.unitType ?? null; s.pausedUnitId = meta.unitId ?? null; s.autoStartTime = meta.autoStartTime || Date.now(); + s.sessionMilestoneLock = meta.milestoneLock ?? null; s.paused = true; try { unlinkSync(pausedPath); } catch (e) { logWarning("session", `pause file cleanup failed: ${e instanceof Error ? e.message : String(e)}`, { file: "auto.ts" }); } ctx.ui.notify( @@ -1247,6 +1309,10 @@ export async function startAuto( if (!s.autoStartTime || s.autoStartTime <= 0) s.autoStartTime = Date.now(); } + if (s.sessionMilestoneLock) { + captureMilestoneLockEnv(s.sessionMilestoneLock); + } + if (!s.paused) { s.stepMode = requestedStepMode; } diff --git a/src/resources/extensions/gsd/auto/session.ts b/src/resources/extensions/gsd/auto/session.ts index 4f8fc82e0..ff69877b9 100644 --- a/src/resources/extensions/gsd/auto/session.ts +++ b/src/resources/extensions/gsd/auto/session.ts @@ -87,6 +87,10 @@ export class AutoSession { previousProjectRootEnv: string | null = null; hadProjectRootEnv = false; projectRootEnvCaptured = false; + previousMilestoneLockEnv: string | null = null; + hadMilestoneLockEnv = false; + milestoneLockEnvCaptured = false; + sessionMilestoneLock: string | null = null; gitService: GitServiceImpl | null = null; // ── Dispatch counters ──────────────────────────────────────────────────── @@ -200,6 +204,10 @@ export class AutoSession { this.previousProjectRootEnv = null; this.hadProjectRootEnv = false; this.projectRootEnvCaptured = false; + this.previousMilestoneLockEnv = null; + this.hadMilestoneLockEnv = false; + this.milestoneLockEnvCaptured = false; + this.sessionMilestoneLock = null; this.gitService = null; // Dispatch diff --git a/src/resources/extensions/gsd/commands/handlers/auto.ts b/src/resources/extensions/gsd/commands/handlers/auto.ts index 923191cfb..283ff77ed 100644 --- a/src/resources/extensions/gsd/commands/handlers/auto.ts +++ b/src/resources/extensions/gsd/commands/handlers/auto.ts @@ -4,7 +4,7 @@ import { existsSync, readFileSync } from "node:fs"; import { resolve } from "node:path"; import { enableDebug } from "../../debug-logger.js"; -import { getAutoDashboardData, isAutoActive, isAutoPaused, pauseAuto, startAuto, stopAuto, stopAutoRemote } from "../../auto.js"; +import { getAutoDashboardData, isAutoActive, isAutoPaused, pauseAuto, startAutoDetached, stopAuto, stopAutoRemote } from "../../auto.js"; import { handleRate } from "../../commands-rate.js"; import { guardRemoteSession, projectRoot } from "../context.js"; import { findMilestoneIds } from "../../milestone-id-utils.js"; @@ -42,26 +42,6 @@ export function parseMilestoneTarget(input: string): { milestoneId: string | nul return { milestoneId: match[1], rest }; } -/** - * Set GSD_MILESTONE_LOCK to target a specific milestone, then run `fn`. - * Clears the env var when `fn` resolves or rejects, so the lock does not - * leak into subsequent commands in the same process. - */ -async function withMilestoneLock(milestoneId: string, fn: () => Promise): Promise { - const previous = process.env.GSD_MILESTONE_LOCK; - process.env.GSD_MILESTONE_LOCK = milestoneId; - try { - await fn(); - } finally { - // Restore previous value (undefined → delete, else restore). - if (previous === undefined) { - delete process.env.GSD_MILESTONE_LOCK; - } else { - process.env.GSD_MILESTONE_LOCK = previous; - } - } -} - export async function handleAutoCommand(trimmed: string, ctx: ExtensionCommandContext, pi: ExtensionAPI): Promise { if (trimmed === "next" || trimmed.startsWith("next ")) { if (trimmed.includes("--dry-run")) { @@ -84,13 +64,10 @@ export async function handleAutoCommand(trimmed: string, ctx: ExtensionCommandCo } } - if (milestoneId) { - await withMilestoneLock(milestoneId, () => - startAuto(ctx, pi, projectRoot(), verboseMode, { step: true }), - ); - } else { - await startAuto(ctx, pi, projectRoot(), verboseMode, { step: true }); - } + startAutoDetached(ctx, pi, projectRoot(), verboseMode, { + step: true, + milestoneLock: milestoneId, + }); return true; } @@ -128,13 +105,11 @@ export async function handleAutoCommand(trimmed: string, ctx: ExtensionCommandCo const { showHeadlessMilestoneCreation } = await import("../../guided-flow.js"); await showHeadlessMilestoneCreation(ctx, pi, projectRoot(), seedContent); } else if (milestoneId) { - // Target a specific milestone — use GSD_MILESTONE_LOCK so state - // derivation only sees this milestone (#2521). - await withMilestoneLock(milestoneId, () => - startAuto(ctx, pi, projectRoot(), verboseMode), - ); + startAutoDetached(ctx, pi, projectRoot(), verboseMode, { + milestoneLock: milestoneId, + }); } else { - await startAuto(ctx, pi, projectRoot(), verboseMode); + startAutoDetached(ctx, pi, projectRoot(), verboseMode); } return true; } @@ -175,10 +150,9 @@ export async function handleAutoCommand(trimmed: string, ctx: ExtensionCommandCo if (trimmed === "") { if (!(await guardRemoteSession(ctx, pi))) return true; - await startAuto(ctx, pi, projectRoot(), false, { step: true }); + startAutoDetached(ctx, pi, projectRoot(), false, { step: true }); return true; } return false; } - diff --git a/src/resources/extensions/gsd/commands/handlers/workflow.ts b/src/resources/extensions/gsd/commands/handlers/workflow.ts index 10282fbcc..85f6276e2 100644 --- a/src/resources/extensions/gsd/commands/handlers/workflow.ts +++ b/src/resources/extensions/gsd/commands/handlers/workflow.ts @@ -18,7 +18,7 @@ import { createRun, listRuns } from "../../run-manager.js"; import { setActiveEngineId, setActiveRunDir, - startAuto, + startAutoDetached, pauseAuto, isAutoActive, getActiveEngineId, @@ -77,7 +77,7 @@ async function handleCustomWorkflow( setActiveEngineId("custom"); setActiveRunDir(runDir); ctx.ui.notify(`Created workflow run: ${defName}\nRun dir: ${runDir}`, "info"); - await startAuto(ctx, pi, base, false); + startAutoDetached(ctx, pi, base, false); } catch (err) { // Clean up engine state so a failed workflow run doesn't pollute the next /gsd auto setActiveEngineId(null); @@ -157,13 +157,8 @@ async function handleCustomWorkflow( ctx.ui.notify("No custom workflow to resume. Use /gsd auto for dev workflow.", "warning"); return true; } - try { - await startAuto(ctx, pi, projectRoot(), false); - ctx.ui.notify("Custom workflow resumed.", "info"); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - ctx.ui.notify(`Failed to resume workflow: ${msg}`, "error"); - } + startAutoDetached(ctx, pi, projectRoot(), false); + ctx.ui.notify("Custom workflow resumed.", "info"); return true; } @@ -278,4 +273,3 @@ export function getNextMilestoneId(basePath: string): string { const uniqueIds = !!loadEffectiveGSDPreferences()?.preferences?.unique_milestone_ids; return nextMilestoneId(milestoneIds, uniqueIds); } - diff --git a/src/resources/extensions/gsd/guided-flow.ts b/src/resources/extensions/gsd/guided-flow.ts index 53f76915f..8892564a6 100644 --- a/src/resources/extensions/gsd/guided-flow.ts +++ b/src/resources/extensions/gsd/guided-flow.ts @@ -15,7 +15,7 @@ import { loadPrompt, inlineTemplate } from "./prompt-loader.js"; import { buildSkillActivationBlock } from "./auto-prompts.js"; import { deriveState } from "./state.js"; import { invalidateAllCaches } from "./cache.js"; -import { startAuto } from "./auto.js"; +import { startAutoDetached } from "./auto.js"; import { clearLock } from "./crash-recovery.js"; import { assessInterruptedSession, @@ -67,7 +67,6 @@ export { showQueue, handleQueueReorder, showQueueAdd, buildExistingMilestonesContext, } from "./guided-flow-queue.js"; -import { getErrorMessage } from "./error-utils.js"; import { logWarning } from "./workflow-logger.js"; // ─── ID Generation with Reservation ───────────────────────────────────────── @@ -244,11 +243,7 @@ export function checkAutoStartAfterDiscuss(): boolean { pendingAutoStartMap.delete(basePath); ctx.ui.notify(`Milestone ${milestoneId} ready.`, "info"); - startAuto(ctx, pi, basePath, false, { step }).catch((err) => { - ctx.ui.notify(`Auto-start failed: ${getErrorMessage(err)}`, "error"); - logWarning("guided", `auto start error: ${getErrorMessage(err)}`); - debugLog("auto-start-failed", { error: getErrorMessage(err) }); - }); + startAutoDetached(ctx, pi, basePath, false, { step }); return true; } @@ -1305,7 +1300,7 @@ export async function showSmartEntry( ], }); if (resume === "resume") { - await startAuto(ctx, pi, basePath, false, { + startAutoDetached(ctx, pi, basePath, false, { interrupted, step: interrupted.pausedSession?.stepMode ?? false, }); @@ -1647,7 +1642,7 @@ export async function showSmartEntry( }); if (choice === "auto") { - await startAuto(ctx, pi, basePath, false); + startAutoDetached(ctx, pi, basePath, false); } else if (choice === "status") { const { fireStatusViaCommand } = await import("./commands.js"); await fireStatusViaCommand(ctx); @@ -1859,7 +1854,7 @@ export async function showSmartEntry( }); if (choice === "auto") { - await startAuto(ctx, pi, basePath, false); + startAutoDetached(ctx, pi, basePath, false); return; } diff --git a/src/resources/extensions/gsd/interrupted-session.ts b/src/resources/extensions/gsd/interrupted-session.ts index 8c6274a05..b0ca579d3 100644 --- a/src/resources/extensions/gsd/interrupted-session.ts +++ b/src/resources/extensions/gsd/interrupted-session.ts @@ -34,6 +34,7 @@ export interface PausedSessionMetadata { activeEngineId?: string; activeRunDir?: string | null; autoStartTime?: number; + milestoneLock?: string | null; } export interface InterruptedSessionAssessment { diff --git a/src/resources/extensions/gsd/tests/start-auto-detached.test.ts b/src/resources/extensions/gsd/tests/start-auto-detached.test.ts new file mode 100644 index 000000000..6726b2616 --- /dev/null +++ b/src/resources/extensions/gsd/tests/start-auto-detached.test.ts @@ -0,0 +1,90 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; + +const gsdDir = resolve(import.meta.dirname, ".."); + +function readGsdFile(relativePath: string): string { + return readFileSync(resolve(gsdDir, relativePath), "utf-8"); +} + +test("command entrypoints use startAutoDetached instead of awaiting startAuto (#3733)", () => { + const autoHandlerSrc = readGsdFile("commands/handlers/auto.ts"); + const workflowHandlerSrc = readGsdFile("commands/handlers/workflow.ts"); + const guidedFlowSrc = readGsdFile("guided-flow.ts"); + + assert.ok( + !autoHandlerSrc.includes("await startAuto("), + "auto command handler should not await startAuto from the active agent turn", + ); + assert.ok( + !workflowHandlerSrc.includes("await startAuto("), + "workflow command handler should not await startAuto from the active agent turn", + ); + assert.ok( + !guidedFlowSrc.includes("await startAuto("), + "guided flow should not await startAuto from the active agent turn", + ); + + assert.ok( + autoHandlerSrc.includes("startAutoDetached("), + "auto command handler should launch auto-mode through startAutoDetached", + ); + assert.ok( + workflowHandlerSrc.includes("startAutoDetached("), + "workflow handler should launch auto-mode through startAutoDetached", + ); + assert.ok( + guidedFlowSrc.includes("startAutoDetached("), + "guided flow should launch auto-mode through startAutoDetached", + ); +}); + +test("startAutoDetached reports failures asynchronously (#3733)", () => { + const autoSrc = readGsdFile("auto.ts"); + + assert.ok( + autoSrc.includes("export function startAutoDetached"), + "auto.ts should export startAutoDetached", + ); + assert.ok( + autoSrc.includes("void startAuto(ctx, pi, base, verboseMode, options).catch"), + "startAutoDetached should launch startAuto without awaiting it", + ); + assert.ok( + autoSrc.includes("ctx.ui.notify(`Auto-start failed: ${message}`, \"error\")"), + "startAutoDetached should surface async startup failures to the user", + ); +}); + +test("detached auto-start preserves milestone lock across pause/stop cleanup (#3733)", () => { + const autoSrc = readGsdFile("auto.ts"); + const sessionSrc = readGsdFile("auto/session.ts"); + + assert.ok( + autoSrc.includes("milestoneLock?: string | null"), + "startAuto/startAutoDetached options should carry an explicit milestone lock", + ); + assert.ok( + autoSrc.includes("s.sessionMilestoneLock = options.milestoneLock ?? null;"), + "startAuto should capture the requested milestone lock before async work begins", + ); + assert.ok( + autoSrc.includes("milestoneLock: s.sessionMilestoneLock ?? undefined"), + "pause metadata should persist the detached milestone lock for resume", + ); + assert.ok( + autoSrc.includes("s.sessionMilestoneLock = meta.milestoneLock ?? null;"), + "resume should restore the persisted milestone lock", + ); + assert.ok( + autoSrc.includes("restoreMilestoneLockEnv();"), + "auto cleanup should restore the previous process milestone-lock env", + ); + + assert.ok( + sessionSrc.includes("sessionMilestoneLock: string | null = null;"), + "AutoSession should track the detached milestone lock explicitly", + ); +});