fix(#4939): answer a caller error for a missing phase dir

[drakeo338]

Fix PR

---

Linked Issue

Fixes #4939

The linked issue must have the confirmed-bug label. If it doesn't, ask a maintainer to confirm the bug before continuing.

---

What was broken

decision-coverage-plan/-verify never checked the phase-dir argument was a directory. Passing the phase number instead made loadPlanContents load zero plans, and both gates read that as real zero coverage.

What this fix does

Both gates now require the argument to be an existing directory that holds plans. A given value that is not a directory fails closed as a caller error (reason: "phase directory not found", total/covered null), and so does a directory with no PLAN file ("phase directory holds no plans"). Verify returns a non-blocking warning with honored/total null instead, and treats a directory with SUMMARY files but no plans as readable. Omitting the argument is unchanged.

Root cause

loadPlanContents answers [] for a missing path, indistinguishable from a phase whose plans genuinely cite nothing.

Testing

How I verified the fix

On head 9a9b488 (Linux, Node v26.5.1), after npm run build:

• node --test tests/decisions.test.cjs tests/gate-decision-coverage-plan.unit.test.cjs tests/gate-decision-coverage-verify.unit.test.cjs tests/check-router-cutover-equivalence.test.cjs: 454 passed, 0 failed.
• npm test: 44,529 passed, 9 failed, 98 skipped. The same 9 fail on a clean next (91e9519) in this checkout: check-env, escapeRegex/row-9 sweep (Node v26 RegExp.escape), emitted-attribution, gate-evaluation-scope and quick-batch, none of which touch these files.
• node scripts/benchmark-compact-content.cjs --check: baseline up to date. npx eslint tests/decisions.test.cjs: clean.

Regression test added?

• Yes — added a test that would have caught this bug

Platforms tested

• Linux
• macOS
• Windows (including backslash path handling)
• N/A (not platform-specific)

Runtimes tested

• Claude Code
• Antigravity
• OpenCode
• Other: ___
• N/A (not runtime-specific)

---

Standards followed

Bugfix within existing behavior, mirrors #2770/#4794's caller-error pattern; per docs/contributor-standards.md, no new ADR needed.

Checklist

• Issue linked above with Fixes #NNN — PR will be auto-closed if missing
• Linked issue has the confirmed-bug label
• Fix is scoped to the reported bug — no unrelated changes included
• Regression test added (or explained why not)
• All existing tests pass (npm test): the 9 failures above also fail on a clean next
• .changeset/ fragment added if this is a user-facing fix (npm run changeset -- --type Fixed --pr <NNN> --body "...") — or no-changelog label applied
• No unnecessary dependencies added

Breaking changes

None

[trek-e]

Review: changes requested

Zero-tolerance policy: every finding below (including Minor/Nit) must be resolved before approval. The queued CI runs for this head are being released so you have a signal to push against.

Head: pr-5030 (f726c57), author drakeo338, branch fix/4939-bug-decision-coverage-a-phase-dir-that-d.
Files: .changeset/wise-dogs-frolic.md, src/check-command-router.cts (+30), tests/decisions.test.cjs (+72).

Summary

Adds isExistingDirectory() (src/check-command-router.cts:171-177) and a guard in both
cmdDecisionCoveragePlan (:425-433) and cmdDecisionCoverageVerify (:539-545): a phase-dir
argument that is GIVEN but is not a directory now answers reason: "phase directory not found"
with total/covered (plan) or total/honored (verify) null instead of a measured-looking
covered: 0. Plan gate still fails closed (passed:false); verify stays non-blocking. Omitted
phase dir unchanged (pinned by #4130 tests at tests/decisions.test.cjs:2579-2598, 2647-2657).
Correct, minimal, fail-closed. Only minor/nit findings. Lint clean, build clean.

Acceptance checklist (issue #4939 + triage Agent Brief)

• Plan gate: phase NUMBER in phase-dir slot no longer reports covered: 0 — src:430-433; test tests/decisions.test.cjs (#4939 "plan gate — the phase NUMBER…").
• Treated as a caller error, fail closed, distinct reason — passed:false, skipped:false, reason:'phase directory not found'. (Issue's example used no-plans-found + numeric totals; the PR's null totals follow the #4794 "null not 0" convention — stricter, acceptable.)
• Message names the argument and says to pass the phase DIRECTORY — src:431.
• Verify gate parity as non-blocking warning — src:542-545; test "#4939: verify gate…".
• Regression tests for nonexistent-dir on both gates — present.
• Correct form unchanged (control test "#4939: control").
• [~] "exists but holds no plans" variant: triage brief made it optional ("if that distinction matters"); not implemented. Not required by the reproduction.

Prior findings status

No prior reviews or comments on the PR (reviews: [], comments: []). First review.

Root-cause verdict

Root cause per triage: loadPlanContents() (src:156-165) returns [] for a missing path,
identical to a genuinely empty scan. The PR distinguishes the case at both consumers before the
scan rather than changing loadPlanContents's return type — this is what the triage Agent Brief
prescribed and covers every consumer (get_impact upstream: only routeCheckCommand; no parallel
SDK handler found via find_code). Not special-casing/swallowing: the unmeasured state is surfaced
as a caller error with null metrics. Guard is placed after context/decision validation, so a
context with no trackable decisions still green-skips first (correct: nothing to check).

Memtrace Evidence

• get_impact(cmdDecisionCoveragePlan, upstream, node a84c023d, branch HEAD): LOW risk, 1 affected — routeCheckCommand (src:1835), process Gsd-coreRouteCheckCommand. (First unscoped call returned ambiguous: 4 candidates across HEAD and fix/4378-seed-id-collision.)
• get_symbol_context(loadPlanContents / cmdDecisionCoverageVerify): ambiguous — multiple declarations (HEAD + stale branch copies); node-id reissue returned "node UUID … is ambiguous in the requested graph scope". No traversal performed.
• recall_decision("decision coverage gate caller error phase directory missing null total covered"): no on-topic decision (hits: coverage-parity gate, resolves-only scope, ADR-612 bracket phase-ID, #3755 consensus gate).
• find_code_review_issues(diff, online, max 60): _graph_state: ready, 0 issues (ast 0, yaml 0, cross_module 0, online 0).
• find_code (is-directory helper): no existing canonical helper to reuse (closest isReadableDirectory in src/runtime-artifact-layout.cts:171, different module).
• npm run lint: clean. npm run build: complete.

Overlap with #5003 (issue #4987) and origin/next

• #5003 adds isPhaseDirAbsent() in src/verification.cts (ENOENT/ENOTDIR → absent; other errnos fall through) and status phase_dir_not_found. #5030 adds isExistingDirectory() in src/check-command-router.cts that maps ALL stat errors to "not found". No textual conflict (different files), but two new private is-dir predicates with divergent errno semantics and divergent vocab (phase directory not found vs phase_dir_not_found). Each is consistent with its own file's existing vocabulary (check gates use prose reasons; verification uses snake_case statuses), so vocab divergence is acceptable; the errno divergence is Nit 2 below.
• origin/next: no existing fix for #4939; PR three-dot diff is clean.

Findings

1. Minor — tests/decisions.test.cjs (#4939 describe, "a FILE in the phase-dir slot" test covers plan gate only). The verify branch src/check-command-router.cts:542 has no FILE-in-slot test, and no verify-side control (dir given → numeric honored). A regression that swaps verify's predicate back to fs.existsSync (true for a file) would not be caught. Fix: add the FILE-in-slot case for check.decision-coverage-verify asserting reason, honored:null, total:null.
2. Nit — src/check-command-router.cts:171-177 swallows every stat error (EACCES, ELOOP, EIO) as "not a directory", and the messages at :431/:543 then assert the directory "does not exist or is not a directory" — a wrong diagnosis for a permissions failure. Fail-closed outcome is right; the text is not. Fix: follow the #4794 pattern already in this function (src:369-380, contextKind) — capture the stat outcome (missing / not a directory / unreadable) and name it in the message.
3. Nit — documented output contracts not updated for the new nullable fields: gsd-core/references/verifier-phase-gates.md:41-48 still documents verify as { skipped, blocking:false, total, honored, not_honored, message } with numeric totals and instructs writing {honored,total} into decision_coverage; gsd-core/workflows/plan-phase.md:1360-1363 same for the plan gate. (Fixed changesets are exempt from the docs rule, so not blocking.) Fix: note reason and that total/covered/honored are null on caller-error outcomes.
4. Nit (pre-existing, same function, same defect class) — src/check-command-router.cts:514-531: verify's could-not-parse branch still answers total: decisions.length, honored: 0 (measured-looking zero), while the plan gate (#4794, :397-403) and this PR's new verify branch (:543) answer null. The function now has two inconsistent "unmeasured" shapes. Fix: null total/honored there too.

Changeset: type Fixed, pr: 5030 correct, bold lead + em dash + symptom-led, trailing (#4939). OK.
Branch name, conventional commits, Fixes #4939, template sections: OK. Test is behavioral (spawns the CLI), no source-grep, no chmod, no wall-clock. No limit → boundary rule N/A; not a parser → fast-check N/A.
Security: argument is interpolated only into a JSON string value emitted via output() (JSON.stringify); no shell/argv/path-write use. No injection, traversal or secrets.
CI: only policy checks have run (all pass); test matrix not yet run (fork approval) — informational.

Verdict

minor — correct root-cause fix with adequate tests for the reported case; four minor/nit items above
(one test gap, one misleading error text, two contract/consistency gaps). No blockers.

[drakeo338]

Pushed bcaadbd with all four findings addressed and the current next merged in; reason stays 'phase directory not found' for every stat outcome, following the contextKind pattern, while the message names the actual one.

[drakeo338]

@trek-e all four findings are addressed (bcaadbd) and the branch is up to date with next. Could you take another look when you have a moment?

[trek-e]

Re-review — #5030 (fixes #4939), head 834d06a94a

Prior findings — all resolved on this head

The FILE-test verify case, the stat-outcome message, the contract docs, and the verify could-not-parse null are all fixed.

Classification & gate compliance

confirmed-bug → Bug-Fix track; Fixes #4939, conventional title/branch, not a draft. No security defect found (no new argv/subprocess/path surface).

Memtrace Evidence

• get_impact (target cmdDecisionCoverageVerify, branch: next, upstream): LOW, 2 affected symbols — the sole caller routeCheckCommand (src/check-command-router.cts:1892-1995) and its file; process Gsd-coreRouteCheckCommand; graph_incomplete: false. (An earlier unscoped query was ambiguous across branches.)
• get_symbol_context (target cmdDecisionCoveragePlan, src/check-command-router.cts): unscoped query ambiguous across branches, no callers/callees — inconclusive; the plan guard shares the routeCheckCommand caller.
• recall_decision (query "decision coverage gate caller error null total covered unmeasured"): no on-topic decision. governing_rules on src/check-command-router.cts: ADR-443 only (unrelated).
• find_code_review_issues (online, max_candidates: 50) on the src/check-command-router.cts diff (tests/docs excluded): 0 issues, _graph_state: ready; weak evidence.

Findings

1. Minor [judgment] src/check-command-router.cts (plan guard ~:443, verify guard ~:557): reason stays "phase directory not found" even when the stat outcome is EACCES/EIO/file, so a machine consumer branching on reason cannot tell missing from unreadable (only the message text differs). You chose this deliberately; either emit distinct reason values or state in the contract docs that consumers must not branch on it.
2. Nit [judgment] tests/decisions.test.cjs (runGateWithStatFailure): t.mock.restoreAll() is not in a finally, so a throw from routeCheckCommand leaves fs.writeSync mocked until the test ends.
3. Nit [judgment] The compact-content baseline numbers were not independently recomputed in this pass; please confirm they were regenerated from this head.

CI

On this head only the policy checks had run and mergeStateStatus is BLOCKED; the fork test matrix was held at action_required. I am releasing those runs now; nothing has failed.

Verdict

Changes requested: findings 1–3 are open and the matrix has not executed.

[drakeo338]

Pushed 011bb78: reason is now distinct per stat outcome (not found, not a directory, unreadable) with docs and tests updated, mock restore moved into finally, and the benchmark baseline regenerated.

[trek-e]

Merge conflict gate: this branch does not merge cleanly into current next (f4be7ed), so review is paused until it is resolved. The following path(s) conflict (verified with git merge-tree --write-tree origin/next refs/pr/5030):

• tests/fixtures/compact-content-benchmark-baseline.json

This is a generated benchmark baseline; regenerate it after the rebase.

Rebase onto next (git fetch origin next && git rebase origin/next), resolve, regenerate any generated files rather than hand-merging them, and push. The full review runs once the branch merges cleanly.

[drakeo338]

Rebased onto next (f4be7ed) and regenerated the compact-content benchmark baseline with the repo's --write generator instead of hand-merging it (0413ddb).

[drakeo338]

Rebased onto next again; the phase-dir check now lives in the shared decision-coverage support module that both gates call, tests pass (8ef455f).

[drakeo338]

Rebased onto next (2cc432e) again and regenerated the compact-content benchmark baseline, decisions tests pass (6818eb9).

[trek-e]

Review — fix(#4939): answer a caller error for a missing phase dir (re-review)

Head reviewed: 59dadff141abae9d27e5057d7977c037503747b1 (two commits: 40f41bfdac, 59dadff141). Prior trek-e review at 011bb78243 was a COMMENTED merge-conflict pause; earlier CHANGES_REQUESTED reviews at f726c577b7 and 834d06a94a.

Prior findings

#	Finding (earlier reviews)	Status at head
1	Verify-side FILE-in-slot test + control missing	Resolved: tests/decisions.test.cjs #4939: verify gate — a FILE in the phase-dir slot… and …control: the phase DIRECTORY…
2	Stat-outcome message wrongly says "does not exist" for EACCES	Resolved: phaseDirProblem src/decision-coverage-support.cts (new fn, ~:63-82) names could not be read (code); EACCES tests for both gates
3	Contract docs not updated	Resolved: gsd-core/references/verifier-phase-gates.md:41-54, gsd-core/workflows/plan-phase.md:1367-1372
4	Verify could-not-parse still answers total: decisions.length, honored: 0	Code resolved (src/gate-decision-coverage-verify.cts:57-62 now null/null) BUT see Blocker 1 (existing tests/golden not updated)
5	reason collapsed all stat outcomes	Resolved: distinct phase directory not found / phase path is not a directory / phase directory unreadable
6	t.mock.restoreAll() not in finally	Resolved: tests/decisions.test.cjs runGateWithStatFailure uses try/finally
7	Baseline not independently recomputed	Not independently re-verified by me (generator script read blocked by Memtrace-first guard). Delta +81 offTokens/+81 onTokens on plan-phase only is consistent with the 6 lines added to plan-phase.md. Author states it was regenerated with --write. Confirm via CI.

Summary

Root-cause fix is sound: both decision-coverage gates now require a given phase-dir argument to be an existing directory (phaseDirProblem, shared in src/decision-coverage-support.cts) before loadPlanContents can answer []. The author also moved the check into the shared support module that both gates call (one owner for the concept). One new defect class introduced: the verify could-not-parse shape change (total/honored -> null) breaks existing tests and a golden that were not updated.

Classification & gate compliance

• Linked issue #4939: OPEN, labels bug, confirmed-bug -> Bug-Fix track. Body has Fixes #4939. Not a draft. Title fix(#4939): …, branch fix/4939-bug-decision-coverage-a-phase-dir-that-d.
• Changeset .changeset/wise-dogs-frolic.md: type Fixed, pr: 5030, bold lead + em dash, symptom-led. OK.
• Docs gate: Fixed -> exempt from docs/; contract docs updated anyway. No new references/workflows files -> no INVENTORY change.
• No competing open PR (gh pr list --search "4939 in:body" -> only #5030).
• Labels on PR: needs-review: merge-conflict is STALE (see Merge). review: changes requested present.

Merge state (verified this turn)

gh pr view: mergeable: MERGEABLE, mergeStateStatus: BEHIND, headRefOid 59dadff141abae9d27e5057d7977c037503747b1. git merge-tree --write-tree origin/next refs/pr/5030 exit 0 (tree 1c109bccba…), no conflicts. Behind origin/next by 2 (957faa55b6 docs #5175, b18e8c53b2 CI timeout history #5163), neither touches PR files; rebase is clean. The merge-conflict label should be removed.

Root-cause verdict

loadPlanContents answers [] for a missing path, indistinguishable from zero coverage. Guard placed after the context/decision checks and before the scan, only when phase-dir argument was given (omitted case pinned by #4130 tests). Fail-closed on plan gate, advisory on verify. Correct, not special-casing. ENOENT/ENOTDIR -> not found, everything else -> unreadable (fail-safe).

Blast radius

Callers: routeCheckCommand only (check.decision-coverage-plan / -verify). Output-shape change is the real radius: verify could-not-parse now null/null (consumers: gsd-core/references/verifier-phase-gates.md decision_coverage pass-through; existing golden + unit tests, below). Risk tier: Low for call graph, Medium for output contract.

Test quality

New #4939 describe in tests/decisions.test.cjs: behavioral (spawn CLI / in-process router), asserts specific reason/null totals/omitted uncovered, fs.statSync/writeSync mocked (no chmod), no wall-clock, no source-grep, restore in finally. Gaps are Minor (see 3, 4). Fail-first: the plan/verify NUMBER tests assert reason/null which base cannot produce (base returns covered: 0).

Co-change completeness

Not run: get_cochange_context not invoked (documented to time out on this repo). Manually found absent-from-diff files that assert the changed output: tests/gate-decision-coverage-verify.unit.test.cjs (U2d, U2e) and tests/fixtures/gate-cutover/decision-coverage-verify-could-not-parse.json (see Blocker 1).

Memtrace Evidence

• get_impact (evaluateDecisionCoveragePlan, upstream, depth 3): returned ambiguous: true, 2 declarations (src fn + test constant), risk: UNKNOWN, no traversal. Not a clean result. Earlier reviews of the same symbol (node-scoped) report LOW, sole caller routeCheckCommand.
• get_symbol_context (phaseDirProblem): found: false (new symbol; index is of next). No data. find_symbol(evaluateDecisionCoveragePlan) located src/gate-decision-coverage-plan.cts:35-150.
• recall_decision (query "decision coverage gate caller error phase directory not found null total covered unmeasured"): returned 10 hits, none on-topic (coverage-parity gate, ADR-612, #3755 consensus gate, resolves-only scope). governing_rules on src/gate-decision-coverage-plan.cts: governed: false, no rules.
• find_code_review_issues (online, max 40): I invoked it with a placeholder diff, not the real PR diff; _graph_state: ready, 0 issues. This is NOT evidence of a clean review; a prior reviewer's run on the real diff (online, max 60) returned 0 issues. find_yaml_rule_matches / find_cross_module_issues not run. detect_changes (4 changed files): affected_symbols: [] for all, i.e. index (of next) has no overlap with the PR-added symbols; not informative.
• find_code for consumers of the verify could-not-parse shape surfaced tests/gate-decision-coverage-verify.unit.test.cjs and tests/fixtures/gate-cutover/decision-coverage-verify-could-not-parse.json; confirmed by Read (below).

Findings

Blocker 1 [judgment, confirmed by reading] The verify could-not-parse change to total: null, honored: null (src/gate-decision-coverage-verify.cts:57-62) contradicts three existing expectations the PR did not update, so they fail at this head:

• tests/gate-decision-coverage-verify.unit.test.cjs:111-121 (U2d) expects total: 1, honored: 0.
• tests/gate-decision-coverage-verify.unit.test.cjs:124-143 (U2e) expects total: 0, honored: 0.
• tests/fixtures/gate-cutover/decision-coverage-verify-could-not-parse.json stdout has "total": 0, "honored": 0; tests/check-router-cutover-equivalence.test.cjs:82 does a byte-for-byte assert.strictEqual(actual.stdout, golden.stdout) for the E2 arm that runs this fixture.
  Fix: update U2d/U2e titles+expectations and regenerate the golden (via the repo's golden generation, not by hand). The PR body's "189 passed, 0 failed" is for the author's local run and does not cover these files; the CI matrix has not executed (below).

Blocker 2 [judgment] CI matrix never executed for this head: 9 fork runs sit at action_required (see CI). A PR whose matrix never ran is not approvable.

Minor 3 [judgment] tests/decisions.test.cjs: the ENOENT/ENOTDIR -> "not found" vs other-errno -> "unreadable" boundary is pinned only on the EACCES side. No test pins that ENOTDIR (e.g. <file>/child) maps to phase directory not found; a swap of the two branches in phaseDirProblem would pass. Add an ENOTDIR (and ENOENT) injection case for both gates.

Minor 4 [judgment] Docs: gsd-core/workflows/plan-phase.md:1367-1370 and gsd-core/references/verifier-phase-gates.md:42-46 contain over-long lines in otherwise wrapped prose (cosmetic); and the docs list 'could-not-parse' among caller errors while the plan-phase list also names "context path is not a file" but not "missing context path argument" (that arm still returns total: 0, covered: 0), so the stated "total/covered null on caller error" is incomplete for that reason. Either list it with its actual shape or state the exception.

Verdict

CHANGES REQUESTED (blocker). Substance of the #4939 fix is correct and prior findings 1-3,5,6 are resolved, but the verify could-not-parse null change breaks U2d, U2e and the E2 golden (unrelated tests not updated), and the matrix has not run. Stale needs-review: merge-conflict label: branch merges cleanly (behind 2, clean rebase).

CI (fresh this turn)

Head 59dadff141abae9d27e5057d7977c037503747b1, mergeStateStatus: BEHIND. statusCheckRollup (7): all SUCCESS (validate-target, Pull request template format, validate-title, Issue link required, GitGuardian, Socket x2). Pending-approval queue for this head: pending=9 — 36936713383 Default Flip Documentation; 36936713307 Validate Branch Name; 36936713442 Changeset Required; 36936713459 Docs Required; 36936713470 Install Smoke; 36936713407 Tests; 36936713503 Security Scan; 36936713613 Mutation Testing; 36936712973 Dependabot Auto-Merge. No failing check; none executed.

[drakeo338]

U2d/U2e and both could-not-parse goldens now expect null totals (goldens regenerated), ENOENT/ENOTDIR cases pin "not found" for both gates, and the doc lines are wrapped with the caller-error list completed; also rebased onto current next.

[trek-e]

Substance at head d8bbaf9395 is clean (the tree is identical to the one reviewed at 02fa81834a), but this branch now conflicts with next after #5171 (typed gate evidence) landed, so it cannot be updated or merged:

• src/decision-coverage-support.cts
• src/gate-decision-coverage-plan.cts
• src/gate-decision-coverage-verify.cts
• gsd-core/references/verifier-phase-gates.md
• tests/fixtures/compact-content-benchmark-baseline.json (regenerate with node scripts/benchmark-compact-content.cjs --write after the build; do not hand-merge)

Please merge origin/next into the branch, resolve the three src/ files against #5171's evidence/exit-status changes, and push. A re-review of the resolved delta will follow.

[drakeo338]

@trek-e merged next in (merge commit). The three src/ files keep #5171's typed evidence and exit-status model with the #4939 caller errors on top: null totals on a caller error, ENOENT/ENOTDIR as "not found", other stat failures through the unreadable path. The baseline is next's version regenerated with the script. I also trimmed the plan-phase.md note to fit the size cap.

[trek-e]

Review — fix(#4939): answer a caller error for a missing phase dir

Head reviewed: 82fdb30d44472a5e450617fba0531e2ae39c735c. Full diff read: yes.

Summary

Both decision-coverage gates now require a given phase-dir argument to be an existing directory via the shared phaseDirProblem (src/decision-coverage-support.cts) before loadPlanContents can answer []. The prior merge conflict with #5171 is resolved; the substance is correct. The test matrix has not executed.

Classification & gate compliance

• #4939 OPEN, confirmed-bug -> bug-fix track; Fixes #4939; conventional title; branch name valid; not a draft.
• Changeset .changeset/wise-dogs-frolic.md: Fixed, pr: 5030, bold lead, symptom-led, (#4939). CHANGELOG untouched.
• Docs: Fixed is exempt; contract docs (verifier-phase-gates.md, plan-phase.md) updated; Emitted-Drift-Ack-Growth trailer present for plan-phase.md. No new references/workflows files, so no INVENTORY change.
• No competing PR for #4939.
• Merge state (this turn): mergeable: MERGEABLE, mergeStateStatus: BEHIND, behind next by 34, git merge-tree --write-tree origin/next refs/pr/5030 exit 0 (clean). The needs-review: merge-conflict label is stale.

Functional checklist / root-cause verdict

Root cause: loadPlanContents answers [] for a path that is not there, indistinguishable from a phase whose plans cite nothing. The fix distinguishes this in the shared support module, only when the argument is given, after context/decision validation. ENOENT/ENOTDIR -> phase directory not found; other errnos -> phase directory unreadable (fail-safe); a file -> phase path is not a directory. Plan gate stays fail-closed with null totals; verify stays non-blocking with null honored/total. All #4939 acceptance items verified in code and tests; the regression tests assert reason and null totals, which base cannot produce (fail-first).

Blast radius

Only routeCheckCommand reaches the changed gates. Output-contract change: verify could-not-parse now null/null (unit U2d/U2e and both goldens updated; docs updated). Risk: Low (call graph), Medium (output contract).

Co-change completeness

Changeset, contract docs, goldens and unit cases are updated. Missing: gate-unit and cutover coverage of the new phase-dir outcomes (Finding 2).

CodeGraph Evidence

• codegraph_node target statEvidence and gateUnreadable: "not found" (the index does not parse .cts and describes next); source verified with git grep on refs/pr/5030: statEvidence maps ABSENT_CODES to none and any other code to unreadable.
• codegraph_impact / codegraph_callers targets evaluateDecisionCoveragePlan / evaluateDecisionCoverageVerify: no indexed data (.cts); callers from source and prior reviews: routeCheckCommand only.
• codegraph_callees target phaseDirProblem: new symbol, absent from the base index; callee from source: statEvidence.

Memtrace unavailable: monthly query quota exhausted (20972/20933, resets 2026-10-20)

Findings

1. Blocker [fact] CI matrix never executed for this head. action_required runs (9): 37172613252 Docs Required; 37172613279 Default Flip Documentation; 37172613284 Mutation Testing; 37172613208 Dependabot Auto-Merge; 37172613213 Validate Branch Name; 37172613258 Install Smoke; 37172613257 Tests; 37172613292 Security Scan; 37172613380 Changeset Required. Only the 7 policy checks ran (all SUCCESS). Not approvable until the matrix runs green.
2. Minor [judgment] tests/gate-decision-coverage-plan.unit.test.cjs, tests/gate-decision-coverage-verify.unit.test.cjs, tests/fixtures/gate-cutover/arms.cjs: no case or arm pins the phase-dir-not-found shape, so the golden equivalence suite does not guard the new output; only tests/decisions.test.cjs does. Add unit cases and E1/E2 arms with generated goldens.
3. Nit [judgment] The branch carries two merge commits (Merge upstream/next ...) and is 34 behind. Run git fetch origin next && git rebase origin/next, resolve, regenerate the compact-content baseline with node scripts/benchmark-compact-content.cjs --write after the build, and push.
4. Nit [judgment] docs/INVENTORY.md:589 description of gate-decision-coverage-verify.cjs omits the new caller-error outcomes.

Verdict

Request changes. The #4939 fix substance is correct and prior findings are resolved, but Finding 1 (matrix not executed) blocks, and Findings 2-4 are open.

[drakeo338]

Rebased onto current next (no merge commits): the branch is now three commits on top of 83aba44. The docs/INVENTORY.md conflict is resolved, the gate unit and cutover coverage and both INVENTORY descriptions are kept, and the baseline was regenerated with node scripts/benchmark-compact-content.cjs --write after the build. The nine action_required matrix runs need a maintainer's approval to start; I can't release them from a fork.

[trek-e]

Automated maintainer re-review of head 4bf35431b3. Zero-tolerance policy: every finding at any severity must be resolved before approval.

Review — fix(#4939): answer a caller error for a missing phase dir (RE-REVIEW)

Head reviewed: 4bf35431b305f35fe2e2d3a86a3cabadf167969c (gh pr view, this turn). mergeable: MERGEABLE, mergeStateStatus: BEHIND, reviewDecision: CHANGES_REQUESTED. Three commits on 83aba44, no merge commits.

Summary

Both decision-coverage gates call the shared phaseDirProblem (src/decision-coverage-support.cts:66-80) before loadPlanContents, only when a phase-dir argument was given. ENOENT/ENOTDIR -> phase directory not found; file -> phase path is not a directory; other stat errors -> phase directory unreadable (outcome unreadable, UNAVAILABLE). Plan gate fails closed with null total/covered; verify stays non-blocking with null honored/total. Verify could-not-parse now null/null (parity with #4794). Substance correct. Remaining blocker is CI: the matrix has not executed.

Prior findings status (review of 82fdb30, 2026-10-07 01:23)

1. Blocker, CI matrix never executed: UNRESOLVED (not author-fixable). gh run list --commit 4bf35431: Tests, Install Smoke, Mutation Testing, Security Scan, Docs Required, Changeset Required, Validate Branch Name, Default Flip Documentation, Dependabot Auto-Merge all completed action_required. Only the 4 policy workflows (Target, Template Format, Title, Issue Link) plus GitGuardian and Socket x2 ran, all SUCCESS.
2. Minor, no unit cases/cutover arms for new outcomes: RESOLVED. tests/gate-decision-coverage-plan.unit.test.cjs U1o/U1p/U1q; tests/gate-decision-coverage-verify.unit.test.cjs U2o/U2p/U2q; tests/fixtures/gate-cutover/arms.cjs:542-544 (E1 x2), :556-557 (E2 x2) with four new goldens under tests/fixtures/gate-cutover/.
3. Nit, merge commits and 34 behind: PARTIAL. Merge commits gone (3 linear commits). Now behind by 1 (ed0f3fd, touches only tests/ci-timeout-budget-history.jsonl; git merge-tree exit 0, clean). Baseline regenerated (commit 4bf3543).
4. Nit, docs/INVENTORY.md:589 omitted new outcomes: RESOLVED (both gate rows at docs/INVENTORY.md:590-591 now list the three reasons and null totals).

Acceptance checklist (issue #4939, verbatim items)

• "The gate should treat '0 plans loaded' as a caller error, the same way #2770 ... and #4794": MET. src/gate-decision-coverage-plan.cts:~160-169 (block, reason, null totals). Verified: nonexistent dir path.
• "cmdDecisionCoverageVerify ... has the same shape, as a non-blocking warning": MET. src/gate-decision-coverage-verify.cts:~108-115 (gateVerdict('advisory', false, ...)).
• Agent Brief: distinct fail-closed reason: MET (not no-plans-found; prose reason per the file's vocabulary, acceptable).
• Brief: regression tests for both gates asserting caller-error shape: MET (tests/decisions.test.cjs new describe, plus unit and golden arms).
• Brief: "exists but holds no plans" distinction: optional per brief ("if that distinction matters"); not implemented. Not required by the reproduction.
• Correct form unchanged: MET (control tests in tests/decisions.test.cjs; omitted phase-dir left to scan, #4130 behavior).

Findings

1. Blocker | CI | PR-level: 9 matrix workflows at action_required for 4bf3543; a PR whose matrix never executed is not approvable. Fix: maintainer releases fork runs; surface results verbatim.
2. Nit | process | branch is BEHIND next by 1 (ed0f3fd, jsonl-only, clean). Fix: git fetch origin && git rebase origin/next (no conflicts expected); re-verify baseline unaffected.
3. Minor | co-change | docs/CONFIGURATION.md:1440-1445 ("Invoking the plan gate directly") documents the missing-context caller error but not the new phase-dir caller errors (phase directory not found / phase path is not a directory / phase directory unreadable). Fixed changesets are docs-exempt, but the contract prose lives here. Fix: add one sentence.
4. Minor | tests | tests/decisions.test.cjs (EACCES cases) and unit U1q/U2q assert outcome/JSON but no case asserts the process exit status for the unreadable arm (UNAVAILABLE/69) through the router; the cutover goldens only cover not-found/is-file (exitCode 0). A swap of gateUnreadable for gateVerdict('block') in phaseDirProblem consumers would keep JSON identical except outcome. U1q/U2q assert outcome: 'unreadable' so this is partly covered at the unit level; add a router-level exit assertion or an injected-stat cutover arm.
5. Nit | changeset | .changeset/wise-dogs-frolic.md names only reason: "phase directory not found" for "both gates" yet the gates answer three reasons (a file gives phase path is not a directory); the later sentence covers unreadable but not the file case. Fix: list the file reason.
6. Nit | PR body | "Ran npm test: 189 passed" is a stale local claim; Platforms list Linux only. Runner verdict (gsd-test) is not shown in the PR; confirm out of band.

Unverified (could not run, per rules): compact-content baseline numbers (+56 tokens off/on, plausible for the 4 lines added to plan-phase.md) and the gsd-test verdict; no source files executed.

Blast radius

codegraph callers: evaluateDecisionCoveragePlan / evaluateDecisionCoverageVerify -> cmdDecisionCoveragePlan/Verify in the router (routeCheckCommand) and tests only. loadPlanContents callers: the two gates only, both guarded now. phaseDirProblem new, called only from the two gates. Index describes next (.cts not parsed for the new symbol), so counts come from source. Output-contract change: verify could-not-parse total/honored 0 -> null (U2d/U2e and both could-not-parse goldens updated; docs/references/verifier-phase-gates.md updated). Risk Low (graph), Medium (contract).

Root cause

loadPlanContents answers [] for an absent path, identical to a plan set citing nothing. Fix distinguishes the case in one shared module before the scan; no catch swallows a sentinel; every caller (both gates) handles the new result; one owner (statEvidence). Not special-cased.

Security

phase-dir arg already contained by resolveContainedPath before use; statEvidence follows symlinks like the prior scan did; stat/scan TOCTOU degrades to unreadable from the scan (fail-safe). Non-ENOENT/ENOTDIR stat errors never read as "missing". No new argv/prompt surface.

Gate/format compliance

confirmed-bug issue, bug track, Fixes #4939, conventional title, not draft, branch valid, changeset Fixed pr: 5030 with trailing (#4939), docs updated (verifier-phase-gates.md, plan-phase.md, INVENTORY.md), Emitted-Drift-Ack-Growth trailer present for plan-phase.md. CONTEXT.md: no decision-coverage vocabulary, no update needed. No new references/workflows files so no manifest change.

Co-change gaps

docs/CONFIGURATION.md (Finding 3); CTS/CJS parity: gates are .cts compiled, no hand-written CJS twin.

Behind-by / rebase

BEHIND by 1 (ed0f3fd tests/ci-timeout-budget-history.jsonl); clean per git merge-tree exit 0.

Verdict

Substance clean / approvable on code; changes requested only for CI (matrix unexecuted, orchestrator releases), plus Minor 3-4 and Nits under zero tolerance.

Memtrace Evidence

Memtrace quota was exhausted for this review run, so the graph-backed checks were NOT executed; this is stated rather than reported as passed:

• get_impact: not run (quota). Blast radius above comes from codegraph where present.
• get_symbol_context: not run (quota). Callers/callees were read via codegraph where present.
• recall_decision: not run (quota). The recorded-decision gate was NOT evaluated; this is not a finding that no decision governs the change.
• find_code_review_issues: not run (quota). The deterministic AST/YAML/cross-module pass did not execute; findings above are judgment-sourced unless marked deterministic.

CI runs for this head are left queued (action_required) because Minor-or-higher findings are open; release will follow once they are addressed. The fix push re-triggers CI once.

[drakeo338]

Merged current next, documented the three phase-dir caller errors in the configuration docs, and pinned exit 69 for the unreadable phase dir in the decisions test

[trek-e]

Review — fix(#4939): answer a caller error for a missing phase dir

Head b9de0e8dd98c488abb4889d589a3e9fb06dad940. Zero-tolerance policy applies.

Summary

Shared phaseDirProblem gates both decision-coverage gates before loadPlanContents. Prior Minor findings (CONFIGURATION.md, exit-69 test) and the changeset nit are resolved.

Classification & gate compliance

confirmed-bug, Fixes #4939, conventional title, not draft, changeset Fixed pr: 5030, docs updated. No competing PR.

Root-cause verdict

loadPlanContents answers [] for a path that is not there, indistinguishable from zero coverage; fixed at the shared owner, not special-cased.

Blast radius

Only routeCheckCommand reaches the gates; verify output contract changes (Major 1).

Test quality

Regression tests assert reason + null totals (fail on base); exit 69 pinned through the router; unit and cutover goldens added. No source-grep or timing assertions.

Findings

1. Blocker CI: 9 matrix workflows unexecuted (action_required).
2. Major Scope: verify could-not-parse total/honored 0 -> null (gate-decision-coverage-verify.cts, U2d/U2e, two goldens, verifier-phase-gates.md, changeset clause) is not in #4939. Remove or split out.
3. Major #4939 asks that "holds no plans" / "0 plans loaded" be a caller error; an existing phase dir with zero plans still reports covered: 0 (decision-coverage-support.cts:66-80, plan:160-169, verify:108-115). Implement with tests for both gates.
4. Nit 15 behind next with merge commits again; git fetch origin && git rebase origin/next (clean).
5. Nit PR body cites head 72e2bc33f and ticks "All existing tests pass" beside 9 listed failures.

Verdict

Changes requested.

Memtrace Evidence

The Memtrace MCP server was unavailable for this review (it failed to connect: memtrace not found in PATH), so no graph-backed check ran. None of the four checks below is a clean result.

• get_impact: not run. Blast radius comes from reading the diff and grepping callers.
• get_symbol_context: not run. Callers and callees of the changed symbols were checked by grep.
• recall_decision: not run. The recorded-decision gate did not execute, so this does not show that no decision governs the change. ADRs and CONTEXT.md predicates were checked by grep instead.
• find_code_review_issues: not run. The deterministic review pass did not run. A manual diff review stood in for it, so the findings above are judgment, not graph facts.

[drakeo338]

Pushed 9a9b488:

• Major 2: the verify could-not-parse change is reverted (source, U2d/U2e, both goldens, verifier-phase-gates.md, changeset).
• Major 3: a phase dir with no PLAN file is now reason: "phase directory holds no plans" with null totals in both gates (U1r, U2r). For verify, it needs no PLAN and no SUMMARY file, since the R4 fixture is summary-only.
• Nit 5: body updated.
• Nit 4: I'll rebase onto next next.