fix: align sync and async read contracts

[steipete]

Summary

• add the operational read-failed code and preserve underlying filesystem failures in cause
• align sync and async secret, FileStore, and temp-workspace reads on semantic path-state codes
• report an existing non-directory ancestor as not-file from both assertNoSymlinkParents() variants on every platform
• document every compatibility migration and pin the contracts with side-by-side regressions

The sync/async audit in #118 found four public error-code divergences. This resolves them without weakening confinement or identity checks: genuine I/O failures are no longer labeled as invalid input or path substitution, stable directories are not-file, and allowMissing suppresses only a truly absent suffix.

Windows reports raw ENOENT for file\\child, while POSIX reports ENOTDIR. The guard now detects that the existing non-final file component is not a directory before probing its child, so both async and sync APIs return the semantic not-file code on every OS.

The follow-up drift pass also found that stable hardlinks and symlinks were collapsed by synchronous FileStore/temp reads, missing temp leaves surfaced raw ENOENT, and I/O errors varied by twin. The same typed validation path now preserves hardlink, symlink, and not-found, while actual ambiguity remains path-mismatch.

Compatibility

Consumers matching synchronous secret invalid-path, asynchronous secret or synchronous store path-mismatch, or top-level Node I/O codes for read failures should match read-failed and inspect cause.code. Consumers treating synchronous directory/link errors as missing or identity drift should match not-file, hardlink, or symlink as appropriate. Callers that relied on a non-directory ancestor being accepted under allowMissing should handle not-file instead.

Verification

• focused parity and adjacent regression suites: 117 passed, 2 platform skips
• native Windows AWS Crabbox cbx_c7ef28949e23, run run_a442f6cf53a1: raw async/sync ENOENT, public async/sync not-file; parity suite 7 passed, 2 POSIX skips
• pnpm check: 1,013 passed, 59 platform skips
• pnpm test:security: 70 passed
• git diff --check
• Codex autoreview (gpt-5.6-sol, high): clean, no accepted/actionable findings
• comparable native-backed coverage: lines 93.18% (6,566/7,046) before, 93.12% (6,587/7,073) after; branches 86.26% before, 86.29% after; 9 added passing tests

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 3, 2026, 4:30 AM ET / 08:30 UTC.

ClawSweeper review

What this changes

This PR adds the public read-failed error code and aligns synchronous and asynchronous secret, FileStore, temporary-workspace, and symlink-parent reads on semantic filesystem error codes.

Merge readiness

⛔ Blocked until real behavior proof is added - 4 items remain

This PR remains necessary: current main still exposes the reported sync/async read-contract divergences, and the proposed patch appears to preserve the package’s guarded-read boundary while normalizing public error outcomes. No discrete correctness defect was found in the reviewed patch, but merge should wait for inspectable after-fix runtime proof rather than the PR body’s unlinked claims alone.

Priority: P2
Reviewed head: fec0e7bfc99bd73de56d9fce5bca6ca2d1c6ebcc

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is a well-scoped, documented contract repair with strong regression intent, but it remains proof-gated until real after-fix runtime evidence is available to reviewers.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The PR body reports a Windows Crabbox run and test totals, but the available body, comments, and scratch artifacts contain no inspectable after-fix terminal output, recording, linked artifact, or redacted runtime log. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The PR body reports a Windows Crabbox run and test totals, but the available body, comments, and scratch artifacts contain no inspectable after-fix terminal output, recording, linked artifact, or redacted runtime log. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	7 items	Current main has the FileStore divergence: The synchronous FileStore read path treats validation failures such as a directory, hardlink, or symlink race as path-mismatch, whereas the PR changes that path to preserve already-typed semantic errors. Current main has the temporary-workspace divergence: The synchronous temporary-workspace reader fabricates raw ENOENT whenever the guarded open fails, discarding specific guarded-open outcomes; the proposed patch routes this through typed failure handling. Current main has the secret I/O-code divergence: The sync secret reader maps most post-open failures to invalid-path, while the async reader maps non-not-found Node errors to path-mismatch; the PR introduces the common operational read-failed outcome with the original cause.
Findings	None	None.
Security	None	None.

How this fits together

fs-safe turns guarded filesystem reads for secrets, rooted stores, and temporary workspaces into bytes or typed public errors. Path traversal, link/type, pinned-descriptor, and identity checks run before the read outcome is normalized for callers.

flowchart LR
  Caller[Application caller] --> APIs[Secret, store, and workspace reads]
  APIs --> Guard[Path, type, link, and identity guards]
  Guard --> Pinned[Pinned file open]
  Pinned --> Normalize[Normalize filesystem failures]
  Normalize --> Result[Bytes or typed public error]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The PR body reports a Windows Crabbox run and test totals, but the available body, comments, and scratch artifacts contain no inspectable after-fix terminal output, recording, linked artifact, or redacted runtime log. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve merge risk (P1) - This is a public compatibility migration: consumers matching legacy invalid-path, path-mismatch, raw Node I/O errors, or treating allowMissing as accepting a non-directory ancestor must adapt to the documented semantic codes.
