fix(ci): stop ten artifact uploads discarding their hidden paths - #2091
Conversation
`actions/upload-artifact` has excluded hidden files and directories by default since v4.4 (this repository pins v4.6.2), and most diagnostics here are written under `.tmp`. Ten upload steps across seven files therefore uploaded nothing from those paths: macos.yml's xcresult bundle, both mutation lanes' reports and shards, replays-nightly's fuzz output, xctest-nightly's results, test-app-build-cache's fixture tarball, and 1874-diagnose's per-iteration logs. Most fail silently, since they pair the omission with `if-no-files-found: warn` or `ignore`. test-app-build-cache sets `error`, so that one does not. A structural guard rather than a shared upload wrapper: the wrapper would be a shallow mirror of the action's options over artifacts with different owners, while the policy question — a hidden path needs the flag — is one rule that belongs in one place. Each workflow still declares its own artifact. test/ci/upload-artifact-hidden-paths.test.ts holds it across every workflow and composite action, and is red if any single flag is dropped.
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. Top changed packed filesNo changed packed files. |
…ents The hidden-path upload fixes and their guard move to #2091, per review: one rule, ten uploads, artifacts with different owners. This branch keeps the diagnose loop's own behaviour. Comments cut to the rule added in #2087. What the narration explained now lives in names and types — `MeasuredVerdict` separates xcodebuild's three words from the two the loop derives, `exitContradictsMeasured` names the rule that a nonzero exit outranks a green measured test, and each shape's `name` says what the case is. Two constants remain, citing measured CI numbers that cannot be encoded. The entry-point test spawns once rather than once per shape. Nine `--experimental-strip-types` subprocesses made it slow and contention-sensitive in a lane that runs it alongside other subprocess tests, and they were re-covering what the direct table already asserts; one representative invocation proves the wiring. 3.7s to 0.07s.
|
[P2] Make the hidden-upload guard cover every supported config file. |
…my revert orphaned Two review findings on the diagnose loop. The upload step carried a comment describing an `include-hidden-files` input it does not set: I split that fix out to #2091 and my revert removed the line but left the prose. Comment gone. The flag itself arrives with #2091, which carries this workflow's upload along with the nine others. The `rc` fix was only asserted through `readIteration`, so dropping or miswiring the CLI argument would have restored the false pass and the lost evidence with the suite still green. The entry-point test now runs both an `rc=0` slow pass and an `rc=65` green measured test, asserting the printed verdict, the summary line's `rc=`, and the evidence file. Verified red against both mutations: hardcoding `0` for the argument, and dropping the rule that a nonzero exit outranks the measured verdict. Remaining comments are the module's one-line pointer to its caller and the measured CI numbers behind SLOW_PASS_MS.
The guard read `.github/workflows/*.yml` and assumed local actions live one directory deep as `action.yml`. GitHub also reads `.yaml` for both, and local actions nest, so a hidden-path upload in any of those shapes passed the gate. It now walks the `.github` tree recursively for either extension, and a second test plants the three shapes the old scan missed and asserts all three are found — executable rather than a one-off manual check. Red against narrowing the extension, against dropping the recursive walk, and against removing any single real flag. Also drops the action-version note under the comment rule in #2087; the version behaviour belongs in the PR, and the assertion message already says what the omission costs.
|
Fixed in The scan was too narrow — you are right on all three shapes. It read Rather than plant the violation by hand once, the shape coverage is now a second test: it fabricates the three shapes the old scan missed — Planted red three ways: Action-version comment removed under the #2087 rule. The v4.4 default change is PR history, and the assertion message already carries what the omission costs. 🤖 Addressed by Claude Code |
|
Re-reviewed exact head |
thymikee
added
the
ready-for-human
1 participant
actions/upload-artifacthas excluded hidden files and directories by default since v4.4, and this repository pins v4.6.2 and writes most of its diagnostics under.tmp. Ten upload steps across seven files were therefore uploading nothing from those paths.if-no-files-foundmacos.ymlwarnmutation-affected.yml×3warn/ unsetmutation-weekly.yml×2warn/ unsetreplays-nightly.ymlignorexctest-nightly.ymlwarntest-app-build-cache.ymlerror1874-diagnose.ymlignoreMost pair the omission with
warnorignore, so they fail silently.test-app-build-cachesetserror, so that one does not.How it surfaced
The #1874 diagnose loop was dispatched to collect per-iteration logs. The summary arrived with its cadence excerpt and none of the logs it points at —
.tmpis hidden, so every per-iteration log that workflow has ever kept had been discarded. Checking whether anything else did the same found nine more.Why a guard rather than a wrapper
A shared upload wrapper would be a shallow mirror of the action's options across artifacts with genuinely different owners — mutation reports, fixture caches, XCTest bundles, session diagnostics. The policy question is one rule ("a hidden path needs the flag") and belongs in one place; the artifact itself belongs to the workflow that produces it.
test/ci/upload-artifact-hidden-paths.test.tscentralises the policy and leaves each workflow declaring its own artifact.Validation
Parse-only, no device lane. The guard walks every workflow and composite action; dropping any single flag turns it red (verified against
xctest-nightly.yml). Format, lint, typecheck andtest/cigreen.Not claimed: that this explains any specific past failure. The last eight
Test App Build Cacheruns onmainare green, and the most recent published zero artifacts — consistent with no rebuild being selected, which proves nothing either way about the upload path.Split out of #2059 on review; that PR keeps the diagnose-loop behaviour and will drop its duplicate of these changes once this lands.