[GH-ISSUE #3542] [Bug] Fast CI skips portable inventory validation when the required timing report is missing #1317

New issue

Closed

opened 2026-10-16 21:04:10 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-16 21:04:10 +00:00

Owner

Originally created by @N0zoM1z0 on GitHub (Oct 14, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/3542

Originally assigned to: @N0zoM1z0 on GitHub.

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, private paths and sensitive target data.

Area

Other — CI/test-report completeness

Observed behavior

The portable job runs its mandatory inventory validator only when hashFiles('test-timings.json') != ''. If the timing file is absent, fast CI skips the validator entirely. The final portable gate and scripts/ci/required.mjs only inspect successful group/job outcomes, so both accept this controlled state even when the remaining merged JSON is incomplete.

This is a reproduced fail-open condition, not an allegation that a current green hosted run omitted tests. Main eb44921c4 has a complete downloaded portable report: 1,284 unique files, 14,650 passed, 125 skipped. Its unrelated macOS failure is separate.

Reproduction

Tested locally on 1438422dfedf1de29368b595e09a04f1fc10750d, and first on eb44921c4. No tracked source was changed.

In a temporary Git fixture, track tests/first.test.ts and tests/second.test.ts, both passing tests. Use the checkout's locked Vitest dependency.
Produce one available real blob: vitest run tests/first.test.ts --reporter=blob. Merge it with the workflow's JSON, JUnit and actual scripts/ci/test-timing-reporter.mjs reporters. The real merge exits 0, producing a successful JSON report for one file.
Execute the actual scripts/ci/test-report.mjs with CI_VERIFY_TEST_INVENTORY=true, successful source/shard/merge outcomes, and the two-file Git inventory. It correctly exits 1 with Incomplete portable test inventory.
Remove only test-timings.json; retain the JSON and JUnit. Evaluate the checked-in summary condition against the actual missing-file state: it is false, so this validator is skipped.
Execute the checked-in Require successful test shards shell command with the controlled successful outcomes: exit 0. Execute the actual scripts/ci/required.mjs with a valid fast classification selecting code and successful required jobs: exit 0.
As a control, invoking the report script directly without the timing file exits 1 (ENOENT). The script is strict; its workflow admission condition bypasses that protection.

tracked inventory:                  2 files
real merged JSON:                   1 file, success=true
present timing, inventory validator: exit 1 (expected rejection)
missing timing, summary selected:    false
actual portable final gate:          exit 0
actual required gate:                exit 0
JSON and JUnit still present:        true

The experiment uses real Vitest execution/merging and the checked-in workflow expression and gate commands. Group outcomes are controlled inputs; no fault-injected hosted workflow was dispatched. GitHub's upload action itself was not simulated as an executed upload; the existing artifact paths still contain the JSON/JUnit.

Expected behavior

A missing mandatory timing/inventory input must fail a selected portable job. Retain partial diagnostic uploads, but require report presence and complete inventory before allowing a successful gate. The full-tier history upload has a separate missing-file protection; the demonstrated open path is fast CI.

Impact and scope

Suggested priority: P2. A report-generation/configuration/transport omission can disable the last completeness check precisely when its input disappears. Existing regression tests reject incomplete present reports but do not exercise the missing-file workflow condition. This is a workflow ownership defect, not a request to add blanket retries or execute tests twice.

REA version

Source 1438422dfedf1de29368b595e09a04f1fc10750d.

Runtime and platform

Linux x64; Node 24.18.0, npm 11.16.0, Vitest 5.0.3.

Interface / earliest failure stage

Other (CI); transport/result projection of portable test reports.

Relevant evidence

Conditional summary admission
Strict inventory validator
Final portable gate
Existing actual-report and workflow guard families: all 278 cases passed during this audit. The new negative control is outside that present coverage.

Proposed acceptance

Exercise the real workflow admission and both present/missing report cases; missing or malformed mandatory reports fail, incomplete inventory still fails, complete valid reports pass, and partial failure diagnostics remain available. No production runtime changes or weakened coverage are needed.

Originally created by @N0zoM1z0 on GitHub (Oct 14, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3542 Originally assigned to: @N0zoM1z0 on GitHub. ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, private paths and sensitive target data. ### Area Other — CI/test-report completeness ### Observed behavior The portable job runs its mandatory inventory validator only when `hashFiles('test-timings.json') != ''`. If the timing file is absent, fast CI skips the validator entirely. The final portable gate and `scripts/ci/required.mjs` only inspect successful group/job outcomes, so both accept this controlled state even when the remaining merged JSON is incomplete. This is a reproduced fail-open condition, **not an allegation that a current green hosted run omitted tests**. Main `eb44921c4` has a complete downloaded portable report: 1,284 unique files, 14,650 passed, 125 skipped. Its unrelated macOS failure is separate. ### Reproduction Tested locally on `1438422dfedf1de29368b595e09a04f1fc10750d`, and first on `eb44921c4`. No tracked source was changed. 1. In a temporary Git fixture, track `tests/first.test.ts` and `tests/second.test.ts`, both passing tests. Use the checkout's locked Vitest dependency. 2. Produce one available real blob: `vitest run tests/first.test.ts --reporter=blob`. Merge it with the workflow's JSON, JUnit and actual `scripts/ci/test-timing-reporter.mjs` reporters. The real merge exits 0, producing a successful JSON report for one file. 3. Execute the actual `scripts/ci/test-report.mjs` with `CI_VERIFY_TEST_INVENTORY=true`, successful source/shard/merge outcomes, and the two-file Git inventory. It correctly exits **1** with `Incomplete portable test inventory`. 4. Remove only `test-timings.json`; retain the JSON and JUnit. Evaluate the checked-in summary condition against the actual missing-file state: it is **false**, so this validator is skipped. 5. Execute the checked-in `Require successful test shards` shell command with the controlled successful outcomes: **exit 0**. Execute the actual `scripts/ci/required.mjs` with a valid fast classification selecting code and successful required jobs: **exit 0**. 6. As a control, invoking the report script directly without the timing file exits **1** (`ENOENT`). The script is strict; its workflow admission condition bypasses that protection. ```text tracked inventory: 2 files real merged JSON: 1 file, success=true present timing, inventory validator: exit 1 (expected rejection) missing timing, summary selected: false actual portable final gate: exit 0 actual required gate: exit 0 JSON and JUnit still present: true ``` The experiment uses real Vitest execution/merging and the checked-in workflow expression and gate commands. Group outcomes are controlled inputs; **no fault-injected hosted workflow was dispatched**. GitHub's upload action itself was not simulated as an executed upload; the existing artifact paths still contain the JSON/JUnit. ### Expected behavior A missing mandatory timing/inventory input must fail a selected portable job. Retain partial diagnostic uploads, but require report presence and complete inventory before allowing a successful gate. The full-tier history upload has a separate missing-file protection; the demonstrated open path is fast CI. ### Impact and scope Suggested priority: P2. A report-generation/configuration/transport omission can disable the last completeness check precisely when its input disappears. Existing regression tests reject incomplete **present** reports but do not exercise the missing-file workflow condition. This is a workflow ownership defect, not a request to add blanket retries or execute tests twice. ### REA version Source `1438422dfedf1de29368b595e09a04f1fc10750d`. ### Runtime and platform Linux x64; Node 24.18.0, npm 11.16.0, Vitest 5.0.3. ### Interface / earliest failure stage Other (CI); transport/result projection of portable test reports. ### Relevant evidence - [Conditional summary admission](https://github.com/morluto/rea/blob/1438422dfedf1de29368b595e09a04f1fc10750d/.github/workflows/ci.yml#L436) - [Strict inventory validator](https://github.com/morluto/rea/blob/1438422dfedf1de29368b595e09a04f1fc10750d/scripts/ci/test-report.mjs#L97) - [Final portable gate](https://github.com/morluto/rea/blob/1438422dfedf1de29368b595e09a04f1fc10750d/.github/workflows/ci.yml#L468) - Existing actual-report and workflow guard families: all 278 cases passed during this audit. The new negative control is outside that present coverage. ### Proposed acceptance Exercise the real workflow admission and both present/missing report cases; missing or malformed mandatory reports fail, incomplete inventory still fails, complete valid reports pass, and partial failure diagnostics remain available. No production runtime changes or weakened coverage are needed.

bsakaguchi closed this issue

2026-10-16 21:04:10 +00:00

bsakaguchi commented

2026-10-16 21:04:11 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 14, 2026):

Merged via normal protected-branch squash in #3549 as 0f623732bddfa27542a86cfc1c5fb34ee9e9a195.

All 35 PR-head code CI jobs passed; downloaded artifact11908857948 verifies 1,284 unique files /14,673 passed /125 skipped /zero failures /no duplicates. Exact-head source review5/5 with no inline findings. Base regression:1 failed /8 passed; fix:34 relevant tests pass, static checks/actionlint/discovery pass.

Main concurrently advanced by the non-overlapping Hopper launcher change044fb9127. Qualified the actual combined revision8cb6e248 separately:12 files /153 tests pass, all8static tasks and architecture guards pass, all1,285current files discovered once. Reused unchanged-head CI transparently; did not claim a new complete hosted run of this integration. Actual squash parent044fb9127 and tree7c9a7caa47be69f552feb8a7e06542fbc99a412e exactly match the qualified candidate. Anonymous author email and both requested Kaoru/gensokyo-dev trailers verified.

The owner explicitly authorized autonomous agent review/ready/merge without further human permission checkpoints. Human inspection facts were not fabricated; their readiness boxes stayed unchecked and the resulting non-required metadata failure is recorded as an owner-authorized process exception. All four GitHub-enforced checks passed; no admin bypass or status manipulation was used.

@N0zoM1z0 commented on GitHub (Oct 14, 2026): Merged via normal protected-branch squash in #3549 as `0f623732bddfa27542a86cfc1c5fb34ee9e9a195`. All 35 PR-head code CI jobs passed; downloaded artifact11908857948 verifies 1,284 unique files /14,673 passed /125 skipped /zero failures /no duplicates. Exact-head source review5/5 with no inline findings. Base regression:1 failed /8 passed; fix:34 relevant tests pass, static checks/actionlint/discovery pass. Main concurrently advanced by the non-overlapping Hopper launcher change044fb9127. Qualified the actual combined revision8cb6e248 separately:12 files /153 tests pass, all8static tasks and architecture guards pass, all1,285current files discovered once. Reused unchanged-head CI transparently; did not claim a new complete hosted run of this integration. Actual squash parent044fb9127 and tree7c9a7caa47be69f552feb8a7e06542fbc99a412e exactly match the qualified candidate. Anonymous author email and both requested Kaoru/gensokyo-dev trailers verified. The owner explicitly authorized autonomous agent review/ready/merge without further human permission checkpoints. Human inspection facts were not fabricated; their readiness boxes stayed unchecked and the resulting non-required metadata failure is recorded as an owner-authorized process exception. All four GitHub-enforced checks passed; no admin bypass or status manipulation was used.

bsakaguchi referenced this issue

2026-10-17 21:10:22 +00:00

[PR #1317] [MERGED] test: share the client registration command to restore the jscpd gate #2796

bsakaguchi referenced this issue

2026-10-17 21:10:23 +00:00

[PR #1322] [MERGED] perf(json): encode streamed JSON iteratively #2799