morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4558] [Bug] firmwareIntegration publication cleanup-retry tests fail deterministically since `2d163c12` #2370

New issue

Closed

opened 2026-10-17 21:03:49 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-17 21:03:49 +00:00

Owner

Originally created by @jayinvers on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4558

Area

Other (source tests / CI)

Observed behavior

Two tests in tests/boundary/firmware/firmwareIntegration.test.ts fail together in every CI run that actually executed the file on current main (4838f444):

"retains a setup-failed publication tree for provider cleanup retry" (assertion at line 196): expected error.diagnostics.previous_error to contain "injected post-acquisition setup failure"; the received error is a ProviderAdapterError composite whose diagnostics are {primary_error, cleanup_error} with cleanup_error reason "injected first cleanup failure" and the generic adapter user message ("Analysis could not complete. Run rea doctor …").
"retains the complete firmware result when published output cleanup stays uncertain" (assertion at line 269): expected result.error to be a ProviderCleanupError instance; received Error: Provider unblob adapter failed during extract_firmware (a plain ProviderAdapterError).

This blocks unrelated PRs' CI (observed on PR #4551, a native Swift-symbol change).

Reproduction

Commands actually run locally (WSL Ubuntu 24.04.5, Node 24.18.0, x64; repository npm ci):

At 4838f444 (main HEAD): npx vitest run tests/boundary/firmware/firmwareIntegration.test.ts --project boundary → 2 failed | 33 passed (35) — exactly the two tests above, 2/2 runs.
At 2b7b698d (parent of 2d163c12): → 35 passed (35), 1 run.
At 2d163c12: → 2 failed | 33 passed (35), same two tests, 1 run.

CI observations (ubuntu runners):

main 4838f444, run 38942701489: Test shard (5/5) and Portable tests failed with these two tests (log shows firmwareIntegration.test.ts (35 tests | 2 failed), failures at :196:20 and :269:26).
PR #4551 runs 38945824578 and 38948581290: Test shard (5/5) failed the same way.
Run 38943231467 on the same SHA is green, but it only re-ran the "Copilot" reviewer job (event dynamic) and executed no tests — it is not evidence of a passing suite. This is why the failure can be mistaken for an intermittent flake; every run that executed the file failed.

Expected behavior

Per docs/tool-design.md "Own the lifecycle" — "Bind cleanup failures to their exact resource owner. Resolve current cleanup uncertainty only after that owner confirms release, while preserving the original failure and completed observations." — a publication failure whose rollback also fails must surface a composed error preserving the primary failure and the cleanup failure. Since 2d163c12, publishFirmwareExtraction composes this case with analysisErrorWithCleanupFailure (diagnostics {primary_error, cleanup_error}), consistent with tests/process-global/firmwareFileLifetime.test.ts added in the same commit. The two integration tests still assert the pre-2d163c12 shape (a bare ProviderCleanupError with flat diagnostics.previous_error) and were not updated by that commit.

Impact and scope

Test-side only; no production behavior change is claimed. The composed error still classifies as cleanup_incomplete via projectAnalysisError, still carries previous_error inside the cleanup_error projection, retains the partial observation, and names the exact retained resources. Scope: Linux-only test file (it.skipIf(process.platform !== "linux")). Frequency: 100% of runs that executed the file (observed in 3 CI runs and locally at 2/2; parent commit 0/1).

REA version

Source revision 4838f444 (main at report time). First known bad: 2d163c12. Last known good: 2b7b698d.

Runtime and platform

CI: ubuntu-latest shards (Test shard (5/5), Portable tests). Local reproduction: WSL Ubuntu 24.04.5, Node 24.18.0, x64.

Target and artifact context

Not applicable; synthetic firmware fixture from tests/fixtures/firmware/.

Interface

Other (repository CI / source tests)

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Local run at 4838f444 (and at 2d163c12), obtained with the command above:

FAIL boundary tests/boundary/firmware/firmwareIntegration.test.ts > retains a setup-failed publication tree for provider cleanup retry
FAIL boundary tests/boundary/firmware/firmwareIntegration.test.ts > retains the complete firmware result when published output cleanup stays uncertain
AssertionError: expected Error: Provider unblob adapter failed dur… { …(14) } to be an instance of ProviderCleanupError
      Tests  2 failed | 33 passed (35)

Source reference: src/firmware/FirmwarePublication.ts, publishFirmwareExtraction catch path — since 2d163c12 it throws analysisErrorWithCleanupFailure(primary, cleanup, "extract_firmware") instead of a bare ProviderCleanupError when tree.rollback() fails.

