Skip to content

ci: hand the packed tarball to CI as an artifact - #1763

Merged
sergey-shandar merged 2 commits into
mainfrom
claude/private-ts-todo-partial-bydyc9
Aug 28, 2026
Merged

ci: hand the packed tarball to CI as an artifact#1763
sergey-shandar merged 2 commits into
mainfrom
claude/private-ts-todo-partial-bydyc9

Conversation

@sergey-shandar

Copy link
Copy Markdown
Contributor

Step 2 of Stage 2's packed-artifact check, designed in #1757. Step 1 (the job-ordering edge) merged as #1762.

Why

The package is built in CI and then thrown away: node26 runs npm pack and nothing keeps the result. Checking the package as a consumer sees it requires a job with no repository checkout, and such a job can only receive the tarball through an artifact.

This adds the producing half only. The consuming job is the next step and deliberately is not here.

The change

actions/upload-artifact pinned at v7.0.1 (latest release), and an upload step in node26 immediately after npm pack. The generated workflow gains exactly those eight lines.

Two decisions worth stating:

  • The artifact name is an exported constant (packageArtifact), not a string literal. The consuming job downloads by name, so producer and consumer share one definition rather than two literals that can drift apart silently.
  • if-no-files-found: error. The action's default is to warn and upload nothing. That would turn a packing failure into a missing-artifact failure in the consuming job — the wrong place, reported as the wrong cause. This is the same "red for the wrong reason" failure the ordering edge in ci: the job schema can express ordering #1762 exists to prevent.

Proof

packageArtifact covers the properties rather than the shape, and I checked each against a mutant that breaks it:

Property Mutant Caught
upload follows npm pack move it before expected the upload to follow npm pack
name matches the exported constant rename to a literal
if-no-files-found: error drop the key
exactly one job uploads asserted

Ordering is not cosmetic: uploading before packing ships an empty artifact, and if-no-files-found: error would then fire in the wrong job.

Checks

npx tsc clean; full suite with coverage at 100%; npm run ci-update reproduces .github/workflows/ci.yml with no further diff.

No changelog entry: no behavior or public API change for package users — this is the repository's own CI generator.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n


Generated by Claude Code

The package is built in CI and then thrown away: node26 runs `npm pack` and
nothing keeps the result. Checking the package as a consumer sees it needs a
job with no repository checkout, and such a job can only receive the tarball
through an artifact.

Adds the producing half: `actions/upload-artifact` pinned at v7.0.1, and an
upload step in node26 immediately after `npm pack`. The artifact name is an
exported constant rather than a literal, so the consuming job that follows
cannot drift from the producer.

`if-no-files-found: error` is deliberate. The action's default is to warn and
upload nothing, which would turn a packing failure into a missing-artifact
failure in the consuming job — the wrong place, with the wrong cause.

The proof covers the properties, and each was checked against a mutant that
breaks it: the upload follows `npm pack` rather than preceding it (uploading
first ships an empty artifact), the name matches the exported constant, the
no-files behavior is `error`, and exactly one job uploads — a second producer
under one name is a race, not redundancy.

Owned by fjs/ci/todo/ci-integration-tests.md, whose plan already calls for the
artifact publish step; the consuming job is the next step and is not here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
functionalscript b926e29 Commit Preview URL

Branch Preview URL
Aug 28 2026, 08:59 PM

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

Round-trip is clean — regenerating leaves git diff empty, which matters more here than last time since ci.yml is in the diff. The upload step is well-formed and correctly placed: actions/upload-artifact@v7.0.1 exists as a tag, and it sits after npm pack in node26Steps with no working-directory override, so the glob looks where npm pack actually writes. needs is genuinely not used yet, matching your commit message. The -2 lines are the two widened imports, not a deleted assertion. Gates tsc 0, npm test 3529/3529, zero leaves vanished and one added.

Four of the five properties kill under mutation — dropping the step, wrong name, if-no-files-found: warn, and ordering asserted directly.

One gap: path is unpinned. Changing path: '*.tgz' to '*.wrong' leaves npm test at 3529/3529, exit 0. Your own mutant list never claimed path, so this is a gap rather than a wrong claim, and if-no-files-found: error limits the blast radius — a glob matching nothing fails loudly in the producing job. What survives is a path matching the wrong files quietly. assertEq(upload?.path, '*.tgz') closes it.

The path was the one property of the upload step nothing asserted. Reproduced
the gap before fixing it: changing '*.tgz' to '*.wrong' left the suite fully
green.

if-no-files-found: error bounds the damage but does not close it — it catches a
glob matching nothing, and the surviving case is a glob matching the wrong
files, which uploads them quietly under the right artifact name. That is worse
than an empty artifact, because the consuming job then type-checks something
real and unrelated.

assertEq on the path closes it; the same mutant now fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n

Copy link
Copy Markdown
Contributor Author

The path gap is real — reproduced it before fixing, and closed in b926e29.

Confirmed exactly as you described: changing path: '*.tgz' to '*.wrong' left packageArtifact passing and the suite green. My mutant list covered the step's presence, name, ordering and no-files behavior, and simply never touched path — so the property with the least coverage was the one nothing asserted.

Your bounding argument is right and worth making sharper, because it points at why this one matters more than it first looks. if-no-files-found: error catches a glob matching nothing. What survives is a glob matching the wrong files — uploaded quietly, under the correct artifact name, at the correct size. That is worse than an empty artifact: an empty one fails loudly in the producing job, whereas a wrong one flows to the consuming job in step 3, which then type-checks something real and unrelated and reports green. A check that validates the wrong artifact is indistinguishable from a working one until it matters.

assertEq(upload?.path, '*.tgz') added, with a comment recording that distinction so the next person doesn't drop it as redundant with if-no-files-found. The same mutant now fails.

Re-verified: tsc clean, coverage 100%, npm run ci-update produces no further diff — the fix is proof-only, so ci.yml is untouched.


Generated by Claude Code

@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 28, 2026
Merged via the queue into main with commit a491007 Aug 28, 2026
19 checks passed
@sergey-shandar
sergey-shandar deleted the claude/private-ts-todo-partial-bydyc9 branch August 28, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants