Skip to content

enhance(#3473): B4 — an unreadable directory must not report as an empty one (scope: UNREADABLE generalization) #4014

Description

@trek-e

Pre-submission checklist

  • I have confirmed this improves existing behavior — it does not add a new command, workflow, or concept
  • I have searched existing issues and this enhancement has not already been proposed
  • I have read CONTRIBUTING.md and understand I must wait for approved-enhancement before writing any code
  • I can clearly describe the concrete benefit — not just "it would be nicer"

Epic #3473's Phase 8 roll-up raised this and then dropped it: "B4's scope: UNREADABLE generalization — the structural half belongs to §8.4, which shipped without it." It was never given an owner, and the string UNREADABLE appears nowhere in ADR-3473. Filed now so the epic's last unowned item has one, rather than closing on a criterion with a silent gap.

What existing feature or behavior does this improve?

The return contract of findContextMdIn and its six callers — the filesystem half of ADR-3473 §8.4 ("failure is a value").

Current behavior

A directory that cannot be read and a directory that is genuinely empty are output-identical to every consumer.

SCOPE is frozen at src/planning-scope.cts:24-29 (complete | truncated | unscoped | unreadable), with worstScope / SCOPE_SEVERITY at src/planning-snapshot.cts:77-92. The vocabulary exists. The filesystem-listing path does not use it.

findContextMdIn (src/planning-workspace.cts:248) takes string[]the readdirSync happens in the caller, so the function structurally cannot express "I could not read that directory." Each of its six callers therefore invented its own side channel:

caller what it does with an unreadable dir
countPhasePlansAndSummaries (src/roadmap.cts:125-157) ad-hoc contextReadError string (:135-142,155), hasContext:false unchanged
runGapAnalysis (src/gap-checker.cts:193+) ad-hoc phase_dir_read_error (:349)
getPhaseFileStats (src/core-utils.cts:295-326) forwards scan.scope but swallows its own failed readdirSync (:303-313) and still emits hasContext:false
cmdInitPlanPhase, cmdInitPhaseOp, cmdInitManager (src/init.cts) nothing — has_context:false in the emitted JSON bundle

Why #3884 and #3885 did not cover it. #3884 (Phase 7) scoped §8.4 to argv, parseNamedArgs and --pick counts — the command-routing surface, nothing filesystem-shaped. #3885 (B5) shipped only naming: the two ad-hoc error strings above, alongside an unchanged hasContext:false. So a consumer reading the boolean still cannot distinguish absence from unreadability, which is precisely what §8.4 asks to eliminate.

This is the ADR's own signature shape one layer out: the failure and the success are output-identical.

Proposed behavior

A shared listing owner returns { files, scope } — using the existing frozen SCOPE enum rather than a fifth new vocabulary — and the six callers consume scope instead of each inventing a side channel. unreadable becomes distinguishable from unscoped at every consumer, including the init JSON bundles.

Reason and benefit

Why the current behavior is a problem. An EACCES/EROFS on a phase directory currently reports as "this phase has no context," which is a plausible, wrong answer that no caller can tell from the truth. It is the same laundering class as #1884 (a fatal errno surfacing as a retryable one), which epic #3473 already fixed in the locking path and guarded in #3987 via eslint-rules/no-swallowed-precondition.cjs — that rule does not reach this shape, because the classification here is a boolean rather than an errno set.

Concrete benefit. roadmap analyze, gap-checker and the three init bundles stop reporting a permissions failure as an empty phase.

Scope of changes

src/planning-workspace.cts    — findContextMdIn returns { files, scope }; owns the readdirSync
src/roadmap.cts               — consume scope, retire ad-hoc contextReadError
src/gap-checker.cts           — consume scope, retire ad-hoc phase_dir_read_error
src/core-utils.cts            — getPhaseFileStats: stop swallowing its own readdirSync (:303)
src/init.cts                  — three call sites; has_context gains an unreadable-distinct signal
docs/                         — the three CLI surfaces' documented output shape
.changeset/*.md               — Changed
tests/                        — failing-first per surface, incl. an unreadable-vs-empty identity row

This is a return-shape change on three CLI-contract surfacesinit's JSON bundles (has_context / has_research), roadmap analyze, and gap-checker. Expect golden-fixture and install-tree churn.

Breaking changes

Yes — deliberately. Consumers reading has_context:false today get one bucket; afterwards unreadable is distinguishable. That is the point of the change, and it is why this needs its own deliverable rather than being folded into an unrelated PR. The exact wire shape (an added field vs a widened enum) should be settled in the design step, since ADR-3180's own precedent is that null is the explicit non-answer and keys stay present.

Alternatives considered

Area

core

Metadata

Metadata

Assignees

No one assigned

    Labels

    approved-enhancementEnhancement approved — contributor may begin codingarea: corePROJECT.md, REQUIREMENTS.md, templatesenhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions