diff --git a/.changeset/sturdy-rams-forage.md b/.changeset/sturdy-rams-forage.md new file mode 100644 index 000000000..45bf68bb9 --- /dev/null +++ b/.changeset/sturdy-rams-forage.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3329 +--- +Add executor stall detection and safe-resume contracts so interrupted execute-phase runs surface partial-plan drift before dispatching duplicate executor work. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 4a82e93de..24feb2dbf 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -216,6 +216,8 @@ All workflow toggles follow the **absent = enabled** pattern. If a key is missin | `workflow.auto_prune_state` | boolean | `false` | When `true`, automatically prune stale entries from STATE.md at phase boundaries instead of prompting | | `workflow.pattern_mapper` | boolean | `true` | Run the `gsd-pattern-mapper` agent between research and planning to map new files to existing codebase analogs | | `workflow.subagent_timeout` | number | `600` | Timeout in seconds for individual subagent invocations. Increase for long-running research or execution phases | +| `executor.stall_detect_interval_minutes` | number | `5` | Minutes between executor stall checks while an executor agent is active. The execute-phase orchestrator uses this cadence to inspect recent commits and avoid waiting forever on a silent agent. | +| `executor.stall_threshold_minutes` | number | `10` | Minutes without executor completion or expected-branch commit activity before execute-phase offers recovery choices for a possible stalled executor. | | `workflow.inline_plan_threshold` | number | `3` | Maximum number of tasks in a phase before the planner generates a separate PLAN.md file instead of inlining tasks in the prompt | | `workflow.drift_threshold` | number | `3` | Minimum number of new structural elements (new directories, barrel exports, migrations, route modules) introduced during a phase before the post-execute codebase-drift gate takes action. See [#2003](https://github.com/gsd-build/get-shit-done/issues/2003). Added in v1.39 | | `workflow.drift_action` | string | `warn` | What to do when `workflow.drift_threshold` is exceeded after `/gsd-execute-phase`. `warn` prints a message suggesting `/gsd-map-codebase --paths …`; `auto-remap` spawns `gsd-codebase-mapper` scoped to the affected paths. Added in v1.39 | diff --git a/get-shit-done/bin/lib/config-schema.cjs b/get-shit-done/bin/lib/config-schema.cjs index 570de5bd1..6cc3a6b53 100644 --- a/get-shit-done/bin/lib/config-schema.cjs +++ b/get-shit-done/bin/lib/config-schema.cjs @@ -47,6 +47,8 @@ const VALID_CONFIG_KEYS = new Set([ 'review.ollama_host', 'review.lm_studio_host', 'review.llama_cpp_host', 'workflow.cross_ai_execution', 'workflow.cross_ai_command', 'workflow.cross_ai_timeout', 'workflow.subagent_timeout', + 'executor.stall_detect_interval_minutes', + 'executor.stall_threshold_minutes', 'workflow.inline_plan_threshold', 'hooks.context_warnings', 'hooks.workflow_guard', diff --git a/get-shit-done/bin/lib/config.cjs b/get-shit-done/bin/lib/config.cjs index 13cf2be3e..cf99e820c 100644 --- a/get-shit-done/bin/lib/config.cjs +++ b/get-shit-done/bin/lib/config.cjs @@ -384,6 +384,8 @@ function cmdConfigSet(cwd, keyPath, value, raw) { */ const SCHEMA_DEFAULTS = { 'context_window': 200000, + 'executor.stall_detect_interval_minutes': 5, + 'executor.stall_threshold_minutes': 10, }; function cmdConfigGet(cwd, keyPath, raw, defaultValue) { diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 4a1be0eac..cb86b4e08 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -81,6 +81,8 @@ Read worktree config: ```bash USE_WORKTREES=$(gsd-sdk query config-get workflow.use_worktrees 2>/dev/null || echo "true") +EXECUTOR_STALL_INTERVAL_MINUTES=$(gsd-sdk query config-get executor.stall_detect_interval_minutes 2>/dev/null || echo "5") +EXECUTOR_STALL_THRESHOLD_MINUTES=$(gsd-sdk query config-get executor.stall_threshold_minutes 2>/dev/null || echo "10") ``` If the project uses git submodules, worktree isolation is unsafe **only when a plan touches a submodule path** — the executor commit protocol cannot correctly handle submodule commits inside isolated worktrees. The previous behavior unconditionally disabled worktree isolation whenever `.gitmodules` existed, which penalised every plan in a submodule project even when the plan was nowhere near a submodule. Compute submodule paths once and intersect them per-plan with the plan's declared `files_modified` frontmatter. @@ -146,6 +148,22 @@ MVP_MODE=$(gsd-sdk query phase.mvp-mode "${PHASE_NUMBER}" $MVP_FLAG_ARG --pick a TDD_MODE=$(gsd-sdk query config-get workflow.tdd_mode 2>/dev/null || echo "false") ``` + +Before trusting `STATE.md` or dispatching any executor, derive `CURRENT_PLAN_ID` +from the active incomplete plan in `INIT`, then search recent history: +```bash +CURRENT_PLAN_ID="{phase_number}-{plan_padded}" +SUMMARY_PATH="{phase_dir}/{plan_padded}-SUMMARY.md" +PLAN_COMMITS=$(git log --oneline --grep="${CURRENT_PLAN_ID}" -30) +``` +If production commits exist and `SUMMARY.md is missing`, stop before spawning a +new executor; continuing risks duplicate work and stale `STATE.md`/ROADMAP progress. +Offer these recovery options: +- `close out manually` — inspect commits, write SUMMARY.md, then update STATE/ROADMAP. +- `re-execute from scratch` — revert or supersede partial commits before dispatch. +- `mark-and-skip` — record the anomaly and move on only with explicit confirmation. + + **MVP+TDD gate.** Task-scoped enforcement runs inside plan execution (immediately before each implementation step), where `TASK_FILE`, `PLAN_ID`, and `TASK_ID` are defined. Keep the same predicate and RED-commit contract: ```bash if [ "$MVP_MODE" = "true" ] && [ "$TDD_MODE" = "true" ]; then @@ -494,6 +512,8 @@ increases monotonically across waves. `{status}` is `complete` (success), Before spawning, capture the current HEAD: ```bash EXPECTED_BASE=$(git rev-parse HEAD) + DISPATCH_TS=$(date -u +"%Y-%m-%dT%H:%M:%SZ") + EXPECTED_BRANCH=$(git rev-parse --abbrev-ref HEAD) ``` **Sequential dispatch for parallel execution (waves with 2+ agents):** @@ -666,6 +686,7 @@ increases monotonically across waves. `{status}` is `complete` (success), # For each plan in this wave, check if the executor finished: SUMMARY_EXISTS=$(test -f "{phase_dir}/{plan_number}-{plan_padded}-SUMMARY.md" && echo "true" || echo "false") COMMITS_FOUND=$(git log --oneline --all --grep="{phase_number}-{plan_padded}" --since="1 hour ago" | head -1) + COMMITS_SINCE_DISPATCH=$(git log "${EXPECTED_BRANCH}" --since="${DISPATCH_TS}" --oneline | head -1) ``` **If SUMMARY.md exists AND commits are found:** The agent completed successfully — @@ -676,6 +697,13 @@ increases monotonically across waves. `{status}` is `complete` (success), activity. If commits are still appearing, wait longer. If no activity, report the plan as failed and route to the failure handler in step 6. + **Configurable stall surveillance (#3212):** Every `${EXECUTOR_STALL_INTERVAL_MINUTES}` + minutes while waiting, inspect `git log "${EXPECTED_BRANCH}" --since="${DISPATCH_TS}"` + for activity. If no completion signal, no SUMMARY.md, and no expected-branch + commits appear for `${EXECUTOR_STALL_THRESHOLD_MINUTES}` minutes, pause and + ask for one recovery path: `continue waiting`, `kill and retry`, or + `kill and switch to inline execution`. + **This fallback applies automatically to all runtimes.** Claude Code's Agent() normally returns synchronously, but the fallback ensures resilience if it doesn't. diff --git a/get-shit-done/workflows/execute-plan.md b/get-shit-done/workflows/execute-plan.md index ddefce0c5..957d3dd8a 100644 --- a/get-shit-done/workflows/execute-plan.md +++ b/get-shit-done/workflows/execute-plan.md @@ -9,6 +9,16 @@ Read config.json for planning behavior settings. @~/.claude/get-shit-done/references/git-integration.md + +For each executed plan, the only complete close-out order is: +`production-code commit(s) -> SUMMARY commit -> STATE/ROADMAP update`. + +The only legal half-state is mid-production-commits while the executor is still +actively working. Once production commits for a plan exist, returning without a +committed SUMMARY.md is an illegal partial-plan state. The next execute-phase +resume must detect that condition before dispatching another executor. + + Valid GSD subagent types (use exact names — do not fall back to 'general-purpose'): - gsd-executor — Executes plan tasks, commits, creates SUMMARY.md diff --git a/get-shit-done/workflows/forensics.md b/get-shit-done/workflows/forensics.md index 07e1c60c4..bbc250c4a 100644 --- a/get-shit-done/workflows/forensics.md +++ b/get-shit-done/workflows/forensics.md @@ -115,6 +115,18 @@ For each phase that should be complete: - SUMMARY.md missing → phase was not properly closed - VERIFICATION.md missing → quality check was skipped +### Partial-plan Drift Detection + +**Signal:** commits exist but SUMMARY.md is missing for the current or recently +active plan. + +Run the same comparison as the execute-phase safe-resume verifier: identify the +active plan from STATE.md/phase artifacts, search git history for that plan id, +then compare against the expected SUMMARY.md path. If production commits exist +but SUMMARY.md is missing, flag a high-confidence partial-plan drift anomaly. +This usually means an executor was interrupted after implementation commits but +before atomic close-out. + ### Abandoned Work Detection **Signal:** Large gap between last commit and current time, with STATE.md showing mid-execution. diff --git a/sdk/src/query/config-schema.ts b/sdk/src/query/config-schema.ts index 278d444e8..03ccc3911 100644 --- a/sdk/src/query/config-schema.ts +++ b/sdk/src/query/config-schema.ts @@ -49,6 +49,8 @@ export const VALID_CONFIG_KEYS: ReadonlySet = new Set([ 'review.ollama_host', 'review.lm_studio_host', 'review.llama_cpp_host', 'workflow.cross_ai_execution', 'workflow.cross_ai_command', 'workflow.cross_ai_timeout', 'workflow.subagent_timeout', + 'executor.stall_detect_interval_minutes', + 'executor.stall_threshold_minutes', 'workflow.inline_plan_threshold', 'hooks.context_warnings', 'hooks.workflow_guard', diff --git a/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs b/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs new file mode 100644 index 000000000..9811b02f1 --- /dev/null +++ b/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs @@ -0,0 +1,98 @@ +'use strict'; + +// allow-test-rule: source-text-is-product [#3212] +// The bug is in workflow/config contracts consumed by agents at runtime. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const { spawnSync } = require('node:child_process'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const ROOT = path.join(__dirname, '..'); + +function read(relativePath) { + return fs.readFileSync(path.join(ROOT, relativePath), 'utf8'); +} + +function runGsd(args, cwd) { + return spawnSync(process.execPath, [path.join(ROOT, 'get-shit-done/bin/gsd-tools.cjs'), ...args], { + cwd, + encoding: 'utf8', + }); +} + +describe('bug #3212 execute-phase stall detection and safe resume', () => { + test('config schemas register executor stall detector keys', () => { + const cjs = require('../get-shit-done/bin/lib/config-schema.cjs'); + const sdk = read('sdk/src/query/config-schema.ts'); + + for (const key of ['executor.stall_detect_interval_minutes', 'executor.stall_threshold_minutes']) { + assert.ok(cjs.VALID_CONFIG_KEYS.has(key), `CJS VALID_CONFIG_KEYS must include ${key}`); + assert.ok(sdk.includes(`'${key}'`), `SDK VALID_CONFIG_KEYS must include ${key}`); + } + }); + + test('configuration docs describe stall detector defaults', () => { + const docs = read('docs/CONFIGURATION.md'); + + assert.match(docs, /`executor\.stall_detect_interval_minutes`\s*\|\s*number\s*\|\s*`5`/); + assert.match(docs, /`executor\.stall_threshold_minutes`\s*\|\s*number\s*\|\s*`10`/); + }); + + test('config-get returns schema defaults for executor stall detector keys', (t) => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3212-')); + t.after(() => fs.rmSync(tmp, { recursive: true, force: true })); + fs.mkdirSync(path.join(tmp, '.planning')); + fs.writeFileSync(path.join(tmp, '.planning/config.json'), '{}\n'); + + const interval = runGsd(['config-get', 'executor.stall_detect_interval_minutes', '--raw'], tmp); + const threshold = runGsd(['config-get', 'executor.stall_threshold_minutes', '--raw'], tmp); + + assert.equal(interval.status, 0, interval.stderr); + assert.equal(interval.stdout.trim(), '5'); + assert.equal(threshold.status, 0, threshold.stderr); + assert.equal(threshold.stdout.trim(), '10'); + }); + + test('execute-phase verifies partial-plan drift before dispatch', () => { + const workflow = read('get-shit-done/workflows/execute-phase.md'); + + assert.match(workflow, / { + const workflow = read('get-shit-done/workflows/execute-phase.md'); + + assert.match(workflow, /EXECUTOR_STALL_INTERVAL_MINUTES=.*executor\.stall_detect_interval_minutes/); + assert.match(workflow, /EXECUTOR_STALL_THRESHOLD_MINUTES=.*executor\.stall_threshold_minutes/); + assert.match(workflow, /DISPATCH_TS=/, 'execute-phase must record dispatch timestamp'); + assert.match(workflow, /EXPECTED_BRANCH=/, 'execute-phase must record expected branch'); + assert.match(workflow, /git log "\$\{EXPECTED_BRANCH\}" --since="\$\{DISPATCH_TS\}"/, 'stall check must inspect branch commits since dispatch'); + assert.match(workflow, /continue waiting/, 'stall warning must offer continue waiting'); + assert.match(workflow, /kill and retry/, 'stall warning must offer kill and retry'); + assert.match(workflow, /kill and switch to inline execution/, 'stall warning must offer inline fallback'); + }); + + test('execute-plan documents atomic close-out invariant', () => { + const workflow = read('get-shit-done/workflows/execute-plan.md'); + + assert.match(workflow, //, 'execute-plan must contain a formal atomic close-out invariant'); + assert.match(workflow, /production-code commit\(s\) -> SUMMARY commit -> STATE\/ROADMAP update/, 'invariant must name the legal close-out sequence'); + assert.match(workflow, /only legal half-state is mid-production-commits/, 'invariant must define the only legal half-state'); + }); + + test('forensics includes the partial-plan drift detector', () => { + const workflow = read('get-shit-done/workflows/forensics.md'); + + assert.match(workflow, /Partial-plan Drift Detection/); + assert.match(workflow, /commits exist but SUMMARY.md is missing/); + assert.match(workflow, /safe-resume verifier/); + }); +});