Missing evidence / remaining uncertainty: I did not audit CI history further back than the runs linked above to find the last green main run of this file (proposed check, not run).

Typed outcome or recovery expected

ok:false with cleanupIncomplete:true; diagnostics.primary_error carrying the setup/close failure reason; diagnostics.cleanup_error carrying reason plus previous_error; retained partial observation; projectAnalysisError code cleanup_incomplete with partial_observation; retained output tree removed only after the owning retry succeeds.

Analysis or suspected cause

Source-derived: 2d163c12 ("fix(firmware): own file access through publication and cleanup retry", Refs #4475) changed the rollback-failure composition in publishFirmwareExtraction to preserve primary and cleanup failures through analysisErrorWithCleanupFailure, but did not migrate these two integration assertions; this was verified by the parent/child checkout runs above, so the cause is established rather than suspected. A draft PR updating the two tests to the composed contract (keeping ownership, retention, and projection assertions) follows.

Additional context

The two tests fail together because both inject a cleanup failure into the same publishFirmwareExtraction finalization path. Proposed check (not run): full boundary project run on Linux after the fix.

Originally created by @jayinvers on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4558 ### Area Other (source tests / CI) ### Observed behavior Two tests in `tests/boundary/firmware/firmwareIntegration.test.ts` fail together in every CI run that actually executed the file on current `main` (4838f444): - **"retains a setup-failed publication tree for provider cleanup retry"** (assertion at line 196): expected `error.diagnostics.previous_error` to contain `"injected post-acquisition setup failure"`; the received error is a `ProviderAdapterError` composite whose `diagnostics` are `{primary_error, cleanup_error}` with `cleanup_error` reason `"injected first cleanup failure"` and the generic adapter user message ("Analysis could not complete. Run `rea doctor` …"). - **"retains the complete firmware result when published output cleanup stays uncertain"** (assertion at line 269): expected `result.error` to be a `ProviderCleanupError` instance; received `Error: Provider unblob adapter failed during extract_firmware` (a plain `ProviderAdapterError`). This blocks unrelated PRs' CI (observed on PR #4551, a native Swift-symbol change). ### Reproduction Commands actually run locally (WSL Ubuntu 24.04.5, Node 24.18.0, x64; repository `npm ci`): 1. At `4838f444` (main HEAD): `npx vitest run tests/boundary/firmware/firmwareIntegration.test.ts --project boundary` → **2 failed | 33 passed (35)** — exactly the two tests above, **2/2 runs**. 2. At `2b7b698d` (parent of `2d163c12`): → **35 passed (35)**, 1 run. 3. At `2d163c12`: → **2 failed | 33 passed (35)**, same two tests, 1 run. CI observations (ubuntu runners): - main `4838f444`, run [38942701489](https://github.com/morluto/rea/actions/runs/38942701489): `Test shard (5/5)` and `Portable tests` failed with these two tests (log shows `firmwareIntegration.test.ts (35 tests | 2 failed)`, failures at `:196:20` and `:269:26`). - PR #4551 runs [38945824578](https://github.com/morluto/rea/actions/runs/38945824578) and [38948581290](https://github.com/morluto/rea/actions/runs/38948581290): `Test shard (5/5)` failed the same way. - Run [38943231467](https://github.com/morluto/rea/actions/runs/38943231467) on the same SHA is green, but it only re-ran the "Copilot" reviewer job (event `dynamic`) and executed no tests — it is not evidence of a passing suite. This is why the failure can be mistaken for an intermittent flake; every run that executed the file failed. ### Expected behavior Per [docs/tool-design.md](https://github.com/morluto/rea/blob/main/docs/tool-design.md) "Own the lifecycle" — "Bind cleanup failures to their exact resource owner. Resolve current cleanup uncertainty only after that owner confirms release, while preserving the original failure and completed observations." — a publication failure whose rollback also fails must surface a composed error preserving the primary failure and the cleanup failure. Since `2d163c12`, `publishFirmwareExtraction` composes this case with `analysisErrorWithCleanupFailure` (diagnostics `{primary_error, cleanup_error}`), consistent with `tests/process-global/firmwareFileLifetime.test.ts` added in the same commit. The two integration tests still assert the pre-`2d163c12` shape (a bare `ProviderCleanupError` with flat `diagnostics.previous_error`) and were not updated by that commit. ### Impact and scope Test-side only; no production behavior change is claimed. The composed error still classifies as `cleanup_incomplete` via `projectAnalysisError`, still carries `previous_error` inside the `cleanup_error` projection, retains the partial observation, and names the exact retained resources. Scope: Linux-only test file (`it.skipIf(process.platform !== "linux")`). Frequency: 100% of runs that executed the file (observed in 3 CI runs and locally at 2/2; parent commit 0/1). ### REA version Source revision `4838f444` (main at report time). First known bad: `2d163c12`. Last known good: `2b7b698d`. ### Runtime and platform CI: ubuntu-latest shards (`Test shard (5/5)`, `Portable tests`). Local reproduction: WSL Ubuntu 24.04.5, Node 24.18.0, x64. ### Target and artifact context Not applicable; synthetic firmware fixture from `tests/fixtures/firmware/`. ### Interface Other (repository CI / source tests) ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Local run at `4838f444` (and at `2d163c12`), obtained with the command above: ``` FAIL boundary tests/boundary/firmware/firmwareIntegration.test.ts > retains a setup-failed publication tree for provider cleanup retry FAIL boundary tests/boundary/firmware/firmwareIntegration.test.ts > retains the complete firmware result when published output cleanup stays uncertain AssertionError: expected Error: Provider unblob adapter failed dur… { …(14) } to be an instance of ProviderCleanupError Tests 2 failed | 33 passed (35) ``` Source reference: `src/firmware/FirmwarePublication.ts`, `publishFirmwareExtraction` catch path — since `2d163c12` it throws `analysisErrorWithCleanupFailure(primary, cleanup, "extract_firmware")` instead of a bare `ProviderCleanupError` when `tree.rollback()` fails. Missing evidence / remaining uncertainty: I did not audit CI history further back than the runs linked above to find the last green main run of this file (proposed check, not run). ### Typed outcome or recovery expected `ok:false` with `cleanupIncomplete:true`; `diagnostics.primary_error` carrying the setup/close failure reason; `diagnostics.cleanup_error` carrying `reason` plus `previous_error`; retained partial observation; `projectAnalysisError` code `cleanup_incomplete` with `partial_observation`; retained output tree removed only after the owning retry succeeds. ### Analysis or suspected cause Source-derived: `2d163c12` ("fix(firmware): own file access through publication and cleanup retry", Refs #4475) changed the rollback-failure composition in `publishFirmwareExtraction` to preserve primary and cleanup failures through `analysisErrorWithCleanupFailure`, but did not migrate these two integration assertions; this was verified by the parent/child checkout runs above, so the cause is established rather than suspected. A draft PR updating the two tests to the composed contract (keeping ownership, retention, and projection assertions) follows. ### Additional context The two tests fail together because both inject a cleanup failure into the same `publishFirmwareExtraction` finalization path. Proposed check (not run): full `boundary` project run on Linux after the fix.

bsakaguchi commented

2026-10-17 21:03:50 +00:00

Author

Owner

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026):