• Resolve merge risk (P1) - The PR body reports native Windows proof, but no inspectable terminal transcript, runtime log, recording, or linked artifact is available in the supplied discussion to verify the claimed after-fix behavior.
• Complete next step (P2) - The remaining merge gate is contributor-supplied real behavior proof rather than a narrow repository repair for an automated worker.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	21 files affected; +406/-38 lines	The change updates guarded read behavior, exported error types, user documentation, changelog compatibility notes, and regression coverage together.
Production versus tests	production +126/-27, tests +245/-4	Most growth is focused parity coverage; the production change is justified by four existing public contract divergences.

Merge-risk options

Maintainer options:

1. Add inspectable runtime proof before merge (recommended)
   Attach or link a redacted native runtime transcript that shows the public sync/async results and the preserved underlying read-failure cause before accepting this intentional error-code migration.
2. Accept the compatibility migration with explicit release review
   A maintainer may accept the documented breaking error-code transition based on the expanded regression suite, while owning the absence of inspectable external runtime proof.

Technical review

Best possible solution:

Merge the documented parity migration after a redacted real-runtime transcript demonstrates both sync and async public outcomes, including Windows non-directory ancestry and a post-validation read failure retaining cause.code.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level. Current main visibly maps the relevant sync and async failures differently in FileStore, temporary-workspace, secret-file, and symlink-parent paths; the PR adds focused parity regressions, though this read-only review did not execute them.

Is this the best way to solve the issue?

Yes. Reusing existing guarded-open and typed FsSafeError outcomes is narrower and safer than adding separate sync-only fallback logic; the remaining requirement is proof that the public migration behaves as documented on a real runtime.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5f528c322461.

Labels

Label justifications:

• P2: This is a bounded correction to public filesystem-read contracts with real caller compatibility impact but no demonstrated availability or security regression.
• merge-risk: 🚨 compatibility: Merging changes observable error codes and allowMissing behavior for existing consumers across several exported read APIs.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports a Windows Crabbox run and test totals, but the available body, comments, and scratch artifacts contain no inspectable after-fix terminal output, recording, linked artifact, or redacted runtime log. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Current main has the FileStore divergence: The synchronous FileStore read path treats validation failures such as a directory, hardlink, or symlink race as path-mismatch, whereas the PR changes that path to preserve already-typed semantic errors. (src/file-store.ts:132, 5f528c322461)
• Current main has the temporary-workspace divergence: The synchronous temporary-workspace reader fabricates raw ENOENT whenever the guarded open fails, discarding specific guarded-open outcomes; the proposed patch routes this through typed failure handling. (src/private-temp-workspace.ts:256, 5f528c322461)
• Current main has the secret I/O-code divergence: The sync secret reader maps most post-open failures to invalid-path, while the async reader maps non-not-found Node errors to path-mismatch; the PR introduces the common operational read-failed outcome with the original cause. (src/secret-file.ts:146, 5f528c322461)
• Current main accepts an ambiguous non-directory ancestor on the async guard: The async symlink-parent guard accepts every error classified by isNotFoundPathError when allowMissing is enabled; the PR’s follow-up commit explicitly changes the path walk to reject an existing non-directory ancestor as not-file before platform-specific ENOTDIR/ENOENT handling can diverge. (src/symlink-parents.ts:47, 5f528c322461)
• Focused regression coverage is included: The branch adds a 238-line parity regression suite and updates existing sync validation expectations for hardlinks, symlinks, directories, public API snapshots, and documentation contracts. (test/sync-async-contracts.test.ts:1, fec0e7bfc99b)
• Feature-history provenance: The behavior on current main primarily dates to the v0.5.2 release commit, while the immediately preceding merged sync/async secret-read correction established the adjacent guarded-read contract. The current PR is a distinct follow-up rather than an obsolete duplicate. (src/secret-file.ts:136, 240448104ab4)

Likely related people:

• steipete: Git history attributes the released guarded-read surfaces, the merged adjacent secret-read contract fix, and both commits in this PR to Peter Steinberger (steipete). (role: feature owner and recent area contributor; confidence: high; commits: 32759e8923a1, 240448104ab4, 81871a604aa2; files: src/errors.ts, src/file-store.ts, src/private-temp-workspace.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Attach a redacted native Windows terminal transcript showing raw filesystem behavior and matching public sync/async not-file output.
• Attach a redacted live output or runtime log for a post-validation read failure showing read-failed with its retained cause.code.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-08-03T07:06:38.127Z sha 81871a6 :: needs real behavior proof before merge. :: none