[PR #5581] [MERGED] fix(environment): report later bundle write failures as host failures #6978

New issue

Closed

opened 2026-11-01 21:44:27 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:27 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5581
Author: @kaoru0822-kitauji
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/environment-export-write-failure

📝 Commits (6)

4868dd9 fix(environment): report later bundle write failures as host failures
e5a71d9 Merge remote-tracking branch 'origin/main' into fix/environment-export-write-failure
7c63a0e fix(environment): name the host-refused path in export permission failures
3dbe5f0 fix(environment): attribute pathless bundle write failures to their file
e969529 fix(environment): attribute package tarball read failures to the tarball
92502e2 docs(environment): state which export failures name a path

📊 Changes

5 files changed (+414 additions, -37 deletions)

View changed files

📝 docs/cli.md (+6 -4)
📝 src/application/environment/environmentBundleWrite.test.ts (+167 -0)
📝 src/application/environment/environmentBundleWrite.ts (+112 -17)
📝 src/application/environment/environmentExport.ts (+74 -16)
📝 tests/boundary/cli/environmentExportCli.test.ts (+55 -0)

📄 Description

What broke: environment-export reported a host write failure on a later nested bundle file (e.g. ENAMETOOLONG) as an invalid directory argument (#5522).
Fix: only destination admission may refuse the directory argument; failures after admission become host write failures naming the failed path and the partial output to clean up.
Risk: low; error code changes only for post-admission write failures; Linux path-length repro verified by a reviewer, macOS covered by the CLI boundary test.

Summary

Fixes #5522. rea environment-export reported a path failure on a nested bundle file, which happens after the output directory has been admitted and other files written, as an invalid_request refusal of the directory argument. That response dropped the host cause and the absolute failed path. Such failures are now reported as host write failures.

Problem and expected behavior

Trigger: an absent destination whose canonical path fits the shorter bundle files but not artifacts/check_<name>/verify.py. On macOS this is a 986-byte path, per the issue. The host refuses that open with ENAMETOOLONG after README.md, the Dockerfile and env.toml are written. docs/cli.md describes later write failures as the host's failure, leaving the partial folder in place, and not as non-writing argument refusals.

Root cause: exportEnvironmentBundle mapped every ArtifactReaderFailure with reason path and no cleanup to a directory input issue. SafeOutputTree.write raises that same reason for a nested file it cannot create exclusively, so the mapping could not tell admission apart from a later write.

Change and scope

writeBundleFiles (environmentBundleWrite.ts) owns the admission phase. It turns a cleanup-free path refusal into the directory input issue only when SafeOutputTree.create itself failed. This follows the existing admitted pattern used for the package tarball.
exportEnvironmentBundle reports every other ArtifactReaderFailure as AnalysisOutputError. That is the same error type already used for raw host write failures. When the cause chain holds a host system error, the reason now appends that error's message, which carries the code and the absolute path.
Cleanup-free write failures get a user message, remediation (resolve the host failure, then inspect and remove partial output before retrying) and diagnostics.output_directory.
docs/cli.md lists pathname-length failures and says the report names the cause and failed path.

Not included: the occupied-root and permission-path cases (#5457/#5495, #5458) and Evidence staging (#5480). Scope fit: an ordinary fix within existing boundaries. No maintainer decision is needed.

Contract and boundary impact

Semantic owner and earliest changed stage: the environment bundle writer, at admission versus write phase.
Contribution scope fit and linked maintainer decision: not applicable (bug fix).
CLI and MCP/tool-catalog contract: no schema change. The error for a later nested path failure changes from invalid_request/invalid_input to unreadable_output, with details.reason naming the host cause and path. Admission refusals (non-empty or file-occupied directory) are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none. Partial output is still kept.
Target selection, authority, upstream behavior, and cleanup details: no change in cleanup ownership. A cleanup-failure merge still uses the raw primary failure.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: a new CLI boundary test in tests/boundary/cli/environmentExportCli.test.ts builds a real destination of PATH_MAX - 40 bytes (darwin 1024, linux 4096; skipped elsewhere). It asserts exit 1, unreadable_output, no details.issues, a reason containing ENAMETOOLONG and the absolute verify.py path, remediation naming the directory, and a retained README.md.
Base reproduction: with only the two source files reverted to origin/main, the new test fails (the response does not match code: "unreadable_output"). With the fix it passes.

User-visible CLI output (macOS; path shortened):

{"code":"unreadable_output","category":"execution_failure",
 "details":{"operation":"environment-export",
   "reason":"Could not exclusively create extraction path: artifacts/check_grafana-example/verify.py: ENAMETOOLONG: name too long, open '/private/var/folders/…/llll…/artifacts/check_grafana-example/verify.py'"}}

(captured before the user message, remediation and diagnostics were added; the test asserts those fields now)

Remaining proof gaps: run on macOS arm64 only. The Linux branch of the test has not been run locally, and Windows is not covered.
Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:focused -- tests/boundary/cli/environmentExportCli.test.ts src/application/environment/environmentExport.test.ts src/application/environment/environmentBundleWrite.test.ts src/application/environment/environmentInputCleanup.test.ts: 4 files, 26 tests passed (macOS arm64)
The same CLI boundary test file with the source fix reverted: 1 failed (the new test), 6 passed
npm run check:fast: typecheck and lint passed
npm run docs:check: passed
Review follow-up (7c63a0e56): a permission failure after earlier files now names the nested path the host refused, with output_directory as a diagnostic, instead of the bundle root. New real-filesystem test (destination made read-only after the first write) plus the existing read-only-destination test: environmentBundleWrite 8, environmentExport 11, environmentExportCli 7 passed (each file run separately; running the two src/application/environment files in one invocation fails four readEnvironmentExportInputs tests on the unchanged branch too); check:fast and format:check passed.
Review follow-up (3dbe5f01a): pathless descriptor failures (write/fsync, e.g. disk-full ENOSPC) are attributed to the bundle file being written; host-write recovery is applied only when a host system failure is present, so an integrity refusal (tarball rewritten after hashing) keeps its own message. Tests simulate the pathless ENOSPC and the integrity refusal on the second write: environmentBundleWrite 10, environmentExport 11, environmentExportCli 7 passed (each file separately); check:fast and format:check passed.
Review follow-up (e96952939): host failures are attributed by direction via BundleFileHostFailure (reading/writing); package tarball reads (hash pass and copy) name the caller-selected tarball with read recovery instead of the destination and disk-space advice. New test fails the tarball copy read with a pathless EIO: environmentBundleWrite 11, environmentExport 11, environmentExportCli 7 passed (each file separately); check:fast and format:check passed.
Review follow-up (92502e2c1): docs/cli.md and the host-write message now promise the failed bundle file or package tarball only when the failure is on one; a commit-time folder sync failure names only its cause (attributing SafeOutputTree.commit sync failures is a separate shared-boundary change). docs:check passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Only the error classification for failures after admission changes.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. macOS host filesystem only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the reported path is the caller-selected destination plus bundle-relative names. No new side effects.

Author readiness

Human owner: @kaoru0822-kitauji

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5581 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/20/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/environment-export-write-failure` --- ### 📝 Commits (6) - [`4868dd9`](https://github.com/morluto/rea/commit/4868dd9fbf29ae981f25ef33110a1223de793849) fix(environment): report later bundle write failures as host failures - [`e5a71d9`](https://github.com/morluto/rea/commit/e5a71d992b9bab16ca2fdcd77889afad69888ffd) Merge remote-tracking branch 'origin/main' into fix/environment-export-write-failure - [`7c63a0e`](https://github.com/morluto/rea/commit/7c63a0e5690cb36a9d08542a3e359e6bbf0b18fd) fix(environment): name the host-refused path in export permission failures - [`3dbe5f0`](https://github.com/morluto/rea/commit/3dbe5f01ae951162ae8d61a1e6014cefcd7bc431) fix(environment): attribute pathless bundle write failures to their file - [`e969529`](https://github.com/morluto/rea/commit/e96952939e4c1eab35c716ae7a4b9b7f311106af) fix(environment): attribute package tarball read failures to the tarball - [`92502e2`](https://github.com/morluto/rea/commit/92502e2c14a2c7d072261f5d342d5da8ad02c9d0) docs(environment): state which export failures name a path ### 📊 Changes **5 files changed** (+414 additions, -37 deletions) <details> <summary>View changed files</summary> 📝 `docs/cli.md` (+6 -4) 📝 `src/application/environment/environmentBundleWrite.test.ts` (+167 -0) 📝 `src/application/environment/environmentBundleWrite.ts` (+112 -17) 📝 `src/application/environment/environmentExport.ts` (+74 -16) 📝 `tests/boundary/cli/environmentExportCli.test.ts` (+55 -0) </details> ### 📄 Description **What broke:** `environment-export` reported a host write failure on a later nested bundle file (e.g. `ENAMETOOLONG`) as an invalid `directory` argument (#5522). **Fix:** only destination admission may refuse the `directory` argument; failures after admission become host write failures naming the failed path and the partial output to clean up. **Risk:** low; error code changes only for post-admission write failures; Linux path-length repro verified by a reviewer, macOS covered by the CLI boundary test. ## Summary Fixes #5522. `rea environment-export` reported a path failure on a nested bundle file, which happens after the output directory has been admitted and other files written, as an `invalid_request` refusal of the `directory` argument. That response dropped the host cause and the absolute failed path. Such failures are now reported as host write failures. ## Problem and expected behavior Trigger: an absent destination whose canonical path fits the shorter bundle files but not `artifacts/check_<name>/verify.py`. On macOS this is a 986-byte path, per the issue. The host refuses that open with `ENAMETOOLONG` after README.md, the Dockerfile and env.toml are written. `docs/cli.md` describes later write failures as the host's failure, leaving the partial folder in place, and not as non-writing argument refusals. Root cause: `exportEnvironmentBundle` mapped every `ArtifactReaderFailure` with reason `path` and no cleanup to a `directory` input issue. `SafeOutputTree.write` raises that same reason for a nested file it cannot create exclusively, so the mapping could not tell admission apart from a later write. ## Change and scope - `writeBundleFiles` (environmentBundleWrite.ts) owns the admission phase. It turns a cleanup-free `path` refusal into the `directory` input issue only when `SafeOutputTree.create` itself failed. This follows the existing `admitted` pattern used for the package tarball. - `exportEnvironmentBundle` reports every other `ArtifactReaderFailure` as `AnalysisOutputError`. That is the same error type already used for raw host write failures. When the cause chain holds a host system error, the reason now appends that error's message, which carries the code and the absolute path. - Cleanup-free write failures get a user message, remediation (resolve the host failure, then inspect and remove partial output before retrying) and `diagnostics.output_directory`. - docs/cli.md lists pathname-length failures and says the report names the cause and failed path. Not included: the occupied-root and permission-path cases (#5457/#5495, #5458) and Evidence staging (#5480). Scope fit: an ordinary fix within existing boundaries. No maintainer decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: the environment bundle writer, at admission versus write phase. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: no schema change. The error for a later nested path failure changes from `invalid_request`/`invalid_input` to `unreadable_output`, with `details.reason` naming the host cause and path. Admission refusals (non-empty or file-occupied directory) are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. Partial output is still kept. - Target selection, authority, upstream behavior, and cleanup details: no change in cleanup ownership. A cleanup-failure merge still uses the raw primary failure. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: a new CLI boundary test in `tests/boundary/cli/environmentExportCli.test.ts` builds a real destination of `PATH_MAX - 40` bytes (darwin 1024, linux 4096; skipped elsewhere). It asserts exit 1, `unreadable_output`, no `details.issues`, a reason containing `ENAMETOOLONG` and the absolute `verify.py` path, remediation naming the directory, and a retained README.md. - Base reproduction: with only the two source files reverted to `origin/main`, the new test fails (the response does not match `code: "unreadable_output"`). With the fix it passes. - User-visible CLI output (macOS; path shortened): ```json {"code":"unreadable_output","category":"execution_failure", "details":{"operation":"environment-export", "reason":"Could not exclusively create extraction path: artifacts/check_grafana-example/verify.py: ENAMETOOLONG: name too long, open '/private/var/folders/…/llll…/artifacts/check_grafana-example/verify.py'"}} ``` (captured before the user message, remediation and diagnostics were added; the test asserts those fields now) - Remaining proof gaps: run on macOS arm64 only. The Linux branch of the test has not been run locally, and Windows is not covered. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/boundary/cli/environmentExportCli.test.ts src/application/environment/environmentExport.test.ts src/application/environment/environmentBundleWrite.test.ts src/application/environment/environmentInputCleanup.test.ts`: 4 files, 26 tests passed (macOS arm64) - The same CLI boundary test file with the source fix reverted: 1 failed (the new test), 6 passed - `npm run check:fast`: typecheck and lint passed - `npm run docs:check`: passed - Review follow-up (7c63a0e56): a permission failure after earlier files now names the nested path the host refused, with `output_directory` as a diagnostic, instead of the bundle root. New real-filesystem test (destination made read-only after the first write) plus the existing read-only-destination test: `environmentBundleWrite` 8, `environmentExport` 11, `environmentExportCli` 7 passed (each file run separately; running the two `src/application/environment` files in one invocation fails four `readEnvironmentExportInputs` tests on the unchanged branch too); `check:fast` and `format:check` passed. - Review follow-up (3dbe5f01a): pathless descriptor failures (`write`/`fsync`, e.g. disk-full `ENOSPC`) are attributed to the bundle file being written; host-write recovery is applied only when a host system failure is present, so an integrity refusal (tarball rewritten after hashing) keeps its own message. Tests simulate the pathless `ENOSPC` and the integrity refusal on the second write: `environmentBundleWrite` 10, `environmentExport` 11, `environmentExportCli` 7 passed (each file separately); `check:fast` and `format:check` passed. - Review follow-up (e96952939): host failures are attributed by direction via `BundleFileHostFailure` (`reading`/`writing`); package tarball reads (hash pass and copy) name the caller-selected tarball with read recovery instead of the destination and disk-space advice. New test fails the tarball copy read with a pathless `EIO`: `environmentBundleWrite` 11, `environmentExport` 11, `environmentExportCli` 7 passed (each file separately); `check:fast` and `format:check` passed. - Review follow-up (92502e2c1): `docs/cli.md` and the host-write message now promise the failed bundle file or package tarball only when the failure is on one; a commit-time folder sync failure names only its cause (attributing `SafeOutputTree.commit` sync failures is a separate shared-boundary change). `docs:check` passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Only the error classification for failures after admission changes. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. macOS host filesystem only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the reported path is the caller-selected destination plus bundle-relative names. No new side effects. ## Author readiness Human owner: @kaoru0822-kitauji - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:44:27 +00:00