This appears fixed on main by 5eaef92e5 (test(firmware): assert cleanup provenance through composed failures, 2026-10-17). It updates both stale assertions in tests/boundary/firmware/firmwareIntegration.test.ts to check the composed {primary_error, cleanup_error} projection and the retained primary diagnostics, instead of the superseded ProviderCleanupError subclass and flat previous_error.

Evidence: main CI run 39024885316 on 73965f943 (Linux runners) passed all five Test shard jobs and Portable tests. On macOS the file is skipped (35 skipped), so I could not execute it locally. Leaving closure to a maintainer.

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026): This appears fixed on main by 5eaef92e5 (`test(firmware): assert cleanup provenance through composed failures`, 2026-10-17). It updates both stale assertions in `tests/boundary/firmware/firmwareIntegration.test.ts` to check the composed `{primary_error, cleanup_error}` projection and the retained primary diagnostics, instead of the superseded `ProviderCleanupError` subclass and flat `previous_error`. Evidence: main CI run [39024885316](https://github.com/morluto/rea/actions/runs/39024885316) on 73965f943 (Linux runners) passed all five `Test shard` jobs and `Portable tests`. On macOS the file is skipped (35 skipped), so I could not execute it locally. Leaving closure to a maintainer.

bsakaguchi referenced this issue

2026-10-31 00:29:41 +00:00

[GH-ISSUE #2383] [Tracker] Audit analysis and debugging coverage for the TIOBE top 100 languages (October 2026) #794

bsakaguchi closed this issue

2026-10-31 00:33:10 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#2370

No description provided.

Rows
Columns

[GH-ISSUE #4558] [Bug] firmwareIntegration publication cleanup-retry tests fail deterministically since 2d163c12 #2370