diff --git a/.changeset/lively-sloths-forage.md b/.changeset/lively-sloths-forage.md new file mode 100644 index 0000000000..2a74b70c5c --- /dev/null +++ b/.changeset/lively-sloths-forage.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 5192 +--- +**plan-phase's stall watch now waits for the planner and plan-checker to actually return** — it no longer reports them finished at its first check because their own prompt or definition mentions the return markers, and a finished plan revision now continues instead of always ending in the stall menu. The plan-checker now declares `Write` for that one receipt file, so on Codex it runs `workspace-write` instead of `read-only`, and on Claude Code it is denied only `Edit` and `MultiEdit`; on OpenCode, Kilo and Copilot, which grant Write and Edit as one permission, the installer drops that `Write`, so the checker stays read-only there and writes no receipt. No receipt is created when the phase directory is a symlink or resolves outside the project root plan-phase resolved (for example through a symlinked parent), when its path has a `.` or `..` component, or when it holds a control character; a one-line stderr reason names the refusal, and the watch then ends on the agent's own completion result or as stalled. (#5182) diff --git a/agents/gsd-plan-checker.md b/agents/gsd-plan-checker.md index 44b7536fd9..c7729bee6d 100644 --- a/agents/gsd-plan-checker.md +++ b/agents/gsd-plan-checker.md @@ -1,8 +1,14 @@ --- name: gsd-plan-checker description: Verifies plans will achieve phase goal before execution. Goal-backward analysis of plan quality. Spawned by /gsd:plan-phase orchestrator. -tools: Read, Bash, Glob, Grep, Skill +tools: Read, Write, Bash, Glob, Grep, Skill color: green +# hooks: +# PostToolUse: +# - matcher: "Write|Edit" +# hooks: +# - type: command +# command: "npx eslint --fix $FILE 2>/dev/null || true" --- @@ -960,6 +966,12 @@ Return all issues as a structured `issues:` YAML list (see dimension examples fo +**Return receipt (#5182):** if your prompt has a `PATH` line, +your LAST action is to write the marker line you are about to return (e.g. `## ISSUES FOUND`) +to PATH with the Write tool; never use `Bash(cat << 'EOF')` or heredoc. Its directory already exists. This is your only write: never use +Write on a plan or any other file. If the write fails, or you have no Write tool (an install drops it where +Write would grant full edit), note that in your return and continue; never write it another way. + ## VERIFICATION PASSED ```markdown @@ -1031,6 +1043,8 @@ Plans verified. Run `/gsd:execute-phase {phase}` to proceed. +**DO NOT** use Write on any file except the `` path, if your prompt gives one. You never write, split or fix a plan. + **DO NOT** check code existence — that's gsd-verifier's job. You verify plans, not codebase. **DO NOT** run the application. Static plan analysis only. diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index b1a841e20e..331c287aa8 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -931,6 +931,12 @@ See @~/.claude/gsd-core/references/planner-chunked.md for `## OUTLINE COMPLETE` Your orchestrator dispatches on exact marker strings in your final output. Emit exactly one of: +**Return receipt (#5182):** if your prompt has a `PATH` line, +your LAST action, after every plan write and commit, is to write the marker line you are about to +return (e.g. `## PLANNING COMPLETE`, or `## REVISION COMPLETE` in revision mode) to PATH with +the Write tool. Its directory already exists. The orchestrator's bounded stall watch reads only +that file, so never write it early. + ```markdown ## PLANNING COMPLETE ``` @@ -961,6 +967,11 @@ Your orchestrator dispatches on exact marker strings in your final output. Emit ``` (cannot produce a plan, include exactly what is missing) +```markdown +## REVISION COMPLETE +``` +(revision mode only — checker issues addressed; shape: `gsd-core/references/planner-revision.md` Step 7) + ```markdown ## REVISION_CONFLICT ``` diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 66eed2129b..35f9921278 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -239,8 +239,8 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp |----------|-------| | **Spawned by** | `/gsd-plan-phase` (verification loop, max 3 iterations) | | **Parallelism** | Single instance (iterative) | -| **Tools** | Read, Bash, Glob, Grep, Skill | -| **Disallowed Tools** | Write, Edit, MultiEdit | +| **Tools** | Read, Write, Bash, Glob, Grep, Skill | +| **Disallowed Tools** | Edit, MultiEdit | | **Model (balanced)** | Sonnet | | **Color** | Green | | **Produces** | PASS/FAIL verdict with specific feedback | @@ -868,7 +868,7 @@ Thirteen additional agents ship under `agents/gsd-*.md` and are used by specialt | planner | ✓ | ✓ | | ✓ | ✓ | ✓ | | ✓ | ✓ | | roadmapper | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | | executor | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | | | | -| plan-checker | ✓ | | | ✓ | ✓ | ✓ | | | | +| plan-checker | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | | integration-checker | ✓ | | | ✓ | ✓ | ✓ | | | | | ui-checker | ✓ | | | ✓ | ✓ | ✓ | | | | | verifier | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | @@ -882,7 +882,7 @@ Thirteen additional agents ship under `agents/gsd-*.md` and are used by specialt | security-auditor | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | | | | **Principle of Least Privilege:** -- Checkers are read-only (no Write/Edit) — they evaluate, never modify +- Checkers never modify what they check (no Edit); plan-checker's one Write is its stall-watch return receipt (#5182). OpenCode, Kilo and Copilot grant Write and Edit as one `edit` permission, so the installer drops plan-checker's Write there: it stays read-only and writes no receipt - Researchers have web access — they need current ecosystem information - Executors have Edit — they modify code but not web access - Mappers have Write — they write analysis documents but not Edit (no code changes) diff --git a/docs/ja-JP/AGENTS.md b/docs/ja-JP/AGENTS.md index d35ae8b104..4c4d04bc1d 100644 --- a/docs/ja-JP/AGENTS.md +++ b/docs/ja-JP/AGENTS.md @@ -221,7 +221,7 @@ GSD はマルチエージェントアーキテクチャを採用しており、 |------------|-----| | **スポーン元** | `/gsd-plan-phase`（検証ループ、最大3回の反復） | | **並列数** | 単一インスタンス（反復型） | -| **ツール** | Read, Bash, Glob, Grep | +| **ツール** | Read, Write, Bash, Glob, Grep | | **モデル (balanced)** | Sonnet | | **カラー** | Green | | **生成物** | 具体的なフィードバック付きの PASS/FAIL 判定 | @@ -411,7 +411,7 @@ GSD はマルチエージェントアーキテクチャを採用しており、 | planner | ✓ | ✓ | | ✓ | ✓ | ✓ | | ✓ | ✓ | | roadmapper | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | | executor | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | | | | -| plan-checker | ✓ | | | ✓ | ✓ | ✓ | | | | +| plan-checker | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | | integration-checker | ✓ | | | ✓ | ✓ | ✓ | | | | | ui-checker | ✓ | | | ✓ | ✓ | ✓ | | | | | verifier | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | @@ -422,7 +422,7 @@ GSD はマルチエージェントアーキテクチャを採用しており、 | user-profiler | ✓ | | | | | | | | | **最小権限の原則:** -- チェッカーは読み取り専用（Write/Edit なし） — 評価のみを行い、変更は行わない +- チェッカーは確認対象を変更しない（Edit なし） — plan-checker の唯一の Write はストール監視用のリターンレシート（#5182）。OpenCode・Kilo・Copilot は Write と Edit を 1 つの `edit` 権限として付与するため、インストーラーはそこで plan-checker の Write を外す（読み取り専用のまま、レシートは書かれない） - リサーチャーは Web アクセスを持つ — 最新のエコシステム情報が必要なため - エグゼキューターは Edit を持つ — コードを変更するが Web アクセスは不要 - マッパーは Write を持つ — 分析ドキュメントを作成するが Edit は不要（コード変更なし） diff --git a/docs/ko-KR/AGENTS.md b/docs/ko-KR/AGENTS.md index b182513c2c..4baed07360 100644 --- a/docs/ko-KR/AGENTS.md +++ b/docs/ko-KR/AGENTS.md @@ -221,7 +221,7 @@ GSD는 멀티 에이전트 아키텍처를 사용합니다. 가벼운 오케스 |----------|-------| | **생성 주체** | `/gsd-plan-phase` (검증 루프, 최대 3회 반복) | | **병렬성** | 단일 인스턴스 (반복적) | -| **도구** | Read, Bash, Glob, Grep | +| **도구** | Read, Write, Bash, Glob, Grep | | **모델 (balanced)** | Sonnet | | **색상** | Green | | **생성물** | 구체적인 피드백을 포함한 PASS/FAIL 판정 | @@ -411,7 +411,7 @@ GSD는 멀티 에이전트 아키텍처를 사용합니다. 가벼운 오케스 | planner | ✓ | ✓ | | ✓ | ✓ | ✓ | | ✓ | ✓ | | roadmapper | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | | executor | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | | | | -| plan-checker | ✓ | | | ✓ | ✓ | ✓ | | | | +| plan-checker | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | | integration-checker | ✓ | | | ✓ | ✓ | ✓ | | | | | ui-checker | ✓ | | | ✓ | ✓ | ✓ | | | | | verifier | ✓ | ✓ | | ✓ | ✓ | ✓ | | | | @@ -422,7 +422,7 @@ GSD는 멀티 에이전트 아키텍처를 사용합니다. 가벼운 오케스 | user-profiler | ✓ | | | | | | | | | **최소 권한 원칙.** -- Checker는 읽기 전용입니다 (Write/Edit 없음) — 평가만 하며 수정하지 않습니다 +- Checker는 검사 대상을 수정하지 않습니다 (Edit 없음) — plan-checker의 유일한 Write는 stall 감시용 반환 영수증입니다 (#5182). OpenCode, Kilo, Copilot은 Write와 Edit를 하나의 `edit` 권한으로 부여하므로, 설치 프로그램은 그곳에서 plan-checker의 Write를 제거합니다 (읽기 전용으로 유지되고 영수증은 작성되지 않습니다) - Researcher는 웹 접근 권한을 가집니다 — 최신 생태계 정보가 필요하기 때문입니다 - Executor는 Edit 권한을 가집니다 — 코드를 수정하지만 웹 접근은 없습니다 - Mapper는 Write 권한을 가집니다 — 분석 문서를 작성하지만 Edit은 없습니다 (코드 변경 없음) diff --git a/gsd-core/references/agent-contracts.md b/gsd-core/references/agent-contracts.md index f20ed78248..26a06f57b5 100644 --- a/gsd-core/references/agent-contracts.md +++ b/gsd-core/references/agent-contracts.md @@ -11,7 +11,7 @@ This doc describes what IS, not what should be. Casing inconsistencies are docum | Agent | Role | Completion Markers | Consumed by | Kind | |-------|------|--------------------|--------------|------| | gsd-ai-researcher | AI framework research | No marker (writes the AI-SPEC.md framework section via Edit) | `gsd-core/workflows/ai-integration-phase.md` reads the AI-SPEC.md section after the agent returns | artifact+query | -| gsd-planner | Plan creation | `## PLANNING COMPLETE`, `## OUTLINE COMPLETE`, `## PHASE SPLIT RECOMMENDED`, `## ⚠ Source Audit`, `## CHECKPOINT REACHED`, `## PLANNING INCONCLUSIVE`, `## REVISION_CONFLICT` | `gsd-core/workflows/plan-phase.md`, `gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md`, `gsd-core/workflows/plan-review-convergence.md`, `gsd-core/workflows/quick.md`, `gsd-core/workflows/quick/steps/plan-checker-loop.md`, `gsd-core/workflows/verify-work.md` | sentinel-match | +| gsd-planner | Plan creation | `## PLANNING COMPLETE`, `## OUTLINE COMPLETE`, `## PHASE SPLIT RECOMMENDED`, `## ⚠ Source Audit`, `## CHECKPOINT REACHED`, `## PLANNING INCONCLUSIVE`, `## REVISION COMPLETE`, `## REVISION_CONFLICT` | `gsd-core/workflows/plan-phase.md`, `gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md`, `gsd-core/workflows/plan-review-convergence.md`, `gsd-core/workflows/quick.md`, `gsd-core/workflows/quick/steps/plan-checker-loop.md`, `gsd-core/workflows/verify-work.md` | sentinel-match | | gsd-executor | Plan execution | `## PLAN COMPLETE`, `## CHECKPOINT REACHED` | `gsd-core/workflows/plan-phase.md`, `gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md`, `agents/gsd-debug-session-manager.md`, `agents/gsd-debugger.md` | sentinel-match | | gsd-phase-researcher | Phase-scoped research | `## RESEARCH COMPLETE`, `## RESEARCH BLOCKED` | `gsd-core/workflows/plan-phase.md`, `gsd-core/workflows/quick/steps/research-phase.md`, `agents/gsd-project-researcher.md` | sentinel-match | | gsd-project-researcher | Project-wide research | `## RESEARCH COMPLETE`, `## RESEARCH BLOCKED` | `gsd-core/workflows/plan-phase.md`, `gsd-core/workflows/quick/steps/research-phase.md`, `agents/gsd-phase-researcher.md` | sentinel-match | diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index b03b2e04a5..03f62aeb22 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -105,7 +105,7 @@ When `CONTEXT_WINDOW >= 500000`, the planner prompt includes the 3 most recent p **#2401 — `prior_verify_commands` is NOT part of that enrichment and is never gated on `CONTEXT_WINDOW`.** It is a handful of one-line `` commands harvested from the nearest prior phase that had any; the payload is tiny and its absence at 200k is exactly what made the planner re-invent a verify command and author an unrunnable path. Surface it at every context window. -Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_enabled`, `plan_checker_enabled`, `nyquist_validation_enabled`, `commit_docs`, `text_mode`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`, `has_research`, `has_context`, `has_reviews`, `has_plans`, `plan_count`, `phase_status` (#3569), `planning_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`, `granularity`, `prior_verify_commands` (#2401 — array of `{phase, plan, task, command}`, possibly empty; emitted at every context window), `threat_id_duplicates` + `threat_id_duplicate_count` (#4683 — cross-plan threat-ID collisions; consumed at step 5.55). +Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_enabled`, `plan_checker_enabled`, `nyquist_validation_enabled`, `commit_docs`, `text_mode`, `phase_found`, `phase_dir`, `project_root` (`PROJECT_ROOT`), `phase_number`, `phase_name`, `phase_slug`, `padded_phase`, `has_research`, `has_context`, `has_reviews`, `has_plans`, `plan_count`, `phase_status` (#3569), `planning_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`, `granularity`, `prior_verify_commands` (#2401 — array of `{phase, plan, task, command}`, possibly empty; emitted at every context window), `threat_id_duplicates` + `threat_id_duplicate_count` (#4683 — cross-plan threat-ID collisions; consumed at step 5.55). **#2517:** omit the `model=` param from an `Agent()` call when its `researcher`/`planner`/`checker`_model is `"inherit"` or empty — passing `model=""` 404s on non-Claude runtimes; omitting inherits the orchestrator model (mirrors execute-phase). @@ -745,7 +745,7 @@ prohibition recall in the planner). Pass `$COVERAGE` and `$SPECLESS_FALLBACK_DIS ## 7.99. Bounded Stall-Detection Helpers (#2650) Read+execute `gsd-core/workflows/plan-phase/steps/stall-detection-helpers.md` (defines -`gsd_stall_should_recover`/`gsd_stall_watch`, and how `{outputFile}` below is bound; +watch helpers; pre-dispatch, `{receipt}` = `gsd_receipt_path "${PHASE_DIR}" "${PROJECT_ROOT}"`; independent of the teams-status guard above, AC2). ## 8. Spawn gsd-planner Agent @@ -956,6 +956,7 @@ Every task MUST include these fields — they are NOT optional: - [ ] Every SPEC ## Prohibitions resolved item is represented in a plan's must_haves.prohibitions (no silent drops) +{receipt} ``` **If `CHUNKED_MODE` is `false` (default):** Spawn the planner as a single long-lived Agent: @@ -973,7 +974,7 @@ Agent( ) ``` -**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{outputFile}" "${PHASE_DIR}"'/*-PLAN.md' "## PLANNING COMPLETE" "## PHASE SPLIT RECOMMENDED" "## ⚠ Source Audit" "## CHECKPOINT REACHED" "## PLANNING INCONCLUSIVE")` while waiting/active — `marker_received` -> step 9; `stalled` -> 9a. +**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** (`planner`) `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{receipt}" "${PHASE_DIR}"'/*-PLAN.md' "## PLANNING COMPLETE" "## PHASE SPLIT RECOMMENDED" "## ⚠ Source Audit" "## CHECKPOINT REACHED" "## PLANNING INCONCLUSIVE")` while waiting/active — `marker_received` -> step 9; `stalled` -> 9a; after routing any result (stalled/Retry too): `rm -f "{receipt}"`. - **`false`:** issue the same Agent() call but omit `run_in_background`; await its ordinary runtime-native completion and pass the real returned result to step 9. Skip `gsd_stall_watch` entirely. This is not fire-and-forget; empty, truncated, or unrecognized returns still use step 9a. @@ -1097,6 +1098,7 @@ If an actionable finding remains only in REVIEWS.md and would be invisible to /g - ## VERIFICATION PASSED — all checks pass - ## ISSUES FOUND — structured issue list +{receipt} ``` ``` @@ -1112,14 +1114,14 @@ Agent( **Dispatch/wait gate — `PLANNER_STALL_DETECTION_ENABLED` (config key `planner.stall_detection_enabled`, resolved in `plan-phase/steps/stall-detection-helpers.md`):** - **`true` (default):** use `run_in_background=true` in the Agent() call above, then use `gsd_stall_watch` below. -**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** `TS=$(date +%s)`; repeat `CHECKER_STALL_RESULT=$(gsd_stall_watch "$TS" "{outputFile}" "${PHASE_DIR}"'/*-PLAN.md' "## VERIFICATION PASSED" "## ISSUES FOUND")` while waiting/active. +**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** (`checker`) `TS=$(date +%s)`; repeat `CHECKER_STALL_RESULT=$(gsd_stall_watch "$TS" "{receipt}" "${PHASE_DIR}"'/*-PLAN.md' "## VERIFICATION PASSED" "## ISSUES FOUND")` while waiting/active; after routing any result (stalled/Retry too): `rm -f "{receipt}"`. - **`false`:** issue the same Agent() call but omit `run_in_background`; await its ordinary runtime-native completion and pass the real returned result to step 11. Skip `gsd_stall_watch` entirely. Treat a recognized returned marker exactly like `marker_received`; empty, truncated, or unrecognized returns still use step 11a. ## 11. Handle Checker Return -- **`marker_received` + `## VERIFICATION PASSED`:** If the return lists an entry whose severity is BLOCKER or WARNING, in the YAML issues block or listed in prose (a mention such as "no warnings" or a zero count is not an entry), treat it as `## ISSUES FOUND` instead: display issues, check iteration count, proceed to step 12, which counts those prose entries as BLOCKER/WARNING entries (the no-YAML-block shortcut there does not apply). The sentinel never decides alone, and an entry with a missing or unrecognized severity counts as a BLOCKER (#5578). Otherwise display confirmation, proceed to step 13. -- **`marker_received` + `## ISSUES FOUND`:** Display issues, check iteration count, proceed to step 12. +- **`marker_received` + `## VERIFICATION PASSED`:** If the return lists an entry whose severity is BLOCKER or WARNING, in the YAML issues block or listed in prose (a mention such as "no warnings" or a zero count is not an entry), treat it as `## ISSUES FOUND` instead: display issues, check iteration count, proceed to step 12, which counts those prose entries as BLOCKER/WARNING entries (the no-YAML-block shortcut there does not apply). The sentinel never decides alone, and an entry with a missing or unrecognized severity counts as a BLOCKER (#5578). Otherwise display confirmation, proceed to step 13 (receipt only, no body by the next cycle: 11a). +- **`marker_received` + `## ISSUES FOUND`:** Display issues, check iteration count, proceed to step 12 (no issue list: 11a). - **`stalled`:** Automatically surface 11a's recovery choice (Accept verification / Retry checker / Stop) — no manual interrupt needed. - **Empty / truncated / no recognized marker:** → Filesystem fallback (step 11a). @@ -1194,6 +1196,7 @@ the alternatives rather than applying or working around it. Full contract: Do NOT replan from scratch unless fundamental. Return what changed. +{receipt} ``` ``` @@ -1209,7 +1212,7 @@ Agent( **Dispatch/wait gate — `PLANNER_STALL_DETECTION_ENABLED` (config key `planner.stall_detection_enabled`, resolved in `plan-phase/steps/stall-detection-helpers.md`):** - **`true` (default):** use `run_in_background=true` in the Agent() call above, then use `gsd_stall_watch` below. -**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** (7.99; no marker, mtimes only) `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{outputFile}" "${PHASE_DIR}"'/*-PLAN.md')` while waiting/active — `stalled` -> 1) Accept as revised, to step 13, 2) Retry, 3) Stop. +**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** (`revision`) `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{receipt}" "${PHASE_DIR}"'/*-PLAN.md' "## REVISION COMPLETE" "## REVISION_CONFLICT")` while waiting/active — `marker_received` -> below; `stalled` -> 1) Accept as revised, to step 13, 2) Retry, 3) Stop; after routing any result (stalled/Retry too): `rm -f "{receipt}"`. - **`false`:** issue the same Agent() call but omit `run_in_background`; await its ordinary runtime-native completion and pass the real returned result into the existing revision-return handling. Skip `gsd_stall_watch` entirely; an empty, truncated, or unrecognized result keeps the existing filesystem fallback. diff --git a/gsd-core/workflows/plan-phase/detail/elaboration.md b/gsd-core/workflows/plan-phase/detail/elaboration.md index 8ba94864ed..a2d8aa81a8 100644 --- a/gsd-core/workflows/plan-phase/detail/elaboration.md +++ b/gsd-core/workflows/plan-phase/detail/elaboration.md @@ -100,9 +100,22 @@ Use AskUserQuestion for each gap (or batch if multiple gaps). ## § 11a — Filesystem Fallback (Checker) -Fires when the checker's Agent() call comes back without either completion marker (`## VERIFICATION PASSED` / `## ISSUES FOUND`). The spine already computed `DISK_PLANS` before reaching this elaboration. +Fires when the checker's Agent() call comes back without either completion marker (`## VERIFICATION PASSED` / `## ISSUES FOUND`), or when one of those markers arrived on the return receipt but the body step 11 reads (the issue list, or the findings a PASSED return lists) has not arrived by the next cycle (`steps/stall-detection-helpers.md`). The spine already computed `DISK_PLANS` before reaching this elaboration. -**If `DISK_PLANS` > 0:** Plans exist on disk; the checker return was empty or truncated (the +**If `DISK_PLANS` > 0 and a marker arrived on the receipt without its body:** the checker's +findings cannot be read, so the receipt alone never counts as a pass or as 0 issues. Display: + +```text +◆ Checker receipt says {MARKER}, but its full return has not arrived. {DISK_PLANS} plan(s) exist on disk. + Its findings cannot be read yet, so the receipt alone is not treated as a result. +``` + +Offer 3 options, Retry first: +1. **Retry checker**: re-spawn the checker with the same prompt (return to step 10) +2. **Accept verification**: treat as `## VERIFICATION PASSED` and continue to step 13 +3. **Stop**: exit; user can re-run `/gsd:plan-phase {N}` to resume + +**If `DISK_PLANS` > 0 and no marker arrived:** Plans exist on disk; the checker return was empty or truncated (the Windows stdio hang pattern — the subagent finished but the return never arrived). Display: ```text diff --git a/gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md b/gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md index b018c36666..82ef505708 100644 --- a/gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md +++ b/gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md @@ -66,7 +66,8 @@ Agent( End the file with a final line `## OUTLINE COMPLETE` — §8.5.1's resume-check greps the file for it, so it MUST be written here, not just returned. - Return: ## OUTLINE COMPLETE with plan count.", + Return: ## OUTLINE COMPLETE with plan count. + {receipt}", subagent_type="gsd-planner", model="{planner_model}", description="Outline Phase {phase} (chunked)", @@ -74,7 +75,7 @@ Agent( ) ``` -**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{outputFile}" "$OUTLINE_FILE" "## OUTLINE COMPLETE")` while waiting/active. +**ORCHESTRATOR RULE — ALL RUNTIMES (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** before dispatch bind `{receipt}` via `gsd_receipt_path "${PHASE_DIR}" outline "${PROJECT_ROOT}"`, `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{receipt}" "$OUTLINE_FILE" "## OUTLINE COMPLETE")` while waiting/active; after routing any result (stalled/Retry too): `rm -f "{receipt}"`. - **`false`:** issue the same Agent() call but omit `run_in_background`; await its ordinary runtime-native completion and consume the real returned result. Skip `gsd_stall_watch` entirely and treat a recognized `## OUTLINE COMPLETE` return exactly like `marker_received`; empty or unrecognized returns keep the existing Retry/Stop path. @@ -158,7 +159,8 @@ GSD_ARGS=${GSD_ARGS%$'\n'} Wave: {wave} | Depends on: {depends_on} Phase requirement IDs to cover in this plan: {plan_requirements} - Return: ## PLAN COMPLETE with the plan ID.", + Return: ## PLAN COMPLETE with the plan ID. + {receipt}", subagent_type="gsd-planner", model="{planner_model}", description="Plan {plan_id} (chunked {k}/{N})", @@ -172,8 +174,8 @@ GSD_ARGS=${GSD_ARGS%$'\n'} message, before waiting on any of them. 5. **ORCHESTRATOR RULE — ALL RUNTIMES, per batch (when `PLANNER_STALL_DETECTION_ENABLED` is `true`):** for every entry dispatched in this round, - `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{outputFile}" "$PLAN_FILE" "## PLAN COMPLETE")` - while waiting/active for THAT entry. Serial dispatch waits on one entry at a time (unchanged). + bind that entry's own `{receipt}` via `gsd_receipt_path "${PHASE_DIR}" "{plan_id}" "${PROJECT_ROOT}"` before its dispatch, `TS=$(date +%s)`; repeat `PLANNER_STALL_RESULT=$(gsd_stall_watch "$TS" "{receipt}" "$PLAN_FILE" "## PLAN COMPLETE")` + while waiting/active for THAT entry; after routing any result (stalled/Retry too): `rm -f "{receipt}"`. Serial dispatch waits on one entry at a time (unchanged). Concurrent dispatch waits on every entry issued in step 4 before proceeding — this is the "per-batch" join the config makes possible: nothing in step 6 runs until every plan dispatched this round has reached `marker_received` or `stalled`. A `stalled` entry falls into step 7's diff --git a/gsd-core/workflows/plan-phase/steps/stall-detection-helpers.md b/gsd-core/workflows/plan-phase/steps/stall-detection-helpers.md index d03f21ea0d..20cf82a0af 100644 --- a/gsd-core/workflows/plan-phase/steps/stall-detection-helpers.md +++ b/gsd-core/workflows/plan-phase/steps/stall-detection-helpers.md @@ -21,31 +21,77 @@ recovery: if the runtime loses the completion handoff, the user may need to interrupt and use the existing filesystem fallback. The completion markers and empty/truncated/unrecognized-return fallback remain unchanged. -**Binding `{outputFile}` (load-bearing, not optional):** every `gsd_stall_watch` -call below takes `{outputFile}` as its second argument — a literal token the -orchestrator must substitute with the REAL path from the immediately preceding -`run_in_background=true` Agent() call's returned `async_launched` result, -exactly as `docs-update.md:471` already does ("Read tool: file_path: `{outputFile -from README agent result}`"). This is NOT a bash variable the snippet below -assigns — there is nothing upstream that assigns one, so a bash variable -reference here would silently stay empty forever. With `{outputFile}` correctly -substituted, `[ -f "$output_file" ]` can find the real file and the -`marker_received` path is reachable; left as a literal (or as an unbound bash -variable), `marker_found` can never become `true` and every spawn silently -falls back to the mtime-only path — for the plan-checker spawn specifically, -that fallback is broken (see next paragraph), so binding this correctly there -is not a nice-to-have. - -**Plan-checker's artifact glob needs the marker, not just mtimes:** the -plan-checker spawn watches `*-PLAN.md` for freshness, but a checker that -PASSES touches none of those files — no fresh mtime, ever, on a clean run. -Without `{outputFile}` correctly bound to the real completion output, a -healthy plan-checker that returns `## VERIFICATION PASSED` in two minutes -would still be declared `stalled` once `planner.stall_threshold_minutes` -elapses — reporting a succeeded agent as hung, which is worse than the -original unbounded wait. The marker path (via `{outputFile}`) is the ONLY -working completion signal for that spawn; the artifact glob is secondary -there. +**Binding `{receipt}` (load-bearing, not optional; #5182):** every `gsd_stall_watch` +call takes `{receipt}` as its second argument: a GSD-owned return receipt, one per +dispatch. Before each Agent() call that carries the receipt line, under either toggle, +the orchestrator runs +`gsd_receipt_path "${PHASE_DIR}" "${PROJECT_ROOT}"` (`` is `planner`, `checker`, +`revision`, `outline`, or the plan ID; `PROJECT_ROOT` is init's `project_root`) and substitutes the printed absolute path for `{receipt}`, +both in the spawn prompt's `{receipt}` line and in +every watch call for that spawn. If `gsd_receipt_path` prints nothing (it fails closed on +an unsafe or symlinked phase dir, naming the reason on stderr), there is no receipt: omit the `` line +and pass `""` as the watch's second argument; the wait then ends on the runtime's +completion result or as `stalled`. Like `$TS`, it is a literal the orchestrator carries +across tool calls; nothing persists across fences, so never pass an unexpanded +`$RECEIPT`/`$TS` expression. The agent's LAST action is to write the marker line it +returns to that path (`agents/gsd-planner.md`, `agents/gsd-plan-checker.md`). +On `marker_received`, the marker that routes (step 11's `## VERIFICATION PASSED` +vs `## ISSUES FOUND`, the revision's COMPLETE vs CONFLICT) is +`gsd_return_marker "{receipt}" `; never re-grep anything else. + +The watch never reads a host output file (`{outputFile}`). On Claude Code that file is +the subagent transcript, which already holds the agent's prompt and definition +snapshot, and both quote the markers, so a match there proves nothing. Other hosts +return no such file at all. The receipt is the same on every runtime, and +`gsd_return_marker` counts a marker only at the START of a receipt line, so quoted or +JSON-encoded marker text can never match. Each watch site's rule runs `rm -f "{receipt}"` +after routing any result, stalled and Retry included. With the toggle `false`, the +prompt still carries the receipt line and the agent still writes it; the orchestrator +consumes the real returned result and removes the receipt the same way. + +**The runtime's completion result ends the wait:** the receipt bounds the wait; it does +not replace the runtime's own completion. If the spawned agent's real completion +result reaches the orchestrator between cycles, stop the watch and route that +result's recognized marker exactly as the `false` path does. Precedence: whichever +of the two ends the wait routes it. The receipt carries only the marker; the full +return (the checker's issue list, the planner's plan count or checkpoint) always +comes from the completion result. If both are present and name different markers, +treat the return as unrecognized (9a/11a). A route that needs the body when it never +arrives fails closed: a receipt-routed `## ISSUES FOUND` with no issue list is +never counted as 0 issues; use 11a and offer Retry checker before Accept. A +receipt-routed `## VERIFICATION PASSED` whose body has not arrived by the next cycle +never proceeds to step 13 on the receipt alone, since step 11 reads that body for +listed findings (#5578): use 11a and offer Retry checker before Accept. A planner +`## CHECKPOINT REACHED`, `## PHASE SPLIT RECOMMENDED` or `## ⚠ Source Audit` whose body +(the question, the proposed split, the gap list) has not arrived by the next cycle +never routes to an empty checkpoint, 9b or 9c: use 9a, name the marker received, and +offer Retry planner before Accept. A revision `## REVISION_CONFLICT` whose body (the +`CONFLICT_*` fields step 12 records) has not arrived by the next cycle never writes an +empty conflict record: use that watch's `stalled` menu, name the marker received, and +offer Retry before Accept. + +The helper functions and the config values above do not persist between tool calls +either: re-run this block in every fence that calls them. + +**Plan-checker receipt:** a checker that PASSES touches no `*-PLAN.md`, so its receipt +is its only completion signal for this watch. The checker declares `Write` for that one +file (a #767 Group B report-writer: Claude denies it only `Edit, MultiEdit`, and Codex +derives `workspace-write` from its `tools:`), so it can write the receipt on both. + +**Known limit, hosts with no separate Write (OpenCode, Kilo, Copilot):** they grant Write +and Edit as one `edit` permission, so the installer drops the checker's Write there and it +stays read-only; no checker receipt is written. The wait ends on the completion result, +as above, or as `stalled` at the threshold. + +**Known limit, kimi-code:** it dispatches `-planner` to its `plan` built-in and +`-checker` to `explore`, and neither can write files (`runtime-aware-dispatch.md`), +so no receipt is written there. The wait ends on the completion result, as above, +or as `stalled` at the threshold. + +**Known limit, no receipt and no fresh artifact:** a checker with no receipt (any +Write failure, or the host above) touches no `*-PLAN.md`, and neither does a planner +before its first `PLAN.md`, so its watch can only end on the completion result or +as `stalled` at the threshold. There is no heartbeat protocol. **Single-cycle by design, not one long-lived loop:** `gsd_stall_watch` sleeps for exactly one `PLANNER_STALL_INTERVAL_MINUTES` and returns — it does NOT @@ -133,31 +179,140 @@ gsd_stall_should_recover() { echo "waiting"; return 0 } +# gsd_receipt_path PHASE_DIR SPAWN [ROOT] — print a fresh, absolute return-receipt path +# PHASE_DIR/.gsd-returns/SPAWN.EPOCH.XXXXXXXX (#5182). The directory is created +# (with a `*` .gitignore); the file is NOT (a host Write tool may refuse to +# overwrite a file the agent never read). Absolute because a subagent's cwd may +# differ from the orchestrator's; `mktemp -u` keeps back-to-back dispatches in one +# second distinct; EPOCH lets gsd_stall_watch recover the dispatch time if `$TS` +# was lost between tool calls. SPAWN is reduced to [A-Za-z0-9_-], so a plan ID can +# never leave the directory (the character lists are spelled out, not ranges, so +# no locale can widen them). Fails closed (prints nothing, returns 1) on an empty +# PHASE_DIR or one holding a quote, `$`, backtick, `<`, `>` or any control +# character (newline, tab, CR...), which the orchestrator could not substitute +# safely into a prompt line or a quoted bash argument. It also fails closed unless +# the receipt directory sits physically under ROOT, the project root the workflow +# resolved (init's `project_root`; the orchestrator's `pwd -P` when ROOT is empty, so a +# phase dir outside a subdirectory cwd is still accepted when ROOT is passed): a checkout can commit the phase dir, `.planning` or `.gsd-returns` as +# a symlink, and mkdir -p and the .gitignore write would follow it out of the +# project. So a symlinked or dangling phase dir (or a missing component on the way +# to it), a phase dir with a `..` component (after a missing component it would +# pass the ancestor check, and mkdir -p would then resolve it outside the project) +# or a `.` component (init's phase_dir never has one), +# a phase dir that resolves outside the root, and a symlinked .gsd-returns +# or .gitignore all disable the receipt; the watch then ends on the runtime's +# completion result or as `stalled`. Both sides are compared after `cd -P`/`pwd -P` +# (macOS temp dirs live under the /var -> /private/var link; Git Bash maps C:/ to +# /c/ on both sides). Only static, checked-in links are in scope: a link swapped in +# between these checks and the mkdir/printf (check-then-act) is not. Every refusal +# prints one `gsd_receipt_path: no receipt: ` line on stderr (the reason only, +# never the phase dir, which may hold control characters) so the orchestrator can name it. +_gsd_no_receipt() { printf 'gsd_receipt_path: no receipt: %s\n' "$1" >&2; return 1; } +gsd_receipt_path() { + local dir="$1" spawn="${2//[^ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-]/_}" + local root a up phys rest got + case "$dir" in + ''|*[[:cntrl:]]*|*[\'\"\$\`\<\>]*) _gsd_no_receipt 'phase dir is empty or holds a quote, $, backtick, <, > or a control character'; return 1 ;; + esac + case "/$dir/" in *[/\\]..[/\\]*|*[/\\].[/\\]*) _gsd_no_receipt 'phase dir has a . or .. component'; return 1 ;; esac + case "$dir" in + /*|[A-Za-z]:[/\\]*) ;; + *) dir="$(pwd)/$dir" ;; + esac + while [ "${#dir}" -gt 1 ] && [ "${dir%/}" != "$dir" ]; do dir="${dir%/}"; done + if [ -n "${3:-}" ]; then + root=$(CDPATH='' cd -P -- "$3" >/dev/null 2>&1 && pwd -P) || { _gsd_no_receipt 'project root does not resolve'; return 1; } + else + root=$(pwd -P) || { _gsd_no_receipt 'working directory does not resolve'; return 1; } + fi + # Walk up to the nearest existing ancestor; any walked component that is a link + # (tested before -e, so a dangling one counts) is refused. + a="$dir" + while :; do + [ -L "$a" ] && { _gsd_no_receipt 'a symlink on the path to the phase dir'; return 1; } + [ -e "$a" ] && break + up="${a%/*}"; [ -n "$up" ] || up=/ + [ "$up" = "$a" ] && { _gsd_no_receipt 'no existing ancestor of the phase dir'; return 1; } + a="$up" + done + phys=$(CDPATH='' cd -P -- "$a" >/dev/null 2>&1 && pwd -P) || { _gsd_no_receipt 'the phase dir ancestor does not resolve'; return 1; } + case "$phys/" in "$root"/*) ;; *) _gsd_no_receipt 'the phase dir resolves outside the project root'; return 1 ;; esac + rest="${dir#"$a"}" + [ -L "$dir/.gsd-returns" ] && { _gsd_no_receipt '.gsd-returns is a symlink'; return 1; } + mkdir -p "$dir/.gsd-returns" || { _gsd_no_receipt 'cannot create .gsd-returns'; return 1; } + got=$(CDPATH='' cd -P -- "$dir/.gsd-returns" >/dev/null 2>&1 && pwd -P) || { _gsd_no_receipt '.gsd-returns does not resolve'; return 1; } + [ "$got" = "${phys%/}$rest/.gsd-returns" ] || { _gsd_no_receipt '.gsd-returns resolves elsewhere'; return 1; } + [ -L "$dir/.gsd-returns/.gitignore" ] && { _gsd_no_receipt '.gsd-returns/.gitignore is a symlink'; return 1; } + [ -f "$dir/.gsd-returns/.gitignore" ] || printf '*\n' > "$dir/.gsd-returns/.gitignore" + if command -v cygpath >/dev/null 2>&1; then dir=$(cygpath -m "$dir"); fi + mktemp -u "$dir/.gsd-returns/${spawn:-spawn}.$(date +%s).XXXXXXXX" +} + +# gsd_return_marker FILE MARKER... — print the first MARKER that STARTS a line of +# FILE's first 64 lines and first 4096 bytes (the receipt is agent-written; one +# marker line is expected; the byte cap keeps one huge line from stalling the +# watch), or nothing. A marker must end within those 4096 bytes and its boundary +# must be seen (4097 bytes are read): a line cut at the read window ends nowhere, so +# a longer word cut at the cap is never taken for a marker. Literal prefix match (no regex), and the marker must be +# followed by the end of the line, a space, tab, CR, VT, FF or a colon (the +# planner's Source Audit return puts `: Unplanned Items Found` after its marker), +# so `## PLAN COMPLETE-x` or a +# longer word that merely starts with a marker is not that marker. A BOM at the +# start of the file (line 1 only) and a missing final newline are tolerated. A receipt that is a symlink is never +# read. The one owner of "which marker did the agent return": gsd_stall_watch and +# step 11's routing both use it. Indented, mid-line, or JSON-encoded marker text +# (prompts, agent definitions, transcripts) never matches. +gsd_return_marker() { + local file="$1" line m rest n=0 size cut; shift + [ -L "$file" ] && return 0 + [ -f "$file" ] && [ -r "$file" ] || return 0 + size=$(( $(head -c 4097 -- "$file" 2>/dev/null | wc -c) )) + while [ "$n" -lt 64 ]; do + cut=0 + if IFS= read -r line; then : + elif [ -n "$line" ]; then [ "$size" -le 4096 ] || cut=1 + else break + fi + n=$((n + 1)) + if [ "$n" -eq 1 ]; then line="${line#$'\xef\xbb\xbf'}"; fi + for m in "$@"; do + [[ "$line" == "$m"* ]] || continue + rest="${line#"$m"}" + case "${rest:0:1}" in + '') [ "$cut" = 0 ] && { printf '%s\n' "$m"; return 0; } ;; + ' '|$'\t'|$'\r'|$'\v'|$'\f'|:) printf '%s\n' "$m"; return 0 ;; + esac + done + done < <(head -c 4097 -- "$file" 2>/dev/null) + return 0 +} + # gsd_stall_watch — ONE bounded, real (non-LLM-side) sleep-and-check cycle, not # a long-lived loop (see "Single-cycle by design" above — a single Bash tool # call spanning the full threshold risks the host tool's own timeout killing # it first). Sleeps exactly one PLANNER_STALL_INTERVAL_MINUTES, then checks for -# a completion marker in $2 (the outputFile returned by the run_in_background -# Agent() call) or fresh mtime activity under $3 (an artifact glob), against +# a completion marker in $2 (the spawn's {receipt}, via gsd_return_marker) or +# fresh mtime activity under $3 (an artifact glob), against # elapsed time since $1 (an epoch-seconds dispatch_ts the CALLER records once, # before the first call, and passes unchanged on every repeat). Remaining args # are completion markers. Prints exactly one of: marker_received | active | # waiting | stalled. The caller repeats the call while the result is # waiting/active; any other result ends the wait. gsd_stall_watch() { - local dispatch_ts="$1" output_file="$2" artifact_glob="$3"; shift 3 + local dispatch_ts="$1" receipt="$2" artifact_glob="$3"; shift 3 local markers=("$@") - [[ "$dispatch_ts" =~ ^[0-9]+$ ]] || dispatch_ts=$(date +%s) + # A lost `$TS` (shell state does not survive between tool calls) falls back to + # the EPOCH gsd_receipt_path stamped into the receipt name, then to "now". + if ! [[ "$dispatch_ts" =~ ^[0-9]+$ ]]; then + dispatch_ts="${receipt##*/}"; dispatch_ts="${dispatch_ts#*.}"; dispatch_ts="${dispatch_ts%%.*}" + [[ "$dispatch_ts" =~ ^[0-9]+$ ]] || dispatch_ts=$(date +%s) + fi sleep "$(( PLANNER_STALL_INTERVAL_MINUTES * 60 ))" local now elapsed marker_found artifact_fresh now=$(date +%s) elapsed=$(( now - dispatch_ts )) marker_found="false" - if [ -f "$output_file" ]; then - for m in "${markers[@]}"; do - if grep -qF "$m" "$output_file" 2>/dev/null; then marker_found="true"; break; fi - done - fi + if [ -n "$(gsd_return_marker "$receipt" "${markers[@]}")" ]; then marker_found="true"; fi # -mmin -N ("modified less than N minutes ago"), not -newermt "@": # -newermt's "@" shorthand is a GNU-date convenience the shipped # BSD find(1) on macOS does NOT understand ("Can't parse date/time: @@ -165,10 +320,21 @@ gsd_stall_watch() { # silently and permanently degraded artifact_fresh to false on every # macOS run. -mmin -N needs no epoch/date-string conversion at all and is # supported identically by GNU find (Linux, Git-for-Windows' bundled - # findutils) and BSD find (macOS). $artifact_glob stays intentionally - # unquoted — the shell, not find, expands it into the matching file list. + # findutils) and BSD find (macOS). $artifact_glob is one string, a literal + # directory plus a basename pattern (`/*-PLAN.md`) or a literal file. + # The directory part stays quoted (a space in PHASE_DIR must not split it) and + # only the basename is globbed, under nullglob (restored after) with IFS empty, + # so no match gives an empty list and artifact_fresh stays false. artifact_fresh="false" - if [ -n "$(find $artifact_glob -mmin "-${PLANNER_STALL_INTERVAL_MINUTES}" 2>/dev/null)" ]; then + local IFS='' files _had_ng=0 _d="${artifact_glob%/*}" _p="${artifact_glob##*/}" + files=() + [ "$_d" = "$artifact_glob" ] && _d=. + shopt -q nullglob && _had_ng=1 + shopt -s nullglob + # shellcheck disable=SC2206 # the basename is the glob pattern; IFS is empty, so it never splits + [ -n "$artifact_glob" ] && files=("$_d"/$_p) + [ "$_had_ng" = 1 ] || shopt -u nullglob + if [ "${#files[@]}" -gt 0 ] && [ -n "$(find "${files[@]}" -mmin "-${PLANNER_STALL_INTERVAL_MINUTES}" 2>/dev/null)" ]; then artifact_fresh="true" fi gsd_stall_should_recover "$elapsed" "$PLANNER_STALL_THRESHOLD_MINUTES" "$marker_found" "$artifact_fresh" diff --git a/pi/package-agents/gsd-plan-checker.md b/pi/package-agents/gsd-plan-checker.md index da38ca49b2..ad0a2bf534 100644 --- a/pi/package-agents/gsd-plan-checker.md +++ b/pi/package-agents/gsd-plan-checker.md @@ -1,7 +1,7 @@ --- name: gsd-plan-checker description: "Verifies plans will achieve phase goal before execution. Goal-backward analysis of plan quality. Spawned by /gsd:plan-phase orchestrator." -tools: read, bash, find, grep +tools: read, write, bash, find, grep # tools with no Pi equivalent, dropped: Skill --- @@ -961,6 +961,12 @@ Return all issues as a structured `issues:` YAML list (see dimension examples fo +**Return receipt (#5182):** if your prompt has a `PATH` line, +your LAST action is to write the marker line you are about to return (e.g. `## ISSUES FOUND`) +to PATH with the Write tool; never use `Bash(cat << 'EOF')` or heredoc. Its directory already exists. This is your only write: never use +Write on a plan or any other file. If the write fails, or you have no Write tool (an install drops it where +Write would grant full edit), note that in your return and continue; never write it another way. + ## VERIFICATION PASSED ```markdown @@ -1032,6 +1038,8 @@ Plans verified. Run `/gsd:execute-phase {phase}` to proceed. +**DO NOT** use Write on any file except the `` path, if your prompt gives one. You never write, split or fix a plan. + **DO NOT** check code existence — that's gsd-verifier's job. You verify plans, not codebase. **DO NOT** run the application. Static plan analysis only. diff --git a/pi/package-agents/gsd-planner.md b/pi/package-agents/gsd-planner.md index 2dfed5ad23..a20491ac4c 100644 --- a/pi/package-agents/gsd-planner.md +++ b/pi/package-agents/gsd-planner.md @@ -926,6 +926,12 @@ See @__GSD_PI_ENGINE_ROOT__/gsd-core/references/planner-chunked.md for `## OUTLI Your orchestrator dispatches on exact marker strings in your final output. Emit exactly one of: +**Return receipt (#5182):** if your prompt has a `PATH` line, +your LAST action, after every plan write and commit, is to write the marker line you are about to +return (e.g. `## PLANNING COMPLETE`, or `## REVISION COMPLETE` in revision mode) to PATH with +the Write tool. Its directory already exists. The orchestrator's bounded stall watch reads only +that file, so never write it early. + ```markdown ## PLANNING COMPLETE ``` @@ -956,6 +962,11 @@ Your orchestrator dispatches on exact marker strings in your final output. Emit ``` (cannot produce a plan, include exactly what is missing) +```markdown +## REVISION COMPLETE +``` +(revision mode only — checker issues addressed; shape: `gsd-core/references/planner-revision.md` Step 7) + ```markdown ## REVISION_CONFLICT ``` diff --git a/pi/package-agents/manifest.json b/pi/package-agents/manifest.json index dc2877dd7d..44d1079d9f 100644 --- a/pi/package-agents/manifest.json +++ b/pi/package-agents/manifest.json @@ -24,8 +24,8 @@ "gsd-nyquist-auditor.md": "e82d1562e1df663e4a2907bd85b4316d5ab76bfa9d413ac8daecbd6f854fa42d", "gsd-pattern-mapper.md": "3f51a38c2a1e8657190ba88a314fb2c9e66e25f5ab0e3edbc4e2f649b2702d01", "gsd-phase-researcher.md": "2215ab8205f1bcb8b826870a4b8e9a949b6f77778d1bf75fae5ce4a07dd459ba", - "gsd-plan-checker.md": "1734489d067538c0fb818768aae8f336d83d4d1b2e81286eff84b6c512c67cfb", - "gsd-planner.md": "709f81898bf60f17eea9a150098c5ba2e31fddb38ea062fa0b015136502045b4", + "gsd-plan-checker.md": "455489fd4def9f2d2612af26aa40794825b19ce5326472cea877c33350d7fcf0", + "gsd-planner.md": "bb492f530c7ab05866e4a7825b1ac20453a74c37842fe69911a08b7ce67bf900", "gsd-project-researcher.md": "ac7e0065778a3ec40e023af48faa57f715c3fa13900975ef7b8d50142e44cf3c", "gsd-research-synthesizer.md": "d20735f8dc591af470c4cbb892284863c5976e2bd8a77512bf146469158d051a", "gsd-roadmapper.md": "c5cf5fa9df6bcbd7acdc1ec62c20b6e713ed6b6f2b032b17ca3044eebd0626d3", diff --git a/src/install-profiles.cts b/src/install-profiles.cts index 1ffeca5e60..2342c007d1 100644 --- a/src/install-profiles.cts +++ b/src/install-profiles.cts @@ -40,6 +40,7 @@ const { deriveAgentName: _deriveAgentName, applyAgentFrontmatterExtensions: _applyAgentFrontmatterExtensions, appendAgentTools: _appendAgentTools, + scopeReceiptOnlyWrite: _scopeReceiptOnlyWrite, } = conversionModule as { applyAgentPathRewrites: (content: string, runtime: string, pathPrefix: string) => string; processAttribution: (content: string, attribution: string | null | undefined) => string; @@ -51,6 +52,7 @@ const { deriveAgentName: (fileName: string) => string; applyAgentFrontmatterExtensions: (content: string, opts: { runtime: string; agentName: string; targetDir?: string | null }) => string; appendAgentTools: (content: string, grants: string[]) => string; + scopeReceiptOnlyWrite: (content: string, opts: { runtime: string; agentName: string }) => string; }; // #2995 (epic #1671 Phase 6.4): agent bodies join the fragment model. Markers are @@ -1058,7 +1060,10 @@ function stageAgentsForRuntimeWithConverter( content = _applyAgentPathRewrites(content, agentCtx.runtime, agentCtx.pathPrefix); // Step 2: attribution content = _processAttribution(content, agentCtx.attribution); - // Step 3: validated canonical grants, before host conversion. + // Step 3: a receipt-only Write is dropped where the host would widen it to full + // edit (#5182), then the validated canonical grants, before host conversion. The + // grants come second so a Write the user grants in agent_tools is kept. + content = _scopeReceiptOnlyWrite(content, { runtime: agentCtx.runtime, agentName }); content = _appendAgentTools(content, grants); // Step 4: converter (runtime-specific frontmatter/body transform) content = converter(content, isGlobal, { agentName }); diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index bbb9d34638..86ba3c74ed 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -4260,12 +4260,13 @@ function injectDisallowedToolsFrontmatter(content: string, disallowedValue: stri // #767 — Read-only verifier/auditor agents get a Claude-Code disallowedTools deny-list. // Group A (pure read-only) deny Write,Edit,MultiEdit. Group B report-writers Write one -// output file so they deny only Edit,MultiEdit. gsd-nyquist-auditor is intentionally +// output file so they deny only Edit,MultiEdit (gsd-plan-checker joins them: its one +// write is the plan-phase stall-watch return receipt, #5182). gsd-nyquist-auditor is intentionally // excluded (it legitimately uses Write AND Edit to create/patch test files). Relocated // verbatim from bin/install.js (#2875 Part 2) — single source of truth for both the // inline loop (which now requires this export) and the descriptor pipeline. const READONLY_AGENT_DISALLOWED_TOOLS: Record = { - 'gsd-plan-checker': 'Write, Edit, MultiEdit', + 'gsd-plan-checker': 'Edit, MultiEdit', 'gsd-integration-checker': 'Write, Edit, MultiEdit', 'gsd-ui-checker': 'Write, Edit, MultiEdit', 'gsd-verifier': 'Edit, MultiEdit', @@ -4274,6 +4275,68 @@ const READONLY_AGENT_DISALLOWED_TOOLS: Record = { 'gsd-ui-auditor': 'Edit, MultiEdit', }; +// #5182 — agents that declare Write for ONE file only: gsd-plan-checker writes its +// plan-phase stall-watch return receipt and nothing else. A host whose agent tool map +// sends Write and Edit to the same permission cannot grant that one write without +// granting full edit, so there the agent keeps its read-only posture: Write is removed +// from its `tools:` before conversion and no receipt is written on that host (the watch +// ends on the completion result or as stalled; stall-detection-helpers.md). Keyed by +// runtime to the map that runtime's agent converter uses (descriptor converters +// convertClaudeToOpencodeFrontmatter, convertClaudeToKiloFrontmatter, +// convertClaudeAgentToCopilotAgent), and decided by comparing the two entries, so a map +// that later separates Write from Edit gives the receipt back with no change here. +const RECEIPT_ONLY_WRITE_AGENTS: ReadonlySet = new Set(['gsd-plan-checker']); +const AGENT_TOOL_PERMISSION_MAPS: Readonly>>> = { + opencode: claudeToOpencodePermission, + kilo: claudeToKiloAgentPermissions, + copilot: claudeToCopilotTools, +}; + +/** True when `runtime`'s agent tool map grants Write and Edit as one permission. */ +function hostMergesWriteIntoEdit(runtime: string): boolean { + if (!Object.prototype.hasOwnProperty.call(AGENT_TOOL_PERMISSION_MAPS, runtime)) return false; + const map = AGENT_TOOL_PERMISSION_MAPS[runtime]; + return typeof map.Write === 'string' && map.Write === map.Edit; +} + +/** + * Remove one tool from an agent's `tools:` frontmatter (inline comma list or block list). + * Any other shape (a quoted scalar or a flow sequence) is returned unchanged; the shipped + * receipt-only agents use the inline form, and an install test pins the emitted result. + */ +function removeAgentTool(content: string, tool: string): string { + const fence = locateFrontmatterFence(content); + if (!fence?.closed) return content; + const eol = fence.eol; + const lines = content.slice(0, fence.closingStart).split(eol); + const at = lines.findIndex((line) => /^tools:/.test(line)); + if (at === -1) return content; + const value = lines[at].replace(/^tools:[ \t]*/, ''); + const commentAt = value.search(/(^|[ \t])#/); + const list = (commentAt === -1 ? value : value.slice(0, commentAt)).trim(); + if (list) { + if (/^["'[]/.test(list)) return content; + const kept = list.split(',').map((t) => t.trim()).filter((t) => t && t !== tool); + lines[at] = `tools: ${kept.join(', ')}${commentAt === -1 ? '' : ` ${value.slice(commentAt).trim()}`}`; + } else { + for (let i = at + 1; i < lines.length && /^[ \t]*-[ \t]/.test(lines[i]);) { + if (decodeToolScalar(lines[i].replace(/^[ \t]*-[ \t]+/, '')) === tool) lines.splice(i, 1); + else i += 1; + } + } + return lines.join(eol) + content.slice(fence.closingStart); +} + +/** + * Pre-converter step (ADR-1235 §1 descriptor boundary, before the user's agent_tools + * grants are appended, so a grant the user configures is never removed): drop a + * receipt-only agent's Write on a host that would widen it to full edit (#5182). + */ +function scopeReceiptOnlyWrite(content: string, { runtime, agentName }: { runtime: string; agentName: string }): string { + if (!RECEIPT_ONLY_WRITE_AGENTS.has(agentName) || !hostMergesWriteIntoEdit(runtime)) return content; + return removeAgentTool(content, 'Write'); +} + /** * Post-converter frontmatter-extensions step (#2875 Part 2 / ADR-1235 §1 * follow-up). Driven by the runtime descriptor's @@ -4472,6 +4535,9 @@ export = { injectEffortFrontmatter, injectDisallowedToolsFrontmatter, READONLY_AGENT_DISALLOWED_TOOLS, + RECEIPT_ONLY_WRITE_AGENTS, + hostMergesWriteIntoEdit, + scopeReceiptOnlyWrite, applyAgentFrontmatterExtensions, _computePathPrefix: computePathPrefix, _withShellDefaultsPreserved: withShellDefaultsPreserved, diff --git a/tests/agent-descriptor-parity.install.test.cjs b/tests/agent-descriptor-parity.install.test.cjs index ab3eeafaf2..2f5d370a2b 100644 --- a/tests/agent-descriptor-parity.install.test.cjs +++ b/tests/agent-descriptor-parity.install.test.cjs @@ -509,7 +509,7 @@ test('agent-descriptor-parity: J3 — a runtime NOT declaring agentFrontmatterEx test('agent-descriptor-parity: J4 — disallowedTools injected only on a READONLY_AGENT_DISALLOWED_TOOLS hit', () => { const content = '---\nname: gsd-plan-checker\ndescription: x\n---\n\nBody.\n'; const hit = runtimeArtifactConversion.applyAgentFrontmatterExtensions(content, { runtime: 'claude', agentName: 'gsd-plan-checker', targetDir: null }); - assert.match(hit, /^disallowedTools: /m, 'gsd-plan-checker is a declared read-only agent — expected a disallowedTools hit'); + assert.match(hit, /^disallowedTools: Edit, MultiEdit$/m, 'gsd-plan-checker is a declared Group B report-writer (#5182) — expected a disallowedTools hit of Edit, MultiEdit'); const missContent = '---\nname: gsd-not-a-readonly-agent\ndescription: x\n---\n\nBody.\n'; const miss = runtimeArtifactConversion.applyAgentFrontmatterExtensions(missContent, { runtime: 'claude', agentName: 'gsd-not-a-readonly-agent', targetDir: null }); diff --git a/tests/agent-tools.install.test.cjs b/tests/agent-tools.install.test.cjs index c3279d5e93..e8a585a4bf 100644 --- a/tests/agent-tools.install.test.cjs +++ b/tests/agent-tools.install.test.cjs @@ -333,14 +333,119 @@ test('every installable runtime accepts a configured MCP grant without crashing test('Codex grants do not widen the generated TOML sandbox (#4032)', (t) => { const install = installRuntime(t, 'codex', { - defaults: { agent_tools: { 'gsd-plan-checker': ['Write'] } }, + defaults: { agent_tools: { 'gsd-integration-checker': ['Write'] } }, }); - const toml = fs.readFileSync(path.join(install.configDir, 'agents', 'gsd-plan-checker.toml'), 'utf8'); + const toml = fs.readFileSync(path.join(install.configDir, 'agents', 'gsd-integration-checker.toml'), 'utf8'); assert.match(toml, /^sandbox_mode = "read-only"$/m); assert.doesNotMatch(toml, /Write/, 'Codex tool availability is inherited from the parent session, not encoded in agent TOML'); }); +// #5182: gsd-plan-checker declares Write for its stall-watch return receipt only. OpenCode, +// Kilo and Copilot map Write and Edit to one `edit` permission, so there the installer drops +// that Write and the checker keeps its read-only posture; every other host keeps Write. +function checkerEditPosture(runtime, artifact) { + if (runtime === 'copilot') { + const tools = /^tools: \[(.*)\]$/m.exec(artifact); + assert.ok(tools, 'the Copilot checker carries a tools array'); + return tools[1].split(',').map((s) => s.trim().replace(/^'|'$/g, '')).includes('edit') ? 'allow' : 'deny'; + } + const edit = /^ {2}edit: (allow|deny)$/m.exec(artifact); + // OpenCode emits denials only, so an absent `edit:` entry leaves edit allowed. + return edit ? edit[1] : 'allow'; +} + +test('the plan-checker receipt Write is dropped where the host merges Write into edit (#5182)', (t) => { + let ran = 0; + for (const runtime of ['opencode', 'kilo', 'copilot']) { + for (const scope of ['local', 'global']) { + const install = installRuntime(t, runtime, { scope }); + // The host's own agents/ directory: a global install also ships the raw source + // agents under /gsd-core/agents/, which no host loads as an agent definition. + const hostAgents = path.join(install.configDir, 'agents'); + assert.ok(emittedAgentArtifacts(install, 'gsd-plan-checker', hostAgents).every((artifact) => checkerEditPosture(runtime, artifact) === 'deny'), + `${runtime} (${scope}): the checker must not hold edit`); + // Control: the same install still grants edit to an agent that declares Write. + assert.ok(emittedAgentArtifacts(install, 'gsd-verifier', hostAgents).some((artifact) => checkerEditPosture(runtime, artifact) === 'allow'), + `${runtime} (${scope}): control, gsd-verifier declares Write and keeps edit`); + ran += 1; + } + } + assert.equal(ran, 6); + // Hosts with a separate write tool keep the receipt Write. + const claude = installClaude(t).agent('gsd-plan-checker'); + assert.ok(parseTools(claude).includes('Write'), 'claude: the checker keeps Write'); + assert.match(claude, /^disallowedTools: Edit, MultiEdit$/m, 'claude: Edit and MultiEdit stay denied'); +}); + +test('a Write the user grants the plan-checker in agent_tools is kept on a merging host (#5182)', (t) => { + const install = installRuntime(t, 'kilo', { defaults: { agent_tools: { 'gsd-plan-checker': ['Write'] } } }); + const artifacts = emittedAgentArtifacts(install, 'gsd-plan-checker'); + assert.ok(artifacts.some((artifact) => checkerEditPosture('kilo', artifact) === 'allow'), + 'a configured grant is appended after the receipt-only scoping, so it survives'); +}); + +// The runtime -> map table must agree with what each host's real agent converter emits, +// so a host added later whose converter merges Write into edit cannot slip past the +// table: a converter merges them iff `Read, Write` and `Read, Edit` convert identically +// while differing from `Read` alone (a converter that emits no tool restriction at all +// leaves every tool granted either way and has nothing to scope). +test('exactly the hosts whose agent converter merges Write into edit drop the receipt Write (#5182)', () => { + const conversion = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); + const capabilitiesDir = path.join(REPO_ROOT, 'capabilities'); + const agent = (tools) => `---\nname: gsd-probe\ndescription: d\ntools: ${tools}\n---\n\nBody.\n`; + const table = []; + const observed = []; + let converters = 0; + for (const name of fs.readdirSync(capabilitiesDir).sort()) { + const file = path.join(capabilitiesDir, name, 'capability.json'); + if (!fs.existsSync(file)) continue; + if (conversion.hostMergesWriteIntoEdit(name)) table.push(name); + const names = new Set(); + (function walk(node) { + if (!node || typeof node !== 'object') return; + if (node.kind === 'agents' && typeof node.converter === 'string') names.add(node.converter); + for (const key of Object.keys(node)) walk(node[key]); + })(JSON.parse(fs.readFileSync(file, 'utf8'))); + for (const converterName of names) { + const convert = conversion[converterName]; + assert.equal(typeof convert, 'function', `${name}: descriptor converter ${converterName} is exported`); + const run = (tools) => (/Frontmatter$/.test(converterName) ? convert(agent(tools), { isAgent: true }) : convert(agent(tools), false)); + const write = run('Read, Write'); + converters += 1; + if (write === run('Read, Edit') && write !== run('Read')) observed.push(name); + } + } + assert.ok(converters > 10, `the descriptor agent converters were exercised (${converters})`); + assert.deepStrictEqual(table, observed, 'the scoping table names exactly the hosts whose converter merges Write into edit'); + assert.deepStrictEqual(table, ['copilot', 'kilo', 'opencode']); + assert.equal(conversion.hostMergesWriteIntoEdit('__proto__'), false); + assert.equal(conversion.hostMergesWriteIntoEdit('constructor'), false); + assert.deepStrictEqual([...conversion.RECEIPT_ONLY_WRITE_AGENTS], ['gsd-plan-checker']); +}); + +test('receipt-only Write removal handles inline, commented and block tools lists, and nothing else (#5182)', () => { + const { scopeReceiptOnlyWrite } = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); + const on = { runtime: 'kilo', agentName: 'gsd-plan-checker' }; + const cases = [ + ['---\nname: a\ntools: Read, Write, Bash\n---\nWrite body\n', '---\nname: a\ntools: Read, Bash\n---\nWrite body\n'], + ['---\r\ntools: Write, Read # note\r\n---\r\nb', '---\r\ntools: Read # note\r\n---\r\nb'], + ['---\ntools:\n - Read\n - Write\n - "Write"\n - Bash\ncolor: x\n---\n', '---\ntools:\n - Read\n - Bash\ncolor: x\n---\n'], + ['---\ntools: Read, WriteFile\n---\n', '---\ntools: Read, WriteFile\n---\n'], + ['---\ntools: [Read, Write]\n---\n', '---\ntools: [Read, Write]\n---\n'], + ['no frontmatter tools: Write\n', 'no frontmatter tools: Write\n'], + ]; + let ran = 0; + for (const [input, expected] of cases) { + assert.equal(scopeReceiptOnlyWrite(input, on), expected, JSON.stringify(input)); + ran += 1; + } + assert.equal(ran, cases.length); + const inline = cases[0][0]; + assert.equal(scopeReceiptOnlyWrite(inline, { runtime: 'claude', agentName: 'gsd-plan-checker' }), inline, 'a host with a separate Write keeps it'); + assert.equal(scopeReceiptOnlyWrite(inline, { runtime: 'kilo', agentName: 'gsd-verifier' }), inline, 'only receipt-only agents are scoped'); +}); + test('hostile values fail closed while inline Claude tools remain valid tokens (#4032)', (t) => { const rejected = [ null, 1, '', ' ', 'mcp__bad,comma', 'mcp__bad\0nul', 'mcp__bad\nline', 'mcp__bad\u0085nel', 'mcp__bad\u2028line', diff --git a/tests/codex-config-agents.test.cjs b/tests/codex-config-agents.test.cjs index adc9726ff8..0e37db14ec 100644 --- a/tests/codex-config-agents.test.cjs +++ b/tests/codex-config-agents.test.cjs @@ -464,7 +464,7 @@ describe('installCodexConfig (integration)', () => { const checkerToml = fs.readFileSync(path.join(agentsDir, 'gsd-plan-checker.toml'), 'utf8'); assert.ok(checkerToml.includes('name = "gsd-plan-checker"'), 'plan-checker has name'); - assert.ok(checkerToml.includes('sandbox_mode = "read-only"'), 'plan-checker is read-only'); + assert.ok(checkerToml.includes('sandbox_mode = "workspace-write"'), 'plan-checker is workspace-write (#5182 receipt write)'); }); // PATHS-01: no ~/.claude references should leak into generated .toml files (#2320) diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index 95b1908813..3b0c5544c8 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -509,15 +509,15 @@ color: yellow assert.ok(result.includes('sandbox_mode = "workspace-write"'), 'has workspace-write'); }); - test('sets read-only for plan-checker', () => { + test('sets read-only for integration-checker', () => { const checker = `--- -name: gsd-plan-checker -description: Checks plans +name: gsd-integration-checker +description: Checks integration tools: Read, Grep, Glob --- -You check plans.`; - const result = generateCodexAgentToml('gsd-plan-checker', checker); +You check integration.`; + const result = generateCodexAgentToml('gsd-integration-checker', checker); assert.ok(result.includes('sandbox_mode = "read-only"'), 'has read-only'); }); @@ -1139,7 +1139,7 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { 'gsd-nyquist-auditor': 'workspace-write', 'gsd-pattern-mapper': 'workspace-write', 'gsd-phase-researcher': 'workspace-write', - 'gsd-plan-checker': 'read-only', + 'gsd-plan-checker': 'workspace-write', 'gsd-planner': 'workspace-write', 'gsd-project-researcher': 'workspace-write', 'gsd-research-synthesizer': 'workspace-write', @@ -1234,7 +1234,13 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { }); test('T21 mappedRolesDeriveToTheirFormerValue: every former CODEX_AGENT_SANDBOX entry (11) derives to the identical value from its real tool contract', () => { + // #5182 is the one deliberate departure (pinned by its own test below); every + // other former entry must still derive identically. + const DELIBERATE_DEPARTURES = new Set(['gsd-plan-checker']); + let compared = 0; for (const [role, formerValue] of Object.entries(PRE_3897_CODEX_AGENT_SANDBOX)) { + if (DELIBERATE_DEPARTURES.has(role)) continue; + compared++; const derivesWorkspaceWrite = declaresWriteOrEdit(realAgentToolsRaw(role)); const derived = derivesWorkspaceWrite ? 'workspace-write' : 'read-only'; assert.equal( @@ -1244,6 +1250,7 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { ); } assert.equal(Object.keys(PRE_3897_CODEX_AGENT_SANDBOX).length, 11); + assert.equal(compared, 10, 'every former entry except the #5182 departure is compared'); }); test('T22 nonWritingFallbackRoleDerivesReadOnly: a fallback role declaring neither Write nor Edit derives read-only (S2)', () => { @@ -1975,12 +1982,19 @@ describe('CODEX_AGENT_SANDBOX (deleted map, derivation regression baseline)', () }); test('read-only agents still derive read-only', () => { - const readOnlyAgents = ['gsd-plan-checker', 'gsd-integration-checker']; + const readOnlyAgents = ['gsd-integration-checker']; for (const name of readOnlyAgents) { assert.strictEqual(PRE_3897_CODEX_AGENT_SANDBOX[name], 'read-only', `${name} baseline is read-only`); assert.strictEqual(realDerivedSandboxMode(name), 'read-only', `${name} still derives read-only`); } }); + + // #5182: the one deliberate departure from the pre-#3897 baseline. The checker + // declares Write so it can write its stall-watch return receipt on Codex. + test('gsd-plan-checker moved from read-only to workspace-write (#5182)', () => { + assert.strictEqual(PRE_3897_CODEX_AGENT_SANDBOX['gsd-plan-checker'], 'read-only', 'baseline was read-only'); + assert.strictEqual(realDerivedSandboxMode('gsd-plan-checker'), 'workspace-write', 'now derives workspace-write'); + }); }); // ─── generateCodexConfigBlock ─────────────────────────────────────────────────── diff --git a/tests/fixtures/compact-content-benchmark-baseline.json b/tests/fixtures/compact-content-benchmark-baseline.json index 1bb5ed3aa8..c9705f826e 100644 --- a/tests/fixtures/compact-content-benchmark-baseline.json +++ b/tests/fixtures/compact-content-benchmark-baseline.json @@ -28,9 +28,9 @@ "reductionPct": 13.37 }, "plan-phase": { - "offTokens": 28789, - "onTokens": 25521, - "reductionPct": 11.35 + "offTokens": 29161, + "onTokens": 25659, + "reductionPct": 12.01 }, "verify-work": { "offTokens": 15523, @@ -39,8 +39,8 @@ } }, "aggregate": { - "offTokens": 113015, - "onTokens": 95394, - "reductionPct": 15.59 + "offTokens": 113387, + "onTokens": 95532, + "reductionPct": 15.75 } } diff --git a/tests/install.test.cjs b/tests/install.test.cjs index b0cdabb22a..6fa083ac4e 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -1734,7 +1734,7 @@ describe('cleanupWindsurfLegacyDevinSkills — removes pre-#1615 skill artifacts // #767 — must mirror READONLY_AGENT_DISALLOWED_TOOLS in bin/install.js. // If you change the map there, update this too (the parity test will catch drift). const READONLY_AGENT_DISALLOWED_TOOLS_767 = { - 'gsd-plan-checker': 'Write, Edit, MultiEdit', + 'gsd-plan-checker': 'Edit, MultiEdit', 'gsd-integration-checker': 'Write, Edit, MultiEdit', 'gsd-ui-checker': 'Write, Edit, MultiEdit', 'gsd-verifier': 'Edit, MultiEdit', @@ -1743,8 +1743,9 @@ const READONLY_AGENT_DISALLOWED_TOOLS_767 = { 'gsd-ui-auditor': 'Edit, MultiEdit', }; -const GROUP_A_767 = ['gsd-plan-checker', 'gsd-integration-checker', 'gsd-ui-checker']; -const GROUP_B_767 = ['gsd-verifier', 'gsd-doc-verifier', 'gsd-eval-auditor', 'gsd-ui-auditor']; +// #5182: gsd-plan-checker writes its stall-watch return receipt, so it is a Group B report-writer. +const GROUP_A_767 = ['gsd-integration-checker', 'gsd-ui-checker']; +const GROUP_B_767 = ['gsd-plan-checker', 'gsd-verifier', 'gsd-doc-verifier', 'gsd-eval-auditor', 'gsd-ui-auditor']; const REPO_ROOT_767 = path.resolve(__dirname, '..'); const SOURCE_AGENTS_DIR_767 = path.join(REPO_ROOT_767, 'agents'); diff --git a/tests/plan-phase-stall-detection.test.cjs b/tests/plan-phase-stall-detection.test.cjs index d878374847..374095a94d 100644 --- a/tests/plan-phase-stall-detection.test.cjs +++ b/tests/plan-phase-stall-detection.test.cjs @@ -48,6 +48,7 @@ const os = require('node:os'); const path = require('node:path'); const fc = require('fast-check'); const { cleanup, readFileNormalized, readWorkflowCombined, runGsdTools } = require('./helpers.cjs'); +const { splitLines } = require('../gsd-core/bin/lib/text-lines.cjs'); const REPO_ROOT = path.join(__dirname, '..'); const PLAN_PHASE_PATH = path.join(REPO_ROOT, 'gsd-core', 'workflows', 'plan-phase.md'); @@ -330,22 +331,22 @@ describe('bug #2650 plan-phase stall detection — gsd_stall_watch (real executi // before — that part was never the problem and correctly keeps Windows // paths and the injection-guard payload intact; only the transport of // the script itself changes. - function runWatch(intervalMinutes, thresholdMinutes, dispatchTs, outputFile, artifactGlob, markers) { + function runWatch(intervalMinutes, thresholdMinutes, dispatchTs, receiptFile, artifactGlob, markers) { const overrides = `PLANNER_STALL_INTERVAL_MINUTES=${intervalMinutes}\nPLANNER_STALL_THRESHOLD_MINUTES=${thresholdMinutes}\n`; - const call = `gsd_stall_watch ${JSON.stringify(String(dispatchTs))} ${JSON.stringify(outputFile)} ${JSON.stringify(artifactGlob)}` + + const call = `gsd_stall_watch ${JSON.stringify(String(dispatchTs))} ${JSON.stringify(receiptFile)} ${JSON.stringify(artifactGlob)}` + markers.map((m) => ` ${JSON.stringify(m)}`).join(''); const script = `${helpersBash}\n${overrides}${call}\n`; return runBashScript(script, []); } - test('marker present in the real output file (via real grep, interval=0 so sleep is instant) -> marker_received', (t) => { + test('marker line present in the receipt (interval=0 so sleep is instant) -> marker_received', (t) => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2650-watch-')); t.after(() => cleanup(tmp)); - const outputFile = path.join(tmp, 'agent-output.txt'); - fs.writeFileSync(outputFile, 'some agent output\n## PLANNING COMPLETE\nmore text\n'); + const receiptFile = path.join(tmp, 'receipt.md'); + fs.writeFileSync(receiptFile, 'some agent output\n## PLANNING COMPLETE\nmore text\n'); const glob = `${tmp.replace(/\\/g, '/')}/*-PLAN.md`; const now = Math.floor(Date.now() / 1000); - const result = runWatch(0, 10, now, outputFile, glob, ['## PLANNING COMPLETE']); + const result = runWatch(0, 10, now, receiptFile, glob, ['## PLANNING COMPLETE']); assert.equal(result.status, 0, result.stderr); assert.equal(result.stdout.trim(), 'marker_received'); }); @@ -701,7 +702,7 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch const section = workflow.slice(idx, nextSectionIdx === -1 ? undefined : nextSectionIdx); assert.match(section, /run_in_background\s*=\s*true/, 'standard planner spawn must set run_in_background=true'); assert.match(section, /gsd_stall_watch/, 'standard planner spawn must invoke the bounded stall watcher'); - assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{outputFile\}"/, 'standard planner spawn must bind {outputFile} into the stall watcher call, not a dead bash variable'); + assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/, 'standard planner spawn must bind {receipt} (#5182) into the stall watcher call, not a dead bash variable'); }); test('plan-phase.md points at the lazily-loaded chunked-planning-mode.md step file (8.5, #2993)', () => { @@ -725,7 +726,7 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch const section = chunkedDoc.slice(idx, nextSectionIdx === -1 ? undefined : nextSectionIdx); assert.match(section, /run_in_background\s*=\s*true/, 'chunked outline spawn must set run_in_background=true'); assert.match(section, /gsd_stall_watch/, 'chunked outline spawn must invoke the bounded stall watcher'); - assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{outputFile\}"/, 'chunked outline spawn must bind {outputFile} into the stall watcher call, not a dead bash variable'); + assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/, 'chunked outline spawn must bind {receipt} (#5182) into the stall watcher call, not a dead bash variable'); }); test('chunked per-plan spawn (8.5.2) dispatches with run_in_background=true and calls gsd_stall_watch', () => { @@ -740,7 +741,7 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch const section = chunkedDoc.slice(idx); assert.match(section, /run_in_background\s*=\s*true/, 'chunked per-plan spawn must set run_in_background=true'); assert.match(section, /gsd_stall_watch/, 'chunked per-plan spawn must invoke the bounded stall watcher'); - assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{outputFile\}"/, 'chunked per-plan spawn must bind {outputFile} into the stall watcher call, not a dead bash variable'); + assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/, 'chunked per-plan spawn must bind {receipt} (#5182) into the stall watcher call, not a dead bash variable'); }); test('plan-checker spawn (step 10) dispatches with run_in_background=true and calls gsd_stall_watch', () => { @@ -750,7 +751,7 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch const section = workflow.slice(idx, nextSectionIdx === -1 ? undefined : nextSectionIdx); assert.match(section, /run_in_background\s*=\s*true/, 'plan-checker spawn must set run_in_background=true'); assert.match(section, /gsd_stall_watch/, 'plan-checker spawn must invoke the bounded stall watcher'); - assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{outputFile\}"/, 'plan-checker spawn must bind {outputFile} into the stall watcher call — this is the ONLY completion signal on a clean PASS, since a passing checker touches no *-PLAN.md files'); + assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/, 'plan-checker spawn must bind {receipt} (#5182) into the stall watcher call — this is the ONLY watch-visible completion signal on a clean PASS, since a passing checker touches no *-PLAN.md files'); }); test('revision-loop planner respawn (step 12) dispatches with run_in_background=true and calls gsd_stall_watch', () => { @@ -760,7 +761,7 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch const section = workflow.slice(idx, nextSectionIdx === -1 ? undefined : nextSectionIdx); assert.match(section, /run_in_background\s*=\s*true/, 'revision-loop planner respawn must set run_in_background=true'); assert.match(section, /gsd_stall_watch/, 'revision-loop planner respawn must invoke the bounded stall watcher'); - assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{outputFile\}"/, 'revision-loop planner respawn must bind {outputFile} into the stall watcher call, not a dead bash variable'); + assert.match(section, /gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/, 'revision-loop planner respawn must bind {receipt} (#5182) into the stall watcher call, not a dead bash variable'); }); test('no spawn site references an unbound $PLANNER_OUTPUT_FILE / $CHECKER_OUTPUT_FILE bash variable', () => { @@ -773,7 +774,8 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch // that PASSES touches no *-PLAN.md files, so it has NO working completion // signal at all without the marker path — a healthy, already-succeeded // checker would be reported as stalled. The fix replaces the dead bash - // variable with the `{outputFile}` orchestrator-substitution token (the + // variable with an orchestrator-substitution token (#5182 later replaced + // `{outputFile}` with the GSD-owned `{receipt}`; the token convention is the // same convention docs-update.md:471 already uses for a real // run_in_background=true Agent() return). This test proves the dead // variable name is gone from every spawn site, not just that @@ -799,9 +801,9 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch // test noticing (mirrors tests/plan-phase-drift-guard.test.cjs's #913 // ORCHESTRATOR RULE label count, which already does this). const combined = readWorkflowCombined(PLAN_PHASE_PATH); - const callCount = (combined.match(/gsd_stall_watch\s+"\$TS"\s+"\{outputFile\}"/g) || []).length; + const callCount = (combined.match(/gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/g) || []).length; assert.equal(callCount, 5, - `expected exactly 5 gsd_stall_watch "$TS" "{outputFile}" spawn-site invocations across plan-phase.md + steps/*.md, found ${callCount}`); + `expected exactly 5 gsd_stall_watch "$TS" "{receipt}" spawn-site invocations across plan-phase.md + steps/*.md, found ${callCount}`); }); test('all five spawn classes gate background surveillance and retain a runtime-native blocking result path', () => { @@ -832,20 +834,25 @@ describe('bug #2650 plan-phase — all five planner/plan-checker spawns dispatch } }); - test('step 7.99 documents that {outputFile} must be bound from the real Agent() return (not passed literally)', () => { + test('step 7.99 documents that {receipt} must be bound from gsd_receipt_path before dispatch (not passed literally) (#5182)', () => { const idx = workflow.indexOf('## 7.99. Bounded Stall-Detection Helpers'); assert.notEqual(idx, -1); const nextSectionIdx = workflow.indexOf('## 8. Spawn gsd-planner Agent', idx); const section = workflow.slice(idx, nextSectionIdx === -1 ? undefined : nextSectionIdx); - assert.match(section, /\{outputFile\}/, 'step 7.99 must mention {outputFile} so a reader knows it is a binding token, not literal text'); - // The full binding contract (docs-update.md precedent, why a bash variable - // does not work, and the plan-checker completion-signal implication) lives - // in the lazily-loaded reference file to stay under the PRE_PHASE6 cap — - // verify it is actually there, not just gestured at. + assert.match(section, /`\{receipt\}` = `gsd_receipt_path "\$\{PHASE_DIR\}" "\$\{PROJECT_ROOT\}"`/, 'step 7.99 must bind {receipt} from gsd_receipt_path, anchored on the project root (review 5563209291 Minor 1)'); + // Every bind site passes the project root init resolved, never the cwd default. + const binds = [...`${workflow}\n${readChunkedPlanningMode()}\n${readStallHelpersDoc()}`.matchAll(/`gsd_receipt_path "\$\{PHASE_DIR\}" [^`]*`/g)].map((m) => m[0]); + assert.ok(binds.length >= 4, `the bind sites are found (${binds.length})`); + for (const bind of binds) assert.match(bind, /"\$\{PROJECT_ROOT\}"`$/, `${bind}: passes the project root`); + assert.match(workflow, /`project_root` \(`PROJECT_ROOT`\)/, 'plan-phase parses project_root from the init JSON'); + // The full binding contract lives in the lazily-loaded reference file to stay + // under the PRE_PHASE6 cap — verify it is actually there, not just gestured at. + // #5182 replaced the {outputFile} binding (a host transcript that already holds + // the prompt and definition text) with the GSD-owned receipt. const helpersDoc = readStallHelpersDoc(); - assert.match(helpersDoc, /\{outputFile\}/, 'stall-detection-helpers.md must explain the {outputFile} binding contract'); - assert.match(helpersDoc, /docs-update\.md/i, 'stall-detection-helpers.md must cite the docs-update.md precedent for {outputFile} substitution'); - assert.match(helpersDoc, /plan-checker/i, 'stall-detection-helpers.md must explain why binding {outputFile} is load-bearing for the plan-checker spawn specifically'); + assert.match(helpersDoc, /substitutes the printed absolute path for `\{receipt\}`/, 'stall-detection-helpers.md must explain the {receipt} binding contract'); + assert.match(helpersDoc, /\*\*Plan-checker receipt:\*\* a checker that PASSES touches no `\*-PLAN\.md`, so its receipt\s+is its only completion signal for this watch/, + 'stall-detection-helpers.md must explain why the receipt is load-bearing for the plan-checker spawn specifically'); }); test('stall surveillance is not gated behind the teams-status guard (AC2)', () => { @@ -1012,3 +1019,1003 @@ describe('#5451: every stall-detection gate names its config key', () => { }); }); +// The #5182 rows need only the shipped FUNCTION definitions, not the whole fence: +// the fence opens with the runtime-locator preamble and two `gsd_run query +// config-get` lines, i.e. two Node spawns per bash call (about 1 s each on the +// Windows runners). Run through every #5182 row, that preamble cost 169.5 s on +// Windows CI shard 1/3, enough to starve a concurrent file into a spawn timeout. +// Slicing from the first function keeps the executed code the shipped code; +// config resolution stays covered by the #2650 rows above, which run the full fence. +function extractStallFunctionsBash() { + const fence = extractStallHelpersBash(); + const at = fence.indexOf('gsd_stall_should_recover() {'); + if (at === -1) throw new Error('extractStallFunctionsBash: gsd_stall_should_recover() not found in the helpers fence'); + return fence.slice(at); +} + +// ───────────────────────────────────────────────────────────────────────────── +// #5182 — the stall watch must observe the spawned agent's REAL return, not text +// that merely mentions a marker. +// +// Pre-fix, every watch call site bound `{outputFile}` (a host-specific handle; on +// Claude Code the subagent JSONL transcript) and `gsd_stall_watch` grepped the +// WHOLE file for the markers. The transcript holds the agent's prompt and its +// definition snapshot before the agent replies, so the planner/checker watch +// reported `marker_received` at its first check; the revision watch passed no +// markers, so `stalled` was its only exit; and a host with no output file could +// only ever reach `stalled`. +// +// The fix binds a GSD-owned, per-dispatch return receipt (`{receipt}`, printed by +// `gsd_receipt_path`) that the agent writes as its LAST action, and matches a +// marker only at the START of a receipt line (`gsd_return_marker`), which is also +// what step 11 routes on. These rows drive the SHIPPED bash and read the SHIPPED +// call sites (markers are extracted from plan-phase.md, not hand-copied), so a +// call site that drops its markers or rebinds a host file fails here. +// ───────────────────────────────────────────────────────────────────────────── + +describe('bug #5182 — the stall watch observes a GSD-owned return receipt, never prompt or transcript text', () => { + const CHECKER_SECTION = ['## 10. Spawn gsd-plan-checker Agent', '## 11. Handle Checker Return']; + const REVISION_SECTION = ['## 12. Revision Loop', '## 12.5. Plan Bounce']; + const PLANNER_SECTION = ['## 8. Spawn gsd-planner Agent', '## 9. Handle Planner Return']; + + function sectionOf(doc, [start, end]) { + const at = doc.indexOf(start); + assert.notEqual(at, -1, `missing section heading: ${start}`); + const stop = doc.indexOf(end, at); + return doc.slice(at, stop === -1 ? undefined : stop); + } + + // Parse the one `gsd_stall_watch "$TS" "" "m1" "m2" ...` call in a + // section: returns the bound token and the marker list exactly as shipped. + function watchCallOf(section) { + const m = section.match(/gsd_stall_watch\s+"\$TS"\s+"(\{[A-Za-z]+\})"\s+\S+?((?:\s+"[^"]*")*)\)/); + assert.ok(m, 'section must contain one gsd_stall_watch "$TS" "" [markers...] call'); + const markers = [...m[2].matchAll(/"([^"]*)"/g)].map((x) => x[1]); + return { token: m[1], markers }; + } + + // An injected clock: every row reads "now" from this constant through a stubbed + // `date +%s`, never from the wall clock, so threshold boundaries are exact. + const NOW = 1800000000; + + // Run the shipped helpers with `sleep` stubbed (no real interval wait), `date +%s` + // pinned to NOW and the interval/threshold pinned, then run `call`. Returns + // trimmed stdout. + function runHelpers(call, { interval = 5, threshold = 10 } = {}) { + const helpersBash = extractStallFunctionsBash(); + const clock = `date() { if [ "$1" = "+%s" ]; then echo ${NOW}; else command date "$@"; fi; }`; + const script = `${helpersBash}\nsleep() { :; }\n${clock}\nPLANNER_STALL_INTERVAL_MINUTES=${interval}\nPLANNER_STALL_THRESHOLD_MINUTES=${threshold}\n${call}\n`; + const result = runBashScript(script, []); + assert.equal(result.status, 0, result.stderr); + return result.stdout.trim(); + } + + const q = (s) => JSON.stringify(String(s)); + + function watch(dispatchTs, file, glob, markers, opts) { + return runHelpers(`gsd_stall_watch ${q(dispatchTs)} ${q(file)} ${q(glob)}${markers.map((x) => ` ${q(x)}`).join('')}`, opts); + } + + function fixture(t) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5182-')); + t.after(() => cleanup(dir)); + const fwd = dir.replace(/\\/g, '/'); + // A Claude-Code-shaped transcript of a checker that has NOT returned yet: + // record 1 is the prompt (lists both markers), then a definition snapshot + // (quotes the planner's own marker heading). Neither is a return. + const transcript = path.join(dir, 'agent-transcript.jsonl'); + fs.writeFileSync(transcript, [ + JSON.stringify({ type: 'user', message: { role: 'user', content: '... Return:\n- ## VERIFICATION PASSED — all checks pass\n- ## ISSUES FOUND — structured issue list\n' } }), + JSON.stringify({ type: 'attachment', attachment: { type: 'prompt_snapshot', content: '## Return Markers\n\n```markdown\n## PLANNING COMPLETE\n```\n' } }), + ].join('\n') + '\n'); + return { dir, fwd, transcript, noPlans: `${fwd}/none-*-PLAN.md`, receipt: path.join(dir, 'receipt.md') }; + } + + test('AC1: markers present only in a prompt or definition snapshot yield waiting, not marker_received', (t) => { + const f = fixture(t); + const checker = watchCallOf(sectionOf(readPlanPhase(), CHECKER_SECTION)); + assert.equal(watch(NOW, f.transcript, f.noPlans, checker.markers), 'waiting', + 'a file whose only marker text is the prompt/definition must not count as the agent returning'); + const planner = watchCallOf(sectionOf(readPlanPhase(), PLANNER_SECTION)); + assert.equal(watch(NOW, f.transcript, f.noPlans, planner.markers), 'waiting'); + }); + + test('AC1 wiring: all five watch sites bind {receipt}; none binds the host {outputFile}', () => { + const combined = readWorkflowCombined(PLAN_PHASE_PATH); + const receiptCalls = (combined.match(/gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/g) || []).length; + assert.equal(receiptCalls, 5, `expected 5 gsd_stall_watch "$TS" "{receipt}" call sites, found ${receiptCalls}`); + assert.doesNotMatch(combined, /gsd_stall_watch\s+"\$TS"\s+"\{outputFile\}"/, + 'no watch may read a host-specific output file (undocumented on Claude Code, absent on other hosts)'); + }); + + test('every watched spawn prompt hands the agent its receipt path', () => { + const plan = readPlanPhase(); + const chunked = readChunkedPlanningMode(); + const sections = [ + ['planner', sectionOf(plan, PLANNER_SECTION)], + ['checker', sectionOf(plan, CHECKER_SECTION)], + ['revision', sectionOf(plan, REVISION_SECTION)], + ['chunked outline', chunked.slice(chunked.indexOf('### 8.5.1 Outline Phase'), chunked.indexOf('### 8.5.2 Per-Plan Tasks'))], + ['chunked per-plan', chunked.slice(chunked.indexOf('### 8.5.2 Per-Plan Tasks'))], + ]; + for (const [label, section] of sections) { + assert.match(section, /\{receipt\}<\/return_receipt>/, `${label}: prompt must carry {receipt}`); + } + // The binding itself: once at step 7.99 for plan-phase.md's three sites, and at + // each chunked site. + assert.match(sectionOf(plan, ['## 7.99.', '## 8. ']), /gsd_receipt_path "\$\{PHASE_DIR\}" /); + for (const [label, section] of sections.slice(3)) { + assert.match(section, /gsd_receipt_path "\$\{PHASE_DIR\}"/, `${label}: must bind {receipt} from gsd_receipt_path`); + } + }); + + test('AC2: a checker receipt of ## ISSUES FOUND routes to the revision step, never the pass step', (t) => { + const f = fixture(t); + const plan = readPlanPhase(); + const checker = watchCallOf(sectionOf(plan, CHECKER_SECTION)); + assert.deepEqual(checker.markers, ['## VERIFICATION PASSED', '## ISSUES FOUND']); + fs.writeFileSync(f.receipt, '## ISSUES FOUND\n'); + const markerArgs = checker.markers.map((x) => ` ${q(x)}`).join(''); + assert.equal(watch(NOW - 120, f.receipt, f.noPlans, checker.markers), 'marker_received'); + assert.equal(runHelpers(`gsd_return_marker ${q(f.receipt)}${markerArgs}`), '## ISSUES FOUND'); + fs.writeFileSync(f.receipt, '## VERIFICATION PASSED\n'); + assert.equal(runHelpers(`gsd_return_marker ${q(f.receipt)}${markerArgs}`), '## VERIFICATION PASSED'); + // The routing marker comes from the one matcher over the receipt (helpers doc, + // loaded at 7.99); step 11 sends ISSUES FOUND to step 12. + assert.match(readStallHelpersDoc(), /gsd_return_marker "\{receipt\}"/); + const step11 = sectionOf(plan, ['## 11. Handle Checker Return', '## 11a.']); + assert.match(step11, /## ISSUES FOUND[^\n]*step 12/, 'ISSUES FOUND must route to step 12'); + }); + + test('AC3: a finished revision yields marker_received within one poll interval', (t) => { + const f = fixture(t); + const revision = watchCallOf(sectionOf(readPlanPhase(), REVISION_SECTION)); + assert.ok(revision.markers.includes('## REVISION COMPLETE'), 'the revision watch must pass the revision success marker'); + assert.ok(revision.markers.includes('## REVISION_CONFLICT'), 'the revision watch must pass the conflict marker'); + // The revision finished and rewrote a plan during the first interval. + fs.writeFileSync(path.join(f.dir, '01-PLAN.md'), '# plan\n'); + fs.writeFileSync(f.receipt, '## REVISION COMPLETE\n'); + assert.equal(watch(NOW - 5 * 60, f.receipt, `${f.fwd}/*-PLAN.md`, revision.markers, { interval: 5 }), 'marker_received'); + fs.writeFileSync(f.receipt, '## REVISION_CONFLICT\n'); + assert.equal(watch(NOW - 5 * 60, f.receipt, `${f.fwd}/*-PLAN.md`, revision.markers, { interval: 5 }), 'marker_received'); + }); + + // Preservation row (green on next too): the receipt must not weaken the stall exit. + // Boundary triple on the 10-minute threshold (600 s), against the injected clock. + test('AC4: a spawn that never writes its receipt is stalled from the threshold on (limit-1 / limit / limit+1)', (t) => { + const f = fixture(t); + const LIMIT_SECONDS = 10 * 60; + for (const section of [PLANNER_SECTION, CHECKER_SECTION, REVISION_SECTION]) { + const call = watchCallOf(sectionOf(readPlanPhase(), section)); + assert.equal(watch(NOW - (LIMIT_SECONDS - 1), f.receipt, f.noPlans, call.markers, { threshold: 10 }), 'waiting', 'limit-1'); + assert.equal(watch(NOW - LIMIT_SECONDS, f.receipt, f.noPlans, call.markers, { threshold: 10 }), 'stalled', 'limit'); + assert.equal(watch(NOW - (LIMIT_SECONDS + 1), f.receipt, f.noPlans, call.markers, { threshold: 10 }), 'stalled', 'limit+1'); + } + }); + + // The call-site half of AC5 (all five sites bind {receipt}, none {outputFile}) + // is the "AC1 wiring" row above; this row is the behavioral half. + test('AC5: a host transcript or a missing host output is never a return; only the receipt ends the wait', (t) => { + const f = fixture(t); + const checker = watchCallOf(sectionOf(readPlanPhase(), CHECKER_SECTION)); + assert.ok(fs.readFileSync(f.transcript, 'utf8').includes('## VERIFICATION PASSED'), + 'precondition: the transcript contains the checker marker text, so a watch that searched it for that text would find it'); + // Argument 2 given each host's output in place of a receipt: neither one is + // treated as a return, before or past the threshold. + const missing = path.join(f.dir, 'no-such-output'); + for (const hostOutput of [f.transcript, missing]) { + const host = path.basename(hostOutput); + assert.equal(watch(NOW - 60, hostOutput, f.noPlans, checker.markers), 'waiting', `${host}: before the threshold`); + assert.equal(watch(NOW - 11 * 60, hostOutput, f.noPlans, checker.markers), 'stalled', `${host}: past the threshold`); + } + // Only the receipt ends the wait. + fs.writeFileSync(f.receipt, '## VERIFICATION PASSED\n'); + assert.equal(watch(NOW - 60, f.receipt, f.noPlans, checker.markers), 'marker_received'); + }); + + test('gsd_return_marker: line-start literal match, CR-tolerant, empty on anything else', (t) => { + const f = fixture(t); + const m = ` ${q('## PLAN COMPLETE')} ${q('## PLANNING COMPLETE')} ${q('## ⚠ Source Audit')}`; + const run = (content) => { + cleanup(f.receipt); + if (content !== null) fs.writeFileSync(f.receipt, content); + return runHelpers(`gsd_return_marker ${q(f.receipt)}${m}`); + }; + assert.equal(run(null), '', 'missing receipt -> no marker, exit 0'); + assert.equal(run(''), '', 'empty receipt -> no marker'); + assert.equal(run('## PLANNING COMPLETE\r\n'), '## PLANNING COMPLETE', 'CRLF receipt still matches'); + assert.equal(run('## PLANNING COMPLETE (3 plans)\n'), '## PLANNING COMPLETE', 'trailing text after the marker matches'); + assert.equal(run('## PLAN COMPLETE\n'), '## PLAN COMPLETE', '## PLAN COMPLETE is not confused with ## PLANNING COMPLETE'); + assert.equal(run('## ⚠ Source Audit\n'), '## ⚠ Source Audit', 'non-ASCII marker matches literally'); + assert.equal(run(' ## PLANNING COMPLETE\n'), '', 'an indented (quoted) marker does not match'); + assert.equal(run('Emit ## PLANNING COMPLETE when done\n'), '', 'a marker mid-line does not match'); + assert.equal(run(fs.readFileSync(f.transcript, 'utf8')), '', 'JSONL transcript records never match'); + assert.equal(run('note\n## PLAN COMPLETE\n## PLANNING COMPLETE\n'), '## PLAN COMPLETE', 'first marker line wins'); + assert.equal(run('## PLANNING COMPLETE'), '## PLANNING COMPLETE', 'a receipt with no final newline still matches'); + assert.equal(run('\uFEFF## PLANNING COMPLETE\n'), '## PLANNING COMPLETE', 'a leading UTF-8 BOM is tolerated'); + assert.equal(run('## PLAN COMPLETED\n'), '', 'a marker must end at a word boundary'); + cleanup(f.receipt); + fs.mkdirSync(f.receipt); + assert.equal(runHelpers(`gsd_return_marker ${q(f.receipt)}${m}`), '', 'a directory at the receipt path -> no marker'); + }); + + test('gsd_return_marker reads at most the first 64 lines of the receipt', (t) => { + const f = fixture(t); + const m = ` ${q('## PLANNING COMPLETE')}`; + const filler = (n) => 'note\n'.repeat(n); + const run = (content) => { + fs.writeFileSync(f.receipt, content); + return runHelpers(`gsd_return_marker ${q(f.receipt)}${m}`); + }; + assert.equal(run(`## PLANNING COMPLETE\n${filler(500)}`), '## PLANNING COMPLETE', 'a marker on line 1 of a long receipt is found'); + assert.equal(run(`${filler(63)}## PLANNING COMPLETE\n${filler(10)}`), '## PLANNING COMPLETE', 'a marker on line 64 is found'); + assert.equal(run(`${filler(64)}## PLANNING COMPLETE\n`), '', 'a marker only on line 65 is past the cap'); + }); + + // ── fast-check properties (RULESET.TESTS: a discriminating property per parser / + // sanitizer). Each property run is ONE bash invocation over a whole batch of + // generated cases, written to files (never argv: Windows MSYS re-unescapes argv), + // so the shrinker still works on the case array while the spawn cost stays low. + + // Runs `gsd_return_marker` for each [marker, receiptContent] case; returns the + // printed marker ('' for none) per case, in order. + function returnMarkerBatch(dir, cases) { + const caseDir = fs.mkdtempSync(path.join(dir, 'rm-')); + cases.forEach(([marker, content], i) => { + fs.writeFileSync(path.join(caseDir, `${i}.m`), marker); + fs.writeFileSync(path.join(caseDir, `${i}.l`), content); + }); + const D = q(caseDir.replace(/\\/g, '/')); + const out = runHelpers(`i=0; while [ -f ${D}/$i.m ]; do IFS= read -r -d '' m < ${D}/$i.m || true; printf '%s\\t[%s]\\n' "$i" "$(gsd_return_marker ${D}/$i.l "$m")"; i=$((i+1)); done`); + cleanup(caseDir); + const got = new Map(out.split('\n').filter(Boolean).map((l) => { + const tab = l.indexOf('\t'); + // Bracketed so trimming the output can never eat an empty result. + return [Number(l.slice(0, tab)), l.slice(tab + 2, -1)]; + })); + return cases.map((_, i) => got.get(i)); + } + + // The specification, independent of the bash: strip one trailing CR, then one + // leading BOM; match iff the line starts with the marker and the marker is + // followed by the end of the line, whitespace (space, tab, CR, VT, FF) or a + // colon (the planner's Source Audit return puts `: Unplanned Items Found` after its + // marker). Any other character, ASCII or not (`-x`, a letter with an accent), is not + // a boundary. + function returnMarkerOracle(marker, line) { + let l = line.endsWith('\r') ? line.slice(0, -1) : line; + if (l.startsWith('')) l = l.slice(1); + return l.startsWith(marker) && /^(?:$|[ \t\r\v\f:])/.test(l.slice(marker.length)) ? marker : ''; + } + + test('property: gsd_return_marker matches iff the line starts with the marker and the next char is end, whitespace or a colon (CR- and BOM-tolerant)', (t) => { + const f = fixture(t); + const markerArb = fc.constantFrom('## PLANNING COMPLETE', '## PLAN COMPLETE', '## ISSUES FOUND', '## ⚠ Source Audit', '## REVISION_CONFLICT', '#'); + // Word and non-word ASCII, non-ASCII letters (must count as a boundary), and + // marker-ish characters; no newline, CR or NUL inside the line body. + const charArb = fc.constantFrom(...'aZ9_ -(.:#\té⚠ß'.split('')); + const freeArb = fc.string({ unit: charArb, maxLength: 8 }); + const lineArb = markerArb.chain((m) => fc.tuple( + fc.constant(m), + fc.oneof( + freeArb, // unrelated text + fc.constant(m), // exactly the marker + fc.tuple(charArb, freeArb).map(([c, rest]) => m + c + rest), // marker, then one char + freeArb.map((pre) => pre + m), // marker not at line start + fc.nat(m.length).map((k) => m.slice(0, k)), // truncated marker + ), + fc.boolean(), fc.boolean(), fc.boolean(), // BOM, trailing CR, final newline + )); + fc.assert(fc.property(fc.array(lineArb, { minLength: 1, maxLength: 40 }), (rows) => { + const lines = rows.map(([, body, bom, cr]) => (bom ? '' : '') + body + (cr ? '\r' : '')); + const cases = rows.map(([m], i) => [m, lines[i] + (rows[i][4] ? '\n' : '')]); + const got = returnMarkerBatch(f.dir, cases); + return rows.every(([m], i) => got[i] === returnMarkerOracle(m, lines[i])); + }), { numRuns: 10 }); + }); + + test('property: gsd_receipt_path never leaves /.gsd-returns/ for any spawn label, and refuses an unsafe phase dir', (t) => { + const f = fixture(t); + const W = /^[A-Za-z0-9_-]+$/; + const labelArb = fc.string({ unit: fc.constantFrom(...'aZ9_-./\\ \'"$`*?\né⚠'.split('').concat(['..'])), maxLength: 12 }); + fc.assert(fc.property(fc.array(labelArb, { minLength: 1, maxLength: 25 }), (labels) => { + const caseDir = fs.mkdtempSync(path.join(f.dir, 'rp-')); + labels.forEach((l, i) => fs.writeFileSync(path.join(caseDir, `${i}.s`), l)); + const D = q(caseDir.replace(/\\/g, '/')); + const out = runHelpers(`cd ${q(f.dir)}; i=0; while [ -f ${D}/$i.s ]; do IFS= read -r -d '' s < ${D}/$i.s || true; printf '%s\\t%s\\n' "$i" "$(gsd_receipt_path ph "$s")"; i=$((i+1)); done; pwd`); + cleanup(caseDir); + const outLines = out.split('\n'); + const root = outLines.pop(); + const returns = (root.replace(/^\/([A-Za-z])\//, '$1:/') + '/ph/.gsd-returns').toLowerCase(); + return labels.every((label, i) => { + const line = outLines.find((l) => l.startsWith(`${i}\t`)); + const p = line.slice(line.indexOf('\t') + 1); + const slash = p.lastIndexOf('/'); + const parent = p.slice(0, slash).toLowerCase(); + const [stem, epoch, rand] = p.slice(slash + 1).split('.'); + const sameDir = parent === returns || parent.endsWith('/ph/.gsd-returns'); + const ascii = /^[\x20-\x7e]*$/.test(label); + const expected = label.replace(/[^A-Za-z0-9_-]/g, '_') || 'spawn'; + // A non-ASCII character becomes one `_` per character (UTF-8 locale) or one per + // byte (C locale), so for those labels compare with runs of `_` collapsed; an + // ASCII label must match exactly. + const collapse = (x) => x.replace(/_+/g, '_'); + return sameDir && W.test(stem) && epoch === String(NOW) && /^[A-Za-z0-9]+$/.test(rand) && + (ascii ? stem === expected : collapse(stem) === collapse(expected)); + }); + }), { numRuns: 12 }); + // Unsafe phase dirs: any dir holding a quote, `$`, backtick, `<`, `>` or a control + // character (newline, tab, CR, ESC, DEL...) fails closed (no output, non-zero); any + // other dir is accepted. Values travel through files and are read with -d '' so a + // newline reaches the helper intact. + const dirArb = fc.string({ unit: fc.constantFrom(...'ab -\'"$`<>\n\t\r\x01\x1b\x7f'.split('')), minLength: 1, maxLength: 6 }).map((d) => `p${d}`); + const unsafeDir = (d) => /['"$`<>]/.test(d) || [...d].some((c) => c.charCodeAt(0) < 0x20 || c.charCodeAt(0) === 0x7f); + let checked = 0; + fc.assert(fc.property(fc.array(dirArb, { minLength: 1, maxLength: 15 }), (dirs) => { + const caseDir = fs.mkdtempSync(path.join(f.dir, 'pd-')); + dirs.forEach((d, i) => fs.writeFileSync(path.join(caseDir, `${i}.d`), d)); + const D = q(caseDir.replace(/\\/g, '/')); + const out = runHelpers(`cd ${q(f.dir)}; i=0; while [ -f ${D}/$i.d ]; do IFS= read -r -d '' d < ${D}/$i.d || true; if gsd_receipt_path "$d" x >/dev/null; then echo "$i OK"; else echo "$i FAIL"; fi; i=$((i+1)); done`); + cleanup(caseDir); + const verdict = new Map(out.split('\n').map((l) => l.split(' '))); + checked += verdict.size; + return verdict.size === dirs.length && + dirs.every((d, i) => verdict.get(String(i)) === (unsafeDir(d) ? 'FAIL' : 'OK')); + }), { numRuns: 8 }); + assert.ok(checked > 0, 'the unsafe-dir property must execute at least one case'); + }); + + test('gsd_receipt_path: absolute, unique per call, under /.gsd-returns/, directory created, label sanitized', (t) => { + const f = fixture(t); + const rel = 'phase dir/01-x'; + fs.mkdirSync(path.join(f.dir, rel), { recursive: true }); + const script = `${extractStallFunctionsBash()}\ncd ${q(f.dir)}\na=$(gsd_receipt_path ${q(rel)} checker)\nb=$(gsd_receipt_path ${q(rel)} checker)\nc=$(gsd_receipt_path ${q(`${f.fwd}/${rel}`)} '../01/x y')\nprintf '%s\\n' "$a" "$b" "$c"\n[ -d "${rel}/.gsd-returns" ] && echo DIR_OK\n[ -e "$a" ] || echo NOT_PRECREATED\n`; + const result = runBashScript(script, []); + assert.equal(result.status, 0, result.stderr); + const [a, b, c, dirOk, notPre] = result.stdout.trim().split('\n'); + for (const p of [a, b, c]) { + assert.match(p, /^(\/|[A-Za-z]:\/)/, `receipt path must be absolute: ${p}`); + assert.match(p, /\/phase dir\/01-x\/\.gsd-returns\/[^/]+$/, `receipt must live in /.gsd-returns/: ${p}`); + } + assert.notEqual(a, b, 'two dispatches in the same second must get distinct receipts'); + assert.match(path.posix.basename(c), /^___01_x_y/, 'a hostile label cannot leave .gsd-returns/'); + assert.equal(dirOk, 'DIR_OK'); + assert.equal(notPre, 'NOT_PRECREATED', 'the receipt must not be pre-created (a host Write tool may refuse to overwrite an unread file)'); + }); + + test('gsd_receipt_path fails closed on an unsafe phase dir and writes a catch-all .gitignore', (t) => { + const f = fixture(t); + const helpers = extractStallFunctionsBash(); + // A newline, tab, CR or any other control character, `<` and `>` would land in the + // `` prompt line the orchestrator substitutes (review 5477738387 Minor 1). + const unsafe = ['', 'ph"ase', "ph'ase", 'ph$ase', 'ph`ase', + 'ph\nase', 'ph\tase', 'ph\rase', 'ph\x01ase', 'ph\x1base', 'ph\x7fase', 'phase', 'ph\n']; + let refused = 0; + for (const bad of unsafe) { + // Handed over through a file, never interpolated or passed as argv: a `$` or + // backtick in a double-quoted literal is expanded by bash first, and on Windows + // Git Bash's MSYS layer re-splits and unescapes argv, so a `'` argument arrives + // stripped (observed in CI: "ph'ase" reached the helper as "phase"). See + // extractStallFunctionsBash()'s doc comment for the same transport hazard. + const valueFile = path.join(f.dir, 'phase-dir-value.txt'); + fs.writeFileSync(valueFile, bad); + // -d '' reads the whole file, so an embedded or trailing newline reaches the helper. + const r = runBashScript(`${helpers}\ncd ${q(f.dir)}\nIFS= read -r -d '' d < ${q(valueFile.replace(/\\/g, '/'))} || true\nif out=$(gsd_receipt_path "$d" checker); then echo "OK:$out"; else echo FAIL; fi\n`, []); + assert.equal(r.stdout.trim(), 'FAIL', `phase dir ${JSON.stringify(bad)} must be refused`); + refused += 1; + } + assert.equal(refused, unsafe.length, 'every unsafe phase dir case must run'); + assert.equal(fs.readdirSync(f.dir).filter((n) => n.startsWith('ph') && n !== 'phase-dir-value.txt').length, 0, 'a refused phase dir is never created'); + const r = runBashScript(`${helpers}\ncd ${q(f.dir)}\ngsd_receipt_path ph checker >/dev/null && cat ph/.gsd-returns/.gitignore\n`, []); + assert.equal(r.stdout.trim(), '*', '.gsd-returns/ must ignore its own receipts'); + }); + + // A hostile checkout can commit /.gsd-returns (or its .gitignore) as a + // symlink; mkdir -p and the .gitignore printf would follow it out of the phase. + // A skip is allowed only off CI: the CI runners create these links (at 35bf07e37 the + // Windows shard that ran this file ran every symlink row without a skip), so in CI a + // refused link fails the row instead of passing it with zero assertions. + function symlinkOrSkip(t, target, link, type) { + try { + fs.symlinkSync(target, link, type === 'dir' && process.platform === 'win32' ? 'junction' : type); + return true; + } catch (error) { + if (error && ['EPERM', 'EACCES', 'ENOTSUP'].includes(error.code) && !process.env.CI) { + t.skip(`symlink creation is not available here (${error.code}); CI runs this row`); + return false; + } + throw error; + } + } + + test('gsd_receipt_path refuses a symlinked /.gsd-returns and writes nothing through it', (t) => { + const f = fixture(t); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5182-outside-')); + t.after(() => cleanup(outside)); + fs.mkdirSync(path.join(f.dir, 'ph')); + if (!symlinkOrSkip(t, outside, path.join(f.dir, 'ph', '.gsd-returns'), 'dir')) return; + const r = runBashScript(`${extractStallFunctionsBash()}\ncd ${q(f.dir)}\ngsd_receipt_path ph checker\n`, []); + assert.notEqual(r.status, 0, 'a symlinked .gsd-returns must be refused'); + assert.equal(r.stdout, '', 'a refused receipt prints no path'); + assert.deepEqual(fs.readdirSync(outside), [], 'nothing may be created in the link target'); + }); + + test('gsd_receipt_path refuses a symlinked .gsd-returns/.gitignore and leaves its target untouched', (t) => { + const f = fixture(t); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5182-outside-')); + t.after(() => cleanup(outside)); + const returns = path.join(f.dir, 'ph', '.gsd-returns'); + fs.mkdirSync(returns, { recursive: true }); + const victim = path.join(outside, 'victim.txt'); + fs.writeFileSync(victim, 'ORIGINAL\n'); + if (!symlinkOrSkip(t, victim, path.join(returns, '.gitignore'), 'file')) return; + const script = `${extractStallFunctionsBash()}\ncd ${q(f.dir)}\ngsd_receipt_path ph checker\n`; + let r = runBashScript(script, []); + assert.notEqual(r.status, 0, 'a symlinked .gitignore must be refused'); + assert.equal(r.stdout, '', 'a refused receipt prints no path'); + assert.equal(fs.readFileSync(victim, 'utf8'), 'ORIGINAL\n', 'the link target must be unchanged'); + // A dangling link is the write-through case: [ -f ] is false, so an unguarded + // printf would create the target outside the phase. + fs.unlinkSync(path.join(returns, '.gitignore')); + const absent = path.join(outside, 'absent.txt'); + if (!symlinkOrSkip(t, absent, path.join(returns, '.gitignore'), 'file')) return; + r = runBashScript(script, []); + assert.notEqual(r.status, 0, 'a dangling .gitignore symlink must be refused'); + assert.equal(r.stdout, '', 'a refused receipt prints no path'); + assert.equal(fs.existsSync(absent), false, 'the dangling target must not be created'); + }); + + test('a lost $TS falls back to the dispatch epoch stamped in the receipt name, so the threshold stays reachable', (t) => { + const f = fixture(t); + const checker = watchCallOf(sectionOf(readPlanPhase(), CHECKER_SECTION)); + const stale = path.join(f.dir, `checker.${NOW - 11 * 60}.AbCdEfGh`); + assert.equal(watch('', stale, f.noPlans, checker.markers), 'stalled'); + const fresh = path.join(f.dir, `checker.${NOW}.AbCdEfGh`); + assert.equal(watch('', fresh, f.noPlans, checker.markers), 'waiting'); + }); + + test('a receipt-routed ## ISSUES FOUND with no issue list fails closed (never counted as 0 issues)', () => { + const step11 = sectionOf(readPlanPhase(), ['## 11. Handle Checker Return', '## 11a.']); + assert.match(step11, /## ISSUES FOUND[^\n]*11a/, 'a missing issue list routes to 11a, never to a zero count'); + }); + + // Review 5563209291 Minor 2: step 12 reads the conflict record's fields (CONFLICT_*) from + // the revision's return body, which a receipt never carries, so a receipt-only + // ## REVISION_CONFLICT must sit in the fail-closed list beside the other body-needing routes. + test('a receipt-routed ## REVISION_CONFLICT with no body fails closed (never an empty conflict record)', () => { + const revisionSection = sectionOf(readPlanPhase(), REVISION_SECTION); + assert.ok(watchCallOf(revisionSection).markers.includes('## REVISION_CONFLICT'), 'the revision watch passes the conflict marker'); + assert.match(revisionSection, /CONFLICT_DIMENSION/, 'control: step 12 reads conflict fields from the return body'); + const doc = readStallHelpersDoc(); + const at = doc.indexOf('fails closed:'); + assert.notEqual(at, -1); + const failClosed = doc.slice(at, doc.indexOf('\n\n', at)); + let ran = 0; + for (const marker of ['## ISSUES FOUND', '## CHECKPOINT REACHED', '## PHASE SPLIT RECOMMENDED', '## ⚠ Source Audit', '## REVISION_CONFLICT']) { + assert.ok(failClosed.includes(`\`${marker}\``), `${marker} is in the fail-closed list`); + ran += 1; + } + assert.equal(ran, 5); + }); + + // Merge with next 39bb1b367 (#5578): step 11 now reads a ## VERIFICATION PASSED return's body for + // listed BLOCKER/WARNING entries before proceeding, and a receipt never carries that body, so a + // receipt-only PASSED must fail closed like the other body-needing routes instead of reaching step 13. + test('a receipt-routed ## VERIFICATION PASSED with no body fails closed (step 11 reads its findings, #5578)', () => { + const step11 = sectionOf(readPlanPhase(), ['## 11. Handle Checker Return', '## 11a.']); + assert.match(step11, /VERIFICATION PASSED`:\*\* If the return lists an entry whose severity is BLOCKER or WARNING/, 'control: step 11 reads the PASSED body (#5578)'); + const checker = watchCallOf(sectionOf(readPlanPhase(), CHECKER_SECTION)); + assert.ok(checker.markers.includes('## VERIFICATION PASSED'), 'control: the checker watch passes the PASSED marker'); + const doc = readStallHelpersDoc(); + const at = doc.indexOf('fails closed:'); + assert.notEqual(at, -1); + const failClosed = doc.slice(at, doc.indexOf('\n\n', at)); + assert.ok(failClosed.includes('`## VERIFICATION PASSED`'), '## VERIFICATION PASSED is in the fail-closed list'); + assert.match(failClosed, /`## VERIFICATION PASSED`[^.]*never proceeds to step 13/, 'a body-less PASSED never proceeds to step 13 on the receipt alone'); + assert.match(step11, /proceed to step 13 \(receipt only, no body by the next cycle: 11a\)/, 'step 11 itself points a body-less PASSED at 11a'); + }); + + // The helpers send a body-less receipt-routed PASSED or ISSUES FOUND to 11a "and offer Retry + // checker before Accept". 11a's elaboration must carry that entry route itself: a trigger that + // covers it, a banner naming the marker that arrived, and Retry listed before Accept. + test('11a covers a receipt-routed checker marker whose body never arrived: names the marker, Retry before Accept', () => { + const detail = readFileNormalized(path.join(REPO_ROOT, 'gsd-core', 'workflows', 'plan-phase', 'detail', 'elaboration.md')); + const s11a = sectionOf(detail, ['## § 11a', '## § 11 thinking-partner']); + assert.match(s11a, /Fires when[^\n]*return receipt[^\n]*has not arrived by the next cycle/, '11a\'s trigger covers a receipt-routed marker with no body'); + const at = s11a.indexOf('a marker arrived on the receipt without its body'); + assert.notEqual(at, -1, '11a has a case for a marker that arrived on the receipt without its body'); + const receiptCase = s11a.slice(at, s11a.indexOf('**If `DISK_PLANS`', at + 1)); + assert.match(receiptCase, /\{MARKER\}/, 'the receipt-case banner names the marker that arrived'); + const retry = receiptCase.indexOf('**Retry checker**'); + const accept = receiptCase.indexOf('**Accept verification**'); + assert.ok(retry !== -1 && accept !== -1, 'the receipt case offers both Retry checker and Accept verification'); + assert.ok(retry < accept, 'the receipt case lists Retry checker before Accept verification'); + // Control: the original empty/truncated route keeps its own banner and menu. + assert.match(s11a, /Checker return was empty or truncated/, 'control: the empty/truncated banner stays'); + }); + + test('both agent definitions carry the receipt rule; the checker writes it with Write, its only write', () => { + const planner = readFileNormalized(path.join(REPO_ROOT, 'agents', 'gsd-planner.md')); + const checker = readFileNormalized(path.join(REPO_ROOT, 'agents', 'gsd-plan-checker.md')); + for (const [label, doc] of [['gsd-planner', planner], ['gsd-plan-checker', checker]]) { + assert.match(doc, //, `${label} must define the rule`); + } + // #5182 AC5: a Bash write is refused by a read-only sandbox (Codex), so the checker + // declares Write (Group B) and the receipt is its only write. + assert.match(checker.match(/^tools:.*$/m)[0], /\bWrite\b/, 'the checker declares Write for its receipt'); + const rule = checker.slice(checker.indexOf('**Return receipt (#5182):**')); + assert.doesNotMatch(rule.slice(0, 700), /printf/, 'no Bash printf write appears in the checker receipt rule'); + // The limit must hold on spawns that send no receipt (quick, quick-batch, import), so it + // also lives in , outside the receipt-conditional rule. + const anti = checker.slice(checker.indexOf(''), checker.indexOf('')); + assert.ok(anti.includes('``'), 'the write limit names the receipt path in '); + }); + + // Prose-contract rows are reduced to command tokens, so rewording the explanation + // around them does not break the suite (review 5563209291 Nit 4). The checker's + // per-host posture is pinned on the installed artifacts in agent-tools.install.test.cjs. + test('helpers doc: receipts are removed after routing', () => { + const doc = readStallHelpersDoc(); + assert.match(doc, /rm -f "\{receipt\}"/, 'the orchestrator removes the receipt after routing'); + }); + + // -- Review 5477738387 (#5182): physical containment of the receipt directory, hostile + // phase-dir names, receipt reads that cannot be steered or stalled, a spaced phase dir's + // freshness signal, and receipt cleanup at every watch. + const fwd = (p) => p.replace(/\\/g, '/'); + + function outsideDir(t) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5182-outside-')); + t.after(() => cleanup(dir)); + return dir; + } + + // Every entry under `root`, recursively (lstat, so a link is listed, never followed). + function treeOf(root) { + const out = []; + const walk = (d, rel) => { + for (const name of fs.readdirSync(d).sort()) { + const r = rel ? `${rel}/${name}` : name; + out.push(r); + const st = fs.lstatSync(path.join(d, name)); + if (st.isDirectory()) walk(path.join(d, name), r); + } + }; + walk(root, ''); + return out; + } + + // gsd_receipt_path run from `cwd`, PHASE_DIR handed over through a file (never argv); + // `root` is the optional third argument (init's project_root). + function receiptPathFrom(f, cwd, phaseDir, root) { + const valueFile = path.join(f.dir, 'phase-dir-value.bin'); + fs.writeFileSync(valueFile, phaseDir); + const rootArg = root === undefined ? '' : ` ${q(fwd(root))}`; + return runBashScript(`${extractStallFunctionsBash()}\ncd ${q(cwd)}\nIFS= read -r -d '' d < ${q(fwd(valueFile))} || true\ngsd_receipt_path "$d" checker${rootArg}\n`, []); + } + + // A refusal names its reason in exactly one stderr line with a fixed prefix (the + // reason text itself is not pinned). + function assertRefusalReason(r, label) { + const lines = r.stderr.split('\n').filter(Boolean); + assert.equal(lines.length, 1, `${label}: one stderr line, got ${JSON.stringify(r.stderr)}`); + assert.match(lines[0], /^gsd_receipt_path: no receipt: \S/, `${label}: the stderr line names a reason`); + } + + // Review 5563209291 Minor 1: containment is anchored on the project root the workflow + // resolved (init's project_root), not on the orchestrator's cwd. + test('gsd_receipt_path accepts an absolute phase dir under the project root from a cwd outside it, and names the reason when it refuses', (t) => { + const f = fixture(t); + const proj = path.join(f.dir, 'proj'); + const phase = path.join(proj, '.planning', 'phases', '01-x'); + fs.mkdirSync(phase, { recursive: true }); + fs.mkdirSync(path.join(proj, 'sub')); + const sibling = path.join(f.dir, 'sibling'); + fs.mkdirSync(sibling); + // cwd is a subdirectory of the project, so the phase dir is outside the cwd. + const ok = receiptPathFrom(f, path.join(proj, 'sub'), fwd(phase), proj); + assert.equal(ok.status, 0, ok.stderr); + assert.equal(ok.stderr, '', 'an accepted receipt prints nothing on stderr'); + assert.match(ok.stdout.trim(), /\/proj\/\.planning\/phases\/01-x\/\.gsd-returns\/checker\.\d+\.[A-Za-z0-9]+$/); + assert.ok(fs.existsSync(path.join(phase, '.gsd-returns', '.gitignore')), 'the receipt directory is created under the phase dir'); + // The same call from a cwd OUTSIDE the project (the orchestrator's cwd no longer matters). + const fromSibling = receiptPathFrom(f, sibling, fwd(phase), proj); + assert.equal(fromSibling.status, 0, fromSibling.stderr); + // Without the root argument the cwd is the anchor, so the same phase dir is refused, with a reason. + const noRoot = receiptPathFrom(f, path.join(proj, 'sub'), fwd(phase)); + assert.notEqual(noRoot.status, 0, 'with no root argument the cwd anchors the check'); + assert.equal(noRoot.stdout, ''); + assertRefusalReason(noRoot, 'cwd-anchored'); + // A phase dir outside the passed root is still refused, and nothing is created there. + const outside = receiptPathFrom(f, proj, `${fwd(sibling)}/ph`, proj); + assert.notEqual(outside.status, 0, 'a phase dir outside the project root is refused'); + assertRefusalReason(outside, 'outside the root'); + assert.deepEqual(treeOf(sibling), [], 'nothing is created outside the root'); + // A root that does not resolve refuses rather than falling back. + const badRoot = receiptPathFrom(f, proj, fwd(phase), path.join(f.dir, 'no-such-root')); + assert.notEqual(badRoot.status, 0, 'an unresolvable root is refused'); + assertRefusalReason(badRoot, 'unresolvable root'); + }); + + test('every gsd_receipt_path refusal names its reason on one stderr line', (t) => { + const f = fixture(t); + const proj = path.join(f.dir, 'proj'); + fs.mkdirSync(path.join(proj, 'ph'), { recursive: true }); + const outside = outsideDir(t); + const cases = [ + ['ph"ase', 'unsafe character'], + ['ph\nase', 'control character'], + ['', 'empty'], + ['ph/../ph', '.. component'], + ['ph/./x', '. component'], + [`${fwd(outside)}/ph`, 'outside the project'], + ]; + if (symlinkOrSkip(t, outside, path.join(proj, 'ph', '.gsd-returns'), 'dir')) cases.push(['ph', 'symlinked .gsd-returns']); + let ran = 0; + for (const [phaseDir, label] of cases) { + const r = receiptPathFrom(f, proj, phaseDir, proj); + assert.notEqual(r.status, 0, `${label}: refused`); + assert.equal(r.stdout, '', `${label}: no path printed`); + assertRefusalReason(r, label); + assert.ok(!r.stderr.includes(phaseDir) || phaseDir === '' || phaseDir === 'ph', `${label}: the phase dir itself is never echoed`); + ran += 1; + } + assert.ok(ran >= 6); + }); + + // Review 5563209291 Minor 3: path-shaped generation. Segments are drawn from `a`, `.`, + // `..` and `link`; `proj/link` and `proj/a/link` are links to a directory outside the + // project, and a `link` segment anywhere else is a plain (missing) directory name. A dir + // is relative to the project, absolute under it, or absolute under a sibling outside it. + // The oracle is the refusal set: a `.` or `..` segment, a `link` segment where the link + // is, or a sibling base is refused; anything else gets a receipt under /.gsd-returns/. + function crossesLink(segs) { + return segs.some((seg, i) => seg === 'link' && (i === 0 || (i === 1 && segs[0] === 'a'))); + } + test('property: a path built from a, ., .. and link segments gets a receipt iff it has no . or .., crosses no link and stays under the project root', (t) => { + const f = fixture(t); + const proj = path.join(f.dir, 'proj'); + const sibling = path.join(f.dir, 'sibling'); + fs.mkdirSync(path.join(proj, 'a', 'a'), { recursive: true }); + fs.mkdirSync(sibling); + const outside = outsideDir(t); + fs.mkdirSync(path.join(outside, 'a', 'a'), { recursive: true }); + if (!symlinkOrSkip(t, outside, path.join(proj, 'link'), 'dir')) return; + if (!symlinkOrSkip(t, outside, path.join(proj, 'a', 'link'), 'dir')) return; + const caseArb = fc.record({ + // `a` is weighted so accepted paths are common; one fixed case per side is + // appended to every run below, so both sides of the oracle always execute. + segs: fc.array(fc.oneof({ arbitrary: fc.constant('a'), weight: 5 }, fc.constantFrom('.', '..', 'link')), { minLength: 1, maxLength: 4 }), + base: fc.constantFrom('rel', 'abs', 'sibling'), + }); + const P = fwd(proj); + const S = fwd(sibling); + let accepted = 0; + let refused = 0; + fc.assert(fc.property(fc.array(caseArb, { minLength: 1, maxLength: 12 }), (generated) => { + const cases = [...generated, { segs: ['a'], base: 'abs' }, { segs: ['link', 'a'], base: 'rel' }]; + const outsideBefore = treeOf(outside); + const siblingBefore = treeOf(sibling); + const dirs = cases.map(({ segs, base }) => { + const rel = segs.join('/'); + return base === 'rel' ? rel : base === 'abs' ? `${P}/${rel}` : `${S}/${rel}`; + }); + const script = dirs.map((d, i) => + `if p=$(gsd_receipt_path ${q(d)} x ${q(P)} 2>/dev/null); then printf '%s\\t%s\\n' ${i} "$p"; else printf '%s\\tFAIL\\n' ${i}; fi`).join('\n'); + const out = runHelpers(`cd ${q(P)}\n${script}`); + const got = new Map(out.split('\n').filter(Boolean).map((l) => [l.slice(0, l.indexOf('\t')), l.slice(l.indexOf('\t') + 1)])); + const ok = cases.every(({ segs, base }, i) => { + const expectRefused = base === 'sibling' || segs.some((s) => s === '.' || s === '..') || crossesLink(segs); + const p = got.get(String(i)); + if (expectRefused) { refused += 1; return p === 'FAIL'; } + accepted += 1; + return p !== 'FAIL' && p.toLowerCase().includes(`/proj/${segs.join('/')}/.gsd-returns/x.`.toLowerCase()); + }); + return ok && got.size === cases.length && + JSON.stringify(treeOf(outside)) === JSON.stringify(outsideBefore) && + JSON.stringify(treeOf(sibling)) === JSON.stringify(siblingBefore); + }), { numRuns: 8 }); + assert.ok(accepted > 0 && refused > 0, `both sides of the oracle ran (accepted ${accepted}, refused ${refused})`); + }); + + test('gsd_receipt_path refuses a symlinked phase directory (relative or absolute) and creates nothing in its target', (t) => { + const f = fixture(t); + const outside = outsideDir(t); + if (!symlinkOrSkip(t, outside, path.join(f.dir, 'ph'), 'dir')) return; + let ran = 0; + for (const phaseDir of ['ph', `${f.fwd}/ph`, 'ph/sub']) { + const r = receiptPathFrom(f, f.dir, phaseDir); + assert.notEqual(r.status, 0, `${phaseDir}: a symlinked phase dir must be refused`); + assert.equal(r.stdout, '', `${phaseDir}: a refused receipt prints no path`); + ran += 1; + } + assert.equal(ran, 3); + assert.deepEqual(treeOf(outside), [], 'nothing may be created in the link target'); + }); + + test('gsd_receipt_path refuses a phase dir under a symlinked ancestor (.planning linked outside the project)', (t) => { + const f = fixture(t); + const outside = outsideDir(t); + const proj = path.join(f.dir, 'proj'); + fs.mkdirSync(proj); + fs.mkdirSync(path.join(outside, 'phases', '01-x'), { recursive: true }); + if (!symlinkOrSkip(t, outside, path.join(proj, '.planning'), 'dir')) return; + const before = treeOf(outside); + let ran = 0; + // An existing phase dir reached through the link, a missing one, and the absolute form. + for (const phaseDir of ['.planning/phases/01-x', '.planning/phases/02-new', `${fwd(proj)}/.planning/phases/01-x`]) { + const r = receiptPathFrom(f, proj, phaseDir); + assert.notEqual(r.status, 0, `${phaseDir}: a phase dir that resolves outside the project must be refused`); + assert.equal(r.stdout, '', `${phaseDir}: a refused receipt prints no path`); + ran += 1; + } + assert.equal(ran, 3); + assert.deepEqual(treeOf(outside), before, 'nothing may be created outside the project'); + }); + + test('gsd_receipt_path refuses a phase dir with a .. component and creates nothing outside the project', (t) => { + // A `..` after a component that does not exist yet passes the ancestor check + // (the nearest existing ancestor is the project); mkdir -p would then resolve it + // and create the receipt directory outside. No symlink is involved. + const f = fixture(t); + const proj = path.join(f.dir, 'proj'); + fs.mkdirSync(path.join(proj, '.planning'), { recursive: true }); + let ran = 0; + for (const [phaseDir, escaped] of [ + ['newdir/../../ESCAPED', 'ESCAPED'], + [`${fwd(proj)}/n2/../../ESC2`, 'ESC2'], + ['.planning/../../ESC3', 'ESC3'], + ]) { + const r = receiptPathFrom(f, proj, phaseDir); + assert.notEqual(r.status, 0, `${phaseDir}: a .. component must be refused`); + assert.equal(r.stdout, '', `${phaseDir}: a refused receipt prints no path`); + assert.equal(fs.existsSync(path.join(f.dir, escaped)), false, `${phaseDir}: nothing may be created outside the project`); + ran += 1; + } + assert.equal(ran, 3); + }); + + // The rows above link outside the project, so the containment check alone refuses + // them. A link that stays inside the project is refused only by the walk's own + // link test: a phase dir that is itself a link, and a link among the components + // the walk visits (here one that points back at the project, followed by a missing + // child). An existing phase dir reached through a parent link that stays inside the + // project is accepted (only the walked components and physical containment are + // checked), and that row pins the scope of the claim. + test('gsd_receipt_path refuses an in-project link at the phase dir or on the walked path, and accepts an existing phase dir under an in-project parent link', (t) => { + const f = fixture(t); + const proj = path.join(f.dir, 'proj'); + fs.mkdirSync(path.join(proj, '.planning', 'phases'), { recursive: true }); + fs.mkdirSync(path.join(proj, 'inner', 'real'), { recursive: true }); + if (!symlinkOrSkip(t, path.join(proj, 'inner', 'real'), path.join(proj, '.planning', 'phases', '01-link'), 'dir')) return; + if (!symlinkOrSkip(t, proj, path.join(proj, '.planning', 'up'), 'dir')) return; + if (!symlinkOrSkip(t, path.join(proj, 'inner'), path.join(proj, '.planning', 'lp'), 'dir')) return; + let ran = 0; + for (const phaseDir of ['.planning/phases/01-link', '.planning/up/NEWCHILD']) { + const r = receiptPathFrom(f, proj, phaseDir); + assert.notEqual(r.status, 0, `${phaseDir}: an in-project link at the phase dir or on the walked path must be refused`); + assert.equal(r.stdout, '', `${phaseDir}: a refused receipt prints no path`); + ran += 1; + } + assert.equal(ran, 2); + assert.equal(fs.existsSync(path.join(proj, 'inner', 'real', '.gsd-returns')), false, 'nothing is created through the phase-dir link'); + assert.equal(fs.existsSync(path.join(proj, 'NEWCHILD')), false, 'nothing is created through the walked link'); + const ok = receiptPathFrom(f, proj, '.planning/lp/real'); + assert.equal(ok.status, 0, ok.stderr); + assert.match(ok.stdout.trim(), /\/\.planning\/lp\/real\/\.gsd-returns\/checker\.\d+\.[A-Za-z0-9]+$/, 'an existing phase dir under an in-project parent link gets a receipt'); + }); + + test('gsd_receipt_path refuses a dangling .gsd-returns link and a dangling phase-dir link without creating either target', (t) => { + const f = fixture(t); + const outside = outsideDir(t); + fs.mkdirSync(path.join(f.dir, 'ph')); + if (!symlinkOrSkip(t, path.join(outside, 'absent-returns'), path.join(f.dir, 'ph', '.gsd-returns'), 'dir')) return; + if (!symlinkOrSkip(t, path.join(outside, 'absent-phase'), path.join(f.dir, 'ph2'), 'dir')) return; + let ran = 0; + for (const phaseDir of ['ph', 'ph2']) { + const r = receiptPathFrom(f, f.dir, phaseDir); + assert.notEqual(r.status, 0, `${phaseDir}: a dangling link must be refused`); + assert.equal(r.stdout, '', `${phaseDir}: a refused receipt prints no path`); + ran += 1; + } + assert.equal(ran, 2); + assert.deepEqual(treeOf(outside), [], 'no dangling target may be created'); + }); + + test('gsd_return_marker ignores a receipt that is a symlink, even to a file holding a marker', (t) => { + const f = fixture(t); + const outside = outsideDir(t); + const target = path.join(outside, 'forged.md'); + fs.writeFileSync(target, '## ISSUES FOUND\n'); + const link = path.join(f.dir, 'receipt-link.md'); + if (!symlinkOrSkip(t, target, link, 'file')) return; + const m = ` ${q('## VERIFICATION PASSED')} ${q('## ISSUES FOUND')}`; + assert.equal(runHelpers(`gsd_return_marker ${q(fwd(target))}${m}`), '## ISSUES FOUND', 'control: the target itself carries a marker'); + assert.equal(runHelpers(`gsd_return_marker ${q(fwd(link))}${m}`), '', 'a symlinked receipt is never read'); + }); + + // The cap is asserted by output (what is and is not read), never by elapsed time. + test('gsd_return_marker reads at most the first 4096 bytes, so one huge line cannot stall the watch', (t) => { + const f = fixture(t); + const run = (content, marker = '## PLANNING COMPLETE') => { + fs.writeFileSync(f.receipt, content); + return runHelpers(`gsd_return_marker ${q(fwd(f.receipt))} ${q(marker)}`); + }; + assert.equal(run(`${'x'.repeat(4000)}\n## PLANNING COMPLETE\n`), '## PLANNING COMPLETE', 'a marker inside the first 4096 bytes is found'); + assert.equal(run(`${'x'.repeat(5000)}\n## PLANNING COMPLETE\n`), '', 'a marker past the first 4096 bytes is not read'); + assert.equal(run(`${'a'.repeat(1000000)}\n## PLANNING COMPLETE\n`), '', 'a marker after a 1,000,000-byte line is not read'); + assert.equal(run(`## PLANNING COMPLETE ${'a'.repeat(1000000)}\n`), '## PLANNING COMPLETE', 'a marker opening a 1,000,000-byte line is still found'); + }); + + // Review 5563209291 Minor 4: limit-1 / limit / limit+1 for the 4096-byte cap. The marker + // (20 bytes) is placed so its last byte is byte 4095, 4096 or 4097 of the receipt. + test('gsd_return_marker byte cap: a marker ending at byte 4095 or 4096 is found, at 4097 it is not, and a word cut at the cap is not a marker', (t) => { + const f = fixture(t); + const run = (content, marker) => { + fs.writeFileSync(f.receipt, content); + return runHelpers(`gsd_return_marker ${q(fwd(f.receipt))} ${q(marker)}`); + }; + const endingAt = (end, marker, after = '\n') => `${'x'.repeat(end - marker.length - 1)}\n${marker}${after}`; + const M = '## PLANNING COMPLETE'; + const rows = [ + [endingAt(4095, M), M, M, 'ends at byte 4095 (limit-1)'], + [endingAt(4096, M), M, M, 'ends at byte 4096 (limit)'], + [endingAt(4097, M), M, '', 'ends at byte 4097 (limit+1)'], + [endingAt(4096, M, ''), M, M, 'ends the file at byte 4096 with no newline'], + [endingAt(4097, M, ''), M, '', 'ends the file at byte 4097 with no newline'], + // A longer word straddling the cap: `## PLAN COMPLETE` ends at 4096 and `X` is byte + // 4097. Cutting at 4096 alone would read the marker; the boundary byte is read too. + [endingAt(4096, '## PLAN COMPLETE', 'X\n'), '## PLAN COMPLETE', '', 'a longer word cut at the cap'], + [endingAt(4096, '## PLAN COMPLETE', ' x\n'), '## PLAN COMPLETE', '## PLAN COMPLETE', 'control: a real boundary at byte 4097'], + ]; + let ran = 0; + for (const [content, marker, expected, label] of rows) { + assert.equal(Buffer.byteLength(content.split('\n').slice(0, 2).join('\n')) >= 4095, true, `${label}: fixture length`); + assert.equal(run(content, marker), expected, label); + ran += 1; + } + assert.equal(ran, rows.length); + }); + + // Review 5563209291 Minor 6: only a BOM at the start of the file is stripped. + test('gsd_return_marker strips a BOM on line 1 only', (t) => { + const f = fixture(t); + const run = (content) => { + fs.writeFileSync(f.receipt, content); + return runHelpers(`gsd_return_marker ${q(fwd(f.receipt))} ${q('## ISSUES FOUND')}`); + }; + assert.equal(run('\ufeff## ISSUES FOUND\n'), '## ISSUES FOUND', 'a leading BOM on line 1 is tolerated'); + assert.equal(run('note\n\ufeff## ISSUES FOUND\n'), '', 'a BOM opening line 2 is not stripped, so the line does not start with the marker'); + assert.equal(run('note\n## ISSUES FOUND\n'), '## ISSUES FOUND', 'control: the same line 2 without a BOM matches'); + }); + + test('gsd_return_marker: a marker ends at end of line, whitespace, CR or a colon; `-x` or a letter is not a boundary', (t) => { + const f = fixture(t); + const m = ` ${q('## PLAN COMPLETE')} ${q('## ⚠ Source Audit')}`; + const run = (content) => { + fs.writeFileSync(f.receipt, content); + return runHelpers(`gsd_return_marker ${q(fwd(f.receipt))}${m}`); + }; + const cases = [ + ['## PLAN COMPLETE-x\n', ''], + ['## PLAN COMPLETE.x\n', ''], + ['## PLAN COMPLETEé\n', ''], + ['## PLAN COMPLETE x\n', '## PLAN COMPLETE'], + ['## PLAN COMPLETE\tx\n', '## PLAN COMPLETE'], + ['## PLAN COMPLETE\r\n', '## PLAN COMPLETE'], + ['## PLAN COMPLETE\rx\n', '## PLAN COMPLETE'], + ['## PLAN COMPLETE\n', '## PLAN COMPLETE'], + ['## ⚠ Source Audit: Unplanned Items Found\n', '## ⚠ Source Audit'], + ]; + let ran = 0; + for (const [content, expected] of cases) { + assert.equal(run(content), expected, JSON.stringify(content)); + ran += 1; + } + assert.equal(ran, cases.length); + }); + + test('a spaced PHASE_DIR with a fresh *-PLAN.md reads as active, not stalled (the glob is expanded quoted)', (t) => { + const f = fixture(t); + const markers = watchCallOf(sectionOf(readPlanPhase(), PLANNER_SECTION)).markers; + const spaced = path.join(f.dir, 'sp ace', 'ph'); + fs.mkdirSync(spaced, { recursive: true }); + const glob = `${fwd(spaced)}/*-PLAN.md`; + assert.equal(watch(NOW - 11 * 60, f.receipt, glob, markers, { interval: 5 }), 'stalled', 'control: no plan yet, past the threshold'); + fs.writeFileSync(path.join(spaced, '01-PLAN.md'), '# plan\n'); + assert.equal(watch(NOW - 11 * 60, f.receipt, glob, markers, { interval: 5 }), 'active', 'a fresh plan in a spaced phase dir is activity'); + // A literal (non-glob) artifact path with a space, as the chunked sites pass it. + assert.equal(watch(NOW - 11 * 60, f.receipt, `${fwd(spaced)}/01-PLAN.md`, markers, { interval: 5 }), 'active'); + }); + + // -- Review 5477738387 Minor 3: the cleanup is a command at each watch site, not + // helper prose only. The clause lives in the site's ORCHESTRATOR RULE paragraph, + // after its gsd_stall_watch call, and names stalled and Retry, so every route of + // that site (success, ISSUES FOUND, stalled, Retry) removes its receipt. + function rulePara(section) { + const at = section.indexOf('ORCHESTRATOR RULE'); + assert.notEqual(at, -1, 'section must hold an ORCHESTRATOR RULE paragraph'); + const stop = section.indexOf('\n\n', at); + return section.slice(at, stop === -1 ? undefined : stop); + } + + test('every watch site removes its receipt after routing any result, stalled and Retry included', () => { + const plan = readPlanPhase(); + const chunked = readChunkedPlanningMode(); + const sites = [ + ['planner', sectionOf(plan, PLANNER_SECTION)], + ['checker', sectionOf(plan, CHECKER_SECTION)], + ['revision', sectionOf(plan, REVISION_SECTION)], + ['chunked outline', sectionOf(chunked, ['### 8.5.1 Outline Phase', '### 8.5.2 Per-Plan Tasks'])], + ['chunked per-plan', sectionOf(chunked, ['### 8.5.2 Per-Plan Tasks', '\u0000'])], + ]; + let ran = 0; + for (const [label, section] of sites) { + const para = rulePara(section); + const call = para.search(/gsd_stall_watch\s+"\$TS"\s+"\{receipt\}"/); + assert.notEqual(call, -1, `${label}: the rule paragraph holds the watch call`); + assert.match(para.slice(call), /after routing any result \(stalled\/Retry too\): `rm -f "\{receipt\}"`/, + `${label}: the watch rule must end with the receipt cleanup for every route`); + ran += 1; + } + assert.equal(ran, 5); + }); + + test('helpers doc: a site with no receipt and no fresh artifact can only end stalled at the threshold', () => { + const doc = readStallHelpersDoc(); + const at = doc.indexOf('**Known limit, kimi-code:**'); + assert.notEqual(at, -1); + const near = doc.slice(at, at + 900); + assert.match(near, /checker[^\n]*no receipt|no receipt[^\n]*checker/i, 'the checker-without-receipt outcome is stated beside the kimi-code note'); + assert.match(near, /first `?PLAN\.md`?/, 'the planner before its first PLAN.md is named'); + assert.match(near, /`stalled` at the threshold/, 'the outcome is stalled at the threshold'); + }); + + // gsd_receipt_path fails closed (rc=1, no output); the orchestrator must know what to + // bind then, and an empty receipt argument must behave as "no receipt", never as a match. + test('a refused receipt (gsd_receipt_path rc=1) drops the prompt line and watches with an empty receipt', (t) => { + const f = fixture(t); + const doc = readStallHelpersDoc(); + assert.match(doc, /If `gsd_receipt_path` prints nothing[\s\S]{0,200}?omit the `` line\s+and pass `""` as the watch's second argument/, + 'the helpers doc states the binding for a refused receipt'); + const checker = watchCallOf(sectionOf(readPlanPhase(), CHECKER_SECTION)); + assert.equal(watch(NOW, '', f.noPlans, checker.markers), 'waiting', 'an empty receipt is no return'); + assert.equal(watch(NOW - 11 * 60, '', f.noPlans, checker.markers), 'stalled', 'past the threshold an empty receipt ends stalled'); + }); + + // -- Review 5477738387 Minor 5 / 5563209291 Minor 5: the *-PLAN.md freshness window + // (find -mmin -INTERVAL) at its edge, with no wall clock. A `find` shim (a shell + // function shadows the binary for this script) logs its argv and reports a controlled + // age, AGE seconds, for every path it is handed: a path is printed iff AGE is under + // the -mmin window. The rows pin what gsd_stall_watch hands find and how it reads the + // answer; real find's -mmin parsing is covered by the #2650 row above. + test('*-PLAN.md freshness boundary: a plan just inside the interval is active, just outside is waiting', (t) => { + const f = fixture(t); + const markers = watchCallOf(sectionOf(readPlanPhase(), PLANNER_SECTION)).markers; + const plan = path.join(f.dir, '01-PLAN.md'); + fs.writeFileSync(plan, '# plan\n'); + const argvLog = path.join(f.dir, 'find-argv.txt'); + const shim = [ + 'find() {', + ` printf '%s\\n' "$@" > ${q(fwd(argvLog))}`, + ' local mins="" a; local -a paths=()', + ' while [ $# -gt 0 ]; do case "$1" in -mmin) mins="${2#-}"; shift 2 ;; *) paths+=("$1"); shift ;; esac; done', + ' for a in "${paths[@]}"; do [ "$AGE" -lt $(( mins * 60 )) ] && printf \'%s\\n\' "$a"; done', + ' return 0', + '}', + ].join('\n'); + const aged = (age, interval, glob = `${f.fwd}/*-PLAN.md`) => { + cleanup(argvLog); + const call = `gsd_stall_watch ${q(NOW)} ${q(f.receipt)} ${q(glob)}${markers.map((x) => ` ${q(x)}`).join('')}`; + return runHelpers(`${shim}\nAGE=${age}\n${call}`, { interval, threshold: 99999 }); + }; + assert.equal(aged(299, 5), 'active', '4m59s old: inside a 5-minute interval'); + assert.deepEqual(splitLines(fs.readFileSync(argvLog, 'utf8')).filter(Boolean), [fwd(plan), '-mmin', '-5'], + 'find is handed the expanded plan path and the interval as -mmin -N'); + assert.equal(aged(300, 5), 'waiting', '5m00s old: outside a 5-minute interval'); + assert.equal(aged(300, 7), 'active', 'the window follows the interval: 5m00s is inside 7 minutes'); + assert.equal(splitLines(fs.readFileSync(argvLog, 'utf8')).filter(Boolean).pop(), '-7'); + assert.equal(aged(0, 5, `${f.fwd}/none-*-PLAN.md`), 'waiting', 'no plan file: find is not run and there is no activity'); + assert.equal(fs.existsSync(argvLog), false, 'find is not called when the glob matches nothing'); + }); +});