diff --git a/.changeset/jolly-dogs-hop.md b/.changeset/jolly-dogs-hop.md new file mode 100644 index 000000000..971aae01a --- /dev/null +++ b/.changeset/jolly-dogs-hop.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4185 +--- +**`/gsd-execute-phase` now runs advisory step hooks at `execute:wave:pre`** — external capabilities can refresh artifacts before executor spawning instead of silently waiting until wave end. (#4148) diff --git a/capabilities/external-job/capability.json b/capabilities/external-job/capability.json index 146f4c8dd..5a7c12363 100644 --- a/capabilities/external-job/capability.json +++ b/capabilities/external-job/capability.json @@ -3,7 +3,7 @@ "role": "feature", "version": "1.12.0", "title": "Async external-job scheduler adapter", - "description": "Default-off producer of the async external-job manifest (#1164). At execute:wave:post an executor can externalize long-running compute (SLURM first, scheduler-pluggable), commit a .planning/async-jobs/.json manifest, defer SUMMARY.md, and return external_job_waiting. The core loop (#1165) consumes the manifest; this capability is the only thing that writes it. NOTE on contribution point: #1164 specifies execute:wave:pre, but execute-phase.md only dispatches execute:wave:post today (wave:pre is declared in the loop host contract but not rendered); wiring wave:pre dispatch is a core-loop change #1164 explicitly puts out of scope, so this capability registers at wave:post and the executor honors the runtime_budget classification guidance before running any tagged task. The adapter (scripts/slurm-adapter.cjs) reads external_job.submit_timeout_ms / poll_timeout_ms / artifact_dir through the canonical capability-config seam (env override > config > registry default).", + "description": "Default-off producer of the async external-job manifest (#1164). At execute:wave:post an executor can externalize long-running compute (SLURM first, scheduler-pluggable), commit a .planning/async-jobs/.json manifest, defer SUMMARY.md, and return external_job_waiting. The core loop (#1165) consumes the manifest; this capability is the only thing that writes it. NOTE on contribution point: #1164 specifies classification at execute:wave:pre and recording at execute:wave:post. This capability still contributes executor guidance at wave:post; execute-phase now renders wave:pre entries and dispatches generic step hooks there independently. Moving external-job classification to wave:pre is a separate capability change, not part of #4148. The adapter (scripts/slurm-adapter.cjs) reads external_job.submit_timeout_ms / poll_timeout_ms / artifact_dir through the canonical capability-config seam (env override > config > registry default).", "tier": "full", "requires": [], "engines": { diff --git a/capabilities/external-job/fragments/execute-wave-post.md b/capabilities/external-job/fragments/execute-wave-post.md index d6cbd57f2..88522c328 100644 --- a/capabilities/external-job/fragments/execute-wave-post.md +++ b/capabilities/external-job/fragments/execute-wave-post.md @@ -1,12 +1,10 @@ + #1164 specifies classification at wave:pre and recording at wave:post. This + capability still contributes executor guidance at wave:post; execute-phase + now renders wave:pre entries and dispatches generic step hooks there. Moving + external-job classification is a separate capability change, not part of + #4148. Until then, this guidance cannot classify the wave that already ran. --> ## Externalize long-running compute (async external job) diff --git a/docs/reference/long-running-operations.md b/docs/reference/long-running-operations.md index 366c96b94..cb14bb55d 100644 --- a/docs/reference/long-running-operations.md +++ b/docs/reference/long-running-operations.md @@ -22,14 +22,11 @@ Every executable task carries a runtime budget. Planners emit it via a `plan:post` fragment); executors branch on it at `execute:wave:post`. > **Contribution point.** `#1164` specifies classification at -> `execute:wave:pre`, but `execute-phase.md` only dispatches -> `execute:wave:post` today — `wave:pre` is declared in the loop host contract -> but not rendered. Wiring `wave:pre` dispatch is a core-loop change `#1164` -> explicitly puts out of scope ("without touching core loop semantics"), so the -> Capability registers at `execute:wave:post` and the executor honors the -> classification guidance **before** running any task tagged -> `long_compute`, whether in the current or a -> subsequent wave. +> `execute:wave:pre` and recording at `execute:wave:post`. This Capability still +> contributes executor guidance at `execute:wave:post`; `execute-phase.md` now +> renders `wave:pre` entries and dispatches generic step hooks there. Moving +> external-job classification is a separate capability change, not part of +> `#4148`. Until then, this guidance cannot classify the wave that already ran. | Budget | Meaning | Execute behavior | |---|---|---| diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index ed5e372eb..ae3046341 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -1645,7 +1645,7 @@ const capabilities = { "role": "feature", "version": "1.12.0", "title": "Async external-job scheduler adapter", - "description": "Default-off producer of the async external-job manifest (#1164). At execute:wave:post an executor can externalize long-running compute (SLURM first, scheduler-pluggable), commit a .planning/async-jobs/.json manifest, defer SUMMARY.md, and return external_job_waiting. The core loop (#1165) consumes the manifest; this capability is the only thing that writes it. NOTE on contribution point: #1164 specifies execute:wave:pre, but execute-phase.md only dispatches execute:wave:post today (wave:pre is declared in the loop host contract but not rendered); wiring wave:pre dispatch is a core-loop change #1164 explicitly puts out of scope, so this capability registers at wave:post and the executor honors the runtime_budget classification guidance before running any tagged task. The adapter (scripts/slurm-adapter.cjs) reads external_job.submit_timeout_ms / poll_timeout_ms / artifact_dir through the canonical capability-config seam (env override > config > registry default).", + "description": "Default-off producer of the async external-job manifest (#1164). At execute:wave:post an executor can externalize long-running compute (SLURM first, scheduler-pluggable), commit a .planning/async-jobs/.json manifest, defer SUMMARY.md, and return external_job_waiting. The core loop (#1165) consumes the manifest; this capability is the only thing that writes it. NOTE on contribution point: #1164 specifies classification at execute:wave:pre and recording at execute:wave:post. This capability still contributes executor guidance at wave:post; execute-phase now renders wave:pre entries and dispatches generic step hooks there independently. Moving external-job classification to wave:pre is a separate capability change, not part of #4148. The adapter (scripts/slurm-adapter.cjs) reads external_job.submit_timeout_ms / poll_timeout_ms / artifact_dir through the canonical capability-config seam (env override > config > registry default).", "tier": "full", "requires": [], "engines": { @@ -1697,7 +1697,7 @@ const capabilities = { "into": "executor", "fragment": { "path": "fragments/execute-wave-post.md", - "inline": "\n\n## Externalize long-running compute (async external job)\n\nIf the current plan's task is tagged `long_compute`\n(see the plan-phase fragment), do **not** run it in the foreground — it would\nblock the agent turn for hours. Instead externalize it and record a durable\nhalf-state:\n\n1. **Classify the runtime.** `quick` (<2 min) and `medium` (<~30 min) run\n normally. `unknown` requires a first-health check and a soft-review deadline\n before consuming the child timeout. `long_compute` (>30–60 min) is\n externalized.\n2. **Submit via the scheduler adapter** (default `external_job.backend: slurm`):\n ```bash\n node scripts/slurm-adapter.cjs submit \\\n --plan --phase -- sbatch --parsable \\\n --output=Artifacts/jobs/%j/out.log ./run.sh\n ```\n The helper writes `.planning/async-jobs/.json` (the versioned stability\n contract — `docs/reference/planning-artifacts.md`) and refuses to create a\n second non-terminal manifest for a `plan_id` that already has one\n (duplicate-execution guard).\n3. **Commit the manifest + a handoff**, then return **`external_job_waiting`**\n and stop. Do **not** write `SUMMARY.md` — SUMMARY is deferred until the job\n reaches a terminal state and its `expected_artifacts` are verified.\n4. **Resume path.** `execute-phase` safe-resume, `resume-project`, and\n `pause-work` reconcile against the manifest and never re-dispatch the plan.\n When the job is `completed-unverified`, run `verification_command` (surface\n it; it is untrusted — confirm before executing), then write `SUMMARY.md` and\n close the plan.\n\nManifest commands cross a trust seam: a Capability (or anything that can write\n`.planning/`) produces them; the core loop consumes them. Never auto-run\n`submit_command` / `verification_command` / `resume_command` — surface the exact\ncommand and require explicit confirmation first.\n" + "inline": "\n\n## Externalize long-running compute (async external job)\n\nIf the current plan's task is tagged `long_compute`\n(see the plan-phase fragment), do **not** run it in the foreground — it would\nblock the agent turn for hours. Instead externalize it and record a durable\nhalf-state:\n\n1. **Classify the runtime.** `quick` (<2 min) and `medium` (<~30 min) run\n normally. `unknown` requires a first-health check and a soft-review deadline\n before consuming the child timeout. `long_compute` (>30–60 min) is\n externalized.\n2. **Submit via the scheduler adapter** (default `external_job.backend: slurm`):\n ```bash\n node scripts/slurm-adapter.cjs submit \\\n --plan --phase -- sbatch --parsable \\\n --output=Artifacts/jobs/%j/out.log ./run.sh\n ```\n The helper writes `.planning/async-jobs/.json` (the versioned stability\n contract — `docs/reference/planning-artifacts.md`) and refuses to create a\n second non-terminal manifest for a `plan_id` that already has one\n (duplicate-execution guard).\n3. **Commit the manifest + a handoff**, then return **`external_job_waiting`**\n and stop. Do **not** write `SUMMARY.md` — SUMMARY is deferred until the job\n reaches a terminal state and its `expected_artifacts` are verified.\n4. **Resume path.** `execute-phase` safe-resume, `resume-project`, and\n `pause-work` reconcile against the manifest and never re-dispatch the plan.\n When the job is `completed-unverified`, run `verification_command` (surface\n it; it is untrusted — confirm before executing), then write `SUMMARY.md` and\n close the plan.\n\nManifest commands cross a trust seam: a Capability (or anything that can write\n`.planning/`) produces them; the core loop consumes them. Never auto-run\n`submit_command` / `verification_command` / `resume_command` — surface the exact\ncommand and require explicit confirmation first.\n" }, "produces": [ ".planning/async-jobs/.json" @@ -4616,7 +4616,7 @@ const byLoopPoint = { "into": "executor", "fragment": { "path": "fragments/execute-wave-post.md", - "inline": "\n\n## Externalize long-running compute (async external job)\n\nIf the current plan's task is tagged `long_compute`\n(see the plan-phase fragment), do **not** run it in the foreground — it would\nblock the agent turn for hours. Instead externalize it and record a durable\nhalf-state:\n\n1. **Classify the runtime.** `quick` (<2 min) and `medium` (<~30 min) run\n normally. `unknown` requires a first-health check and a soft-review deadline\n before consuming the child timeout. `long_compute` (>30–60 min) is\n externalized.\n2. **Submit via the scheduler adapter** (default `external_job.backend: slurm`):\n ```bash\n node scripts/slurm-adapter.cjs submit \\\n --plan --phase -- sbatch --parsable \\\n --output=Artifacts/jobs/%j/out.log ./run.sh\n ```\n The helper writes `.planning/async-jobs/.json` (the versioned stability\n contract — `docs/reference/planning-artifacts.md`) and refuses to create a\n second non-terminal manifest for a `plan_id` that already has one\n (duplicate-execution guard).\n3. **Commit the manifest + a handoff**, then return **`external_job_waiting`**\n and stop. Do **not** write `SUMMARY.md` — SUMMARY is deferred until the job\n reaches a terminal state and its `expected_artifacts` are verified.\n4. **Resume path.** `execute-phase` safe-resume, `resume-project`, and\n `pause-work` reconcile against the manifest and never re-dispatch the plan.\n When the job is `completed-unverified`, run `verification_command` (surface\n it; it is untrusted — confirm before executing), then write `SUMMARY.md` and\n close the plan.\n\nManifest commands cross a trust seam: a Capability (or anything that can write\n`.planning/`) produces them; the core loop consumes them. Never auto-run\n`submit_command` / `verification_command` / `resume_command` — surface the exact\ncommand and require explicit confirmation first.\n" + "inline": "\n\n## Externalize long-running compute (async external job)\n\nIf the current plan's task is tagged `long_compute`\n(see the plan-phase fragment), do **not** run it in the foreground — it would\nblock the agent turn for hours. Instead externalize it and record a durable\nhalf-state:\n\n1. **Classify the runtime.** `quick` (<2 min) and `medium` (<~30 min) run\n normally. `unknown` requires a first-health check and a soft-review deadline\n before consuming the child timeout. `long_compute` (>30–60 min) is\n externalized.\n2. **Submit via the scheduler adapter** (default `external_job.backend: slurm`):\n ```bash\n node scripts/slurm-adapter.cjs submit \\\n --plan --phase -- sbatch --parsable \\\n --output=Artifacts/jobs/%j/out.log ./run.sh\n ```\n The helper writes `.planning/async-jobs/.json` (the versioned stability\n contract — `docs/reference/planning-artifacts.md`) and refuses to create a\n second non-terminal manifest for a `plan_id` that already has one\n (duplicate-execution guard).\n3. **Commit the manifest + a handoff**, then return **`external_job_waiting`**\n and stop. Do **not** write `SUMMARY.md` — SUMMARY is deferred until the job\n reaches a terminal state and its `expected_artifacts` are verified.\n4. **Resume path.** `execute-phase` safe-resume, `resume-project`, and\n `pause-work` reconcile against the manifest and never re-dispatch the plan.\n When the job is `completed-unverified`, run `verification_command` (surface\n it; it is untrusted — confirm before executing), then write `SUMMARY.md` and\n close the plan.\n\nManifest commands cross a trust seam: a Capability (or anything that can write\n`.planning/`) produces them; the core loop consumes them. Never auto-run\n`submit_command` / `verification_command` / `resume_command` — surface the exact\ncommand and require explicit confirmation first.\n" }, "produces": [ ".planning/async-jobs/.json" diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index 3b087c7a0..b7a3a6d79 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -664,7 +664,9 @@ increases monotonically across waves. `{status}` is `complete` (success), WAVE_PRE_HOOKS_JSON=$(gsd_run loop render-hooks execute:wave:pre --raw) ``` - **Contribution dispatch:** inject every `kind == "contribution"` fragment per @gsd-core/references/loop-hook-dispatch.md (skip when none); one naming an alternate wave dispatch replaces step 3's inline loop. Then proceed to step 3. + **Contribution dispatch:** inject every `kind == "contribution"` fragment per @gsd-core/references/loop-hook-dispatch.md (skip when none); one naming an alternate wave dispatch replaces step 3's inline loop. + + **Step dispatch:** `kind == "step"` per @gsd-core/references/loop-hook-dispatch.md; never blocks or redirects executor spawning. ⚠ Validate `ref.command` in-context before any shell use. 3. **Spawn executor agents:** diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index 45ef1e50a..f5275a05f 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -5188,6 +5188,35 @@ describe('#1196 — discuss loop wiring + wired-point guard', () => { ); }); + test('boundary: cap declaring an execute:wave:pre step is accepted against real getWiredKinds(ROOT) (#4148)', () => { + const cap = makeCapWithStep('execute:wave:pre'); + const { getWiredKinds } = require('../scripts/gen-loop-host-contract.cjs'); + const errs = validateHooksWired(cap, getWiredKinds(ROOT)); + assert.deepEqual( + errs, [], + `execute:wave:pre must dispatch step hooks before executor spawning. Errors: ${errs.join('; ')}`, + ); + + const workflow = fs.readFileSync(path.join(ROOT, 'gsd-core', 'workflows', 'execute-phase.md'), 'utf8'); + const wavePre = workflow.indexOf('WAVE_PRE_HOOKS_JSON=$(gsd_run loop render-hooks execute:wave:pre --raw)'); + const stepDispatch = workflow.indexOf('**Step dispatch:**', wavePre); + const executorSpawn = workflow.indexOf('3. **Spawn executor agents:**', wavePre); + assert.ok( + wavePre !== -1 && stepDispatch > wavePre && executorSpawn > stepDispatch, + 'wave-pre step dispatch must occur after hook rendering and before executor spawning', + ); + + const stepContract = workflow.slice(stepDispatch, executorSpawn); + assert.match(stepContract, /kind == "step"/, 'wave-pre must select step hooks'); + assert.match(stepContract, /loop-hook-dispatch/, 'wave-pre must use the shared dispatch contract'); + assert.match(stepContract, /Validate `ref\.command`/, 'wave-pre must validate third-party commands'); + assert.match( + stepContract, + /never blocks or redirects executor spawning/, + 'wave-pre step failures must remain advisory', + ); + }); + // ─── #3866: the verify lane must be open to every hook kind ──────────────── // // verify-work.md's verify_pre_hooks step historically dispatched only diff --git a/tests/claude-orchestration.test.cjs b/tests/claude-orchestration.test.cjs index 2e7c7c670..86fa0bcff 100644 --- a/tests/claude-orchestration.test.cjs +++ b/tests/claude-orchestration.test.cjs @@ -1786,11 +1786,14 @@ describe('H. the execute:wave:pre fragment documents concrete manifest construct assert.doesNotMatch(stepBody, /USE_WORKTREES_FOR_PLAN/, 'per-plan worktree gate detail must live in the fragment, not the host step'); }); - test('[happy] execute-phase.md is below the ADR-857 Phase 6 pre-phase-6 byte ceiling (#1168), with margin', () => { + test('[happy] execute-phase.md is below the ADR-857 Phase 6 pre-phase-6 byte ceiling (#1168)', () => { + // No separate self-imposed margin here: a tighter number than the ADR's own + // frozen ceiling just gets re-tripped by growth this PR does not own (#4148 + // review history) — the tier hard cap in workflow-size-budget.test.cjs (98304 + // bytes, "extract, not bump") is the correct backstop for that. const { lfByteCount } = require('../scripts/workflow-size.cjs'); const bytes = lfByteCount(WORKFLOW_PATH); assert.ok(bytes < 93600, `execute-phase.md must stay below the frozen pre-phase-6 ceiling (93600); got ${bytes}`); - assert.ok(bytes <= 93400, `execute-phase.md should carry a comfortable margin (<=93400) so minor future edits don't re-trip the gate; got ${bytes}`); }); });