Skip to content

ci: run the wire-compat gate (published pg-core opens HEAD-sealed samples) #266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rubenhensen merged 3 commits into from
Jul 28, 2026
Merged

ci: run the wire-compat gate (published pg-core opens HEAD-sealed samples) #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
394ef02
ci: run the wire-compat gate (published pg-core opens HEAD-sealed sam…
rubenhensen Jul 28, 2026
62f2585
ci: harden the wire-compat gate per review — root lockfile in filter,…
rubenhensen Jul 28, 2026
a9138a0
docs(pg-compat): reference the artifact by head sha, matching the job
rubenhensen Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ on:
branches:
- main
pull_request:
# `edited` included for base retargets (e.g. a stacked PR's base merging):
# paths-filter's verdict depends on the base, and without `edited` a stale
# verdict stays attached to the unchanged head sha.
types: [opened, synchronize, reopened, edited]

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Keep edited — it is doing real work. A base retarget fires edited and none of the three default types, so without it this job's paths-filter verdict would stay pinned to the old base while GitHub reports the check as current. The comment is right.

The cost is the other half of edited: it also fires on every title and description edit, and build.yml has no concurrency group, so each edit re-runs the whole workflow (4x Test, 4x Clippy, 4x Format, plus 3 wasm browser jobs including a macOS runner) with nothing cancelling the superseded run. Worth pairing with:

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

Pre-existing gap rather than something this PR introduced; edited just makes it cheap to hit. Non-blocking.

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Keeping edited is right and I confirmed the reasoning in the comment: a base retarget fires edited and none of the three default types, so without it the paths-filter verdict stays pinned to the old base while GitHub reports the check as current. Not asking you to drop it.

The cost is that types: is a workflow-level key, so this re-triggers all of build.yml, not the two jobs the comment reasons about. I counted this PR's own run (30341907759): 17 check runs from 9 job definitions, including the three Run wasm tests in browsers jobs, each of which builds wasm-pack from source, one of them on macos-latest at 10x billing (5m3s here). And GitHub fires pull_request.edited for title and body edits, not just base retargets. build.yml has no concurrency: block, so nothing cancels the superseded run either — after this change, fixing a typo in a PR description buys a second full CI run including a macOS build.

Either of these settles it, both non-blocking:

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

or, on the expensive jobs only, if: github.event.action != 'edited' || github.event.changes.base. The edited payload carries changes.base only when the base actually moved, so that separates retargets from prose edits precisely; when action is not edited the whole changes object is absent and the expression short-circuits on the left. The concurrency block is the smaller change and helps every trigger, not just this one.

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

types: is a workflow-level key, so edited re-triggers all of build.yml, not just the two jobs the comment is about — and build.yml has no concurrency key anywhere (delivery.yml:73 does). Every PR title/description edit therefore starts a full duplicate run of ~20 jobs without cancelling the in-flight one, including the three Run wasm tests in browsers jobs; the macOS Safari one took 5m3s here, and macOS runners bill at 10x.

Keeping edited is right — a base retarget fires it and none of the three default types, so the paths-filter verdict would otherwise stay pinned to the old base. The fix belongs on the other side:

concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
  cancel-in-progress: ${{ github.event_name == 'pull_request' }}

at the top of build.yml caps it at one live run per PR while leaving main pushes uncancelled. Non-blocking.

branches:
- main
workflow_dispatch:
Expand Down Expand Up @@ -90,3 +94,101 @@ jobs:
retry_on: error
timeout_minutes: 20
command: wasm-pack test --release --headless --\${{ matrix.browser }} ./pg-wasm

# ---------------------------------------------------------------------------
# Wire compatibility: HEAD-sealed containers must open with published readers
# (#251 / #260). #261 adds a wire-compat-js job that downloads the artifact
# this job uploads.
#
# The path filter is a step, not an `on: paths:` key, on purpose: a job that
# is skipped by a path filter never reports, so a required check would sit
# pending forever on PRs that do not touch the wire surface. This shape always
# reports and only does the expensive work when it needs to.
# ---------------------------------------------------------------------------
wire-compat-rust:

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

No permissions: block on the job. dorny/paths-filter defaults token to ${{ github.token }} and, on pull_request, fetches the changed-file list from the REST API — its README lists pull-requests: read as required. It works here because the repo default is permissive, but this is the first GITHUB_TOKEN-consuming step in build.yml (I grepped: zero permissions: keys in this file, versus 7 in delivery.yml). For a check slated to become required, worth pinning:

    permissions:
      contents: read
      pull-requests: read

Good news on the failure mode: the action throws rather than falling back, so a missing permission reds the job instead of silently returning zero changed files. Non-blocking.

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Non-blocking, but cheapest to do while you are here: this job never surfaces the filter verdict, so a downstream job cannot read it. That is what forces pg-compat/README.md:150 to advise #261 to "Repeat the same path condition on the download" — which would be the third copy of the path list, since pg-compat-lint at line 176 already carries a second one, deliberately different in content but identically named wire.

Copies of a gate's trigger list drift in the dangerous direction: add pg-ffi/** (or any future wire-adjacent tree) to one and forget the others, and the gate silently stops running with no job going red — for a pair of gates whose entire value is not going silent, that is the failure mode to design against.

  wire-compat-rust:
    name: Wire compat (published pg-core)
    runs-on: ubuntu-latest
    outputs:
      wire: ${{ steps.changes.outputs.wire }}

That turns #261's half into a one-line if: needs.wire-compat-rust.outputs.wire == 'true' and keeps one source of truth. Since #261 will be written from this file, adding the output now is what makes the README advice unnecessary rather than duplicated.

name: Wire compat (published pg-core)
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read # paths-filter reads the PR's changed files
steps:
- uses: actions/checkout@v4
# SHA-pinned as the one action new to this repo; the rest follow the
# file's existing floating-ref convention.
- uses: dorny/paths-filter@7b450fff21473bca461d4b92ce414b9d0420d706 # v4.0.2

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Nit, and it partly contradicts a note in my own memory that I have now corrected. I read src/main.ts at the pinned commit 7b450ff: on a non-pull_request event the action calls getChangedFilesFromGit(base, ref, ...), where head falls back to github.ref and base falls back to repository.default_branch. On a workflow_dispatch from main those are the same short name, so isBaseSameAsHead is true; beforeSha is set only when eventName === 'push', so it is null here, and the action logs 'before' field is missing in event payload - changes will be detected from last commit and returns getChangesInLastCommit().

So a manual dispatch on main decides whether to run the gate from main's single most recent commit, and reports Wire compat (published pg-core) green whenever that one commit happens to miss the wire surface. That sits a little oddly with the header comment's "always reports and only does the expensive work when it needs to" — dispatch is the one entry point where a human is explicitly asking for the work. (Dispatch from a non-default branch is fine: base resolves to main and you get a real diff.)

If dispatch should force the gate, steps.changes.outputs.wire == 'true' || github.event_name == 'workflow_dispatch' on the three conditional steps covers it.

id: changes
with:
filters: |
wire:
- 'pg-core/**'
- 'pg-wasm/**'
- 'pg-compat/**'
# pg-core has no lockfile of its own: the bytes HEAD seals are a
# function of the ROOT lockfile (a bincode-next/ibe/serde bump
# touches only these two files). Root Cargo.toml also holds the
# exclude list that keeps pg-compat on crates.io pg-core.
- 'Cargo.lock'
- 'Cargo.toml'
- '.github/workflows/build.yml'

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Blocking. The wire filter omits the root Cargo.lock (and root Cargo.toml).

pg-core has no lockfile of its own — I checked the tree at HEAD: only the repo root has a Cargo.lock, plus the one inside the out-of-workspace pg-compat. pg-core is a workspace member, so it resolves versions from the root lockfile, and the seal step below runs cargo run --locked. The bytes HEAD produces are therefore a function of the root Cargo.lock.

So: a cargo update or dependabot PR that bumps ibe, ibs, serde or bincode-next touches only Cargo.lock, steps.changes.outputs.wire evaluates to false, every step below skips on its if, and the job reports green having sealed and opened nothing. That is precisely the class of change CLAUDE.md flags as wire-critical — the bincode_next::config::legacy() byte-compatibility property, and the explicit warning that a bincode-next rc bump is wire-relevant. The gate would be silent on the one PR shape it most needs to catch, and once #262 makes it required that silence is a passing required check.

Suggested change
- '.github/workflows/build.yml'
- 'pg-core/**'
- 'pg-wasm/**'
- 'pg-compat/**'
- 'Cargo.lock'
- 'Cargo.toml'
- '.github/workflows/build.yml'

Root Cargo.toml earns its place too: it holds the workspace dependency table and the exclude list that keeps pg-compat pointed at crates.io pg-core rather than ../pg-core. CLAUDE.md calls that exclusion load-bearing, and a change to it would quietly turn the gate into a self-comparison.


- if: steps.changes.outputs.wire == 'true'
uses: dtolnay/rust-toolchain@stable

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Nit on the pinning rationale, not the pin. The comment above pins paths-filter because "this step decides whether the gate's teeth engage" — but dtolnay/rust-toolchain@stable here is a mutable branch ref, which is weaker than the retagged-tag case the pin defends against, and it supplies the compiler that both seals the samples and builds the reader. It has at least as much say in the verdict. actions/checkout@v4 and actions/upload-artifact@v4 are likewise floating.

Consistent with the rest of build.yml, so fine to leave as-is — but the stated reasoning does not survive being applied to only one of the four actions. Either pin all four or soften the comment.

with:
components: clippy, rustfmt

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Leftover from the lint split: this job no longer runs fmt or clippy, so components: clippy, rustfmt installs two components nothing in it uses. A bare - uses: dtolnay/rust-toolchain@stable (keeping the if:) is enough for the seal and read steps. The components belong in pg-compat-lint, where they already are.

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Carried forward from the last round and still open — flagging once more, still non-blocking. After the lint split this job runs only cargo run (seal) and cargo test (read); neither uses clippy or rustfmt, so these two components are installed and never used. A bare - uses: dtolnay/rust-toolchain@stable with the same if: is enough. pg-compat-lint needs them and already declares them at line 188.

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

These two components are installed but never used: after the lint split this job runs only cargo run (seal) and cargo test (read). A bare - uses: dtolnay/rust-toolchain@stable with the same if: is enough. pg-compat-lint does need them and declares them itself at line 188.


- name: Seal the sample set with HEAD
id: seal
if: steps.changes.outputs.wire == 'true'
shell: bash
run: |
cargo run --locked -p pg-core --features stream --example seal-samples \
-- "$RUNNER_TEMP/wire-compat-artifacts"

# Hand-off to wire-compat-js (#261). Uploaded before the reader runs so a
# red gate still leaves the bytes that broke it.
- name: Upload the sample set
if: steps.seal.outcome == 'success'
uses: actions/upload-artifact@v4

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Blocking, one line. actions/upload-artifact@v4 defaults to overwrite: false, and its own action.yml says that means "the action will fail if an artifact for the given name already exists". Artifact names are scoped to the workflow run, not the run attempt — I checked this PR's own run: the REST list endpoint is /actions/runs/30341907759/artifacts with no attempt parameter, and the returned object carries only workflow_run: {id, head_branch, head_sha, ...}, no run_attempt.

The failure mode is exactly the path this gate exists to walk. Attempt 1: seal OK, upload OK, Open it with published pg-core reds — either a real wire break or a flake (that step resolves pg-core 0.6.1 from crates.io under --locked, so a network blip is a normal cause). Maintainer clicks Re-run failed jobs. Attempt 2: seal succeeds, and the job dies here on a 409 conflict before ever reaching the reader. The re-run reports an artifact-plumbing error instead of the wire verdict it was re-run to get, and "Re-run all jobs" hits the same wall.

This is a well-trodden v4 problem, not a theory — a search for the conflict text plus "re-run" returns 36 issues/PRs, including several whose whole content is this fix: nasa/fprime-actions#11 ("set overwrite: true on artifact upload so re-run works"), apache/datafusion-comet#3723 ("resolve CI artifact upload conflicts on workflow re-runs"), icook/tiny-congress#616 ("gitleaks SARIF artifact upload fails with 409 Conflict on re-runs").

        uses: actions/upload-artifact@v4
        with:
          name: wire-compat-artifacts-${{ github.event.pull_request.head.sha || github.sha }}
          path: ${{ runner.temp }}/wire-compat-artifacts
          retention-days: 7
          overwrite: true

overwrite has been an input since v4.2.0 and the floating v4 tag currently resolves to v4.6.2 (ea165f8), so this works as written. Worth doing in this PR rather than after: once #262 makes Wire compat (published pg-core) required, a re-run that cannot go green is a stuck PR — and the bot has no workflows permission, so the follow-up fix costs another patch-in-comment plus a maintainer apply cycle. You are already in the file.

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Version skew with the repo's own convention: upload-artifact@v4 here, while delivery.yml:139 uses upload-artifact@v7 and delivery.yml:157/:201 use download-artifact@v8. v4 works — this PR's run proves it — but #261's JS consumer will be written against v8 download, and the only artifact producer in build.yml sitting three majors behind the only other producer in the repo is the kind of drift that stays invisible until someone bumps one of them.

with:
name: wire-compat-artifacts-${{ github.event.pull_request.head.sha || github.sha }}

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

The round-3 item, downgraded to a note. upload-artifact's action.yml documents overwrite defaulting to false = "the action will fail if an artifact for the given name already exists", and this name is keyed on the head sha, not on github.run_attempt — so a re-run of this job on an unchanged head would upload the same name twice into one run id.

What I could not settle is whether artifacts from a previous attempt stay attached to the run: actions/upload-artifact#585 shows prior-attempt artifacts becoming inaccessible, but its repro used attempt-numbered names. If they do stay attached, "Re-run failed jobs" after a flaky reader step dies at the upload before the reader ever runs. overwrite: true is one line of insurance either way — your call, and it can land after merge.

path: ${{ runner.temp }}/wire-compat-artifacts
retention-days: 7

- name: Open it with published pg-core
if: steps.changes.outputs.wire == 'true'
shell: bash
env:
PG_COMPAT_ARTIFACTS: ${{ runner.temp }}/wire-compat-artifacts
run: cargo test --manifest-path pg-compat/Cargo.toml --locked

# pg-compat is outside the workspace, so the per-crate fmt/clippy matrices do
# not cover it. Kept separate from wire-compat-rust on purpose: a new stable
# rustc lint must not red a required check named after wire compatibility,
# nor pre-empt the seal/read steps that answer it.
pg-compat-lint:
name: Lint pg-compat
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
steps:
- uses: actions/checkout@v4
- uses: dorny/paths-filter@7b450fff21473bca461d4b92ce414b9d0420d706 # v4.0.2
id: changes
with:
filters: |
wire:
- 'pg-compat/**'
- 'Cargo.lock'
- 'Cargo.toml'
- '.github/workflows/build.yml'

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

The two filters: blocks now duplicate Cargo.lock, Cargo.toml and .github/workflows/build.yml (lines 179-184 here and 121-132 in wire-compat-rust). For a pair of gates whose whole value is not going silent, drift between the two lists would be invisible, so a one-line comment on each saying they are kept in step would earn its keep. Non-blocking.

- if: steps.changes.outputs.wire == 'true'
uses: dtolnay/rust-toolchain@stable
with:
components: clippy, rustfmt
- name: Format pg-compat
if: steps.changes.outputs.wire == 'true'
run: cargo fmt --manifest-path pg-compat/Cargo.toml --all -- --check
- name: Clippy pg-compat
if: steps.changes.outputs.wire == 'true'
run: cargo clippy --manifest-path pg-compat/Cargo.toml --all-targets --locked -- -D warnings
2 changes: 1 addition & 1 deletion CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s
## Workspace & CI

- Workspace members: `pg-core` (lib), `pg-ffi` (C ABI), `pg-pkg` (PKG service), `pg-cli`. `pg-wasm` is a sibling crate the root `Cargo.toml` lists under `exclude`, so it is not part of the workspace and is built separately with wasm-pack (see Release & configuration). Sub-crates share workspace files. Build the workspace from repo root with `cargo build`. A bare `cargo test --workspace` FAILS to compile: `pg-core`'s tests are gated behind its `test` feature (also `rust`/`stream`), so the item is configured out and imports like `crate::test::TestSetup` don't resolve. CI (`.github/workflows/build.yml`) runs tests per crate: `cargo test --manifest-path pg-core/Cargo.toml --features test,rust,stream` for core, `--all-features` for `pkg`/`cli`/`ffi`. None of these cover `pg-wasm`. `pg-core` uses CGWKV + MKEM for multi-recipient encryption (production feature set `["cgwkv", "mkem"]`).
- `pg-compat` is a second excluded sibling crate (root `Cargo.toml` `exclude`), and the exclusion is load-bearing: it depends on `pg-core` from **crates.io** (`=0.6.1`), not on `../pg-core`, so it can open bytes sealed by this tree with published readers. It has its own `Cargo.lock` (run it with `--locked`) and is covered by none of the per-crate test/fmt/clippy matrices, so run `cargo fmt --manifest-path pg-compat/Cargo.toml --all -- --check` and `cargo clippy --manifest-path pg-compat/Cargo.toml --all-targets -- -D warnings` by hand. Its input comes from `cargo run -p pg-core --features stream --example seal-samples -- <dir>`, a deterministic sealer whose output layout is documented in `pg-compat/README.md`. The `wire-compat-rust` job that wires the two together is NOT yet a CI gate: the YAML sits in a comment on PR #265 waiting for a maintainer to apply it, so nothing stops a non-additive wire change until then; run the two commands locally when touching `pg-core/src/client/**` or `consts.rs`.
- `pg-compat` is a second excluded sibling crate (root `Cargo.toml` `exclude`), and the exclusion is load-bearing: it depends on `pg-core` from **crates.io** (`=0.6.1`), not on `../pg-core`, so it can open bytes sealed by this tree with published readers. It has its own `Cargo.lock` (run it with `--locked`). Its input comes from `cargo run -p pg-core --features stream --example seal-samples -- <dir>`, a deterministic sealer whose output layout is documented in `pg-compat/README.md`. CI wires the two together: `wire-compat-rust` in `build.yml` seals with HEAD and opens with published pg-core on any PR touching the wire surface (pg-core/pg-wasm/pg-compat trees, the ROOT `Cargo.lock`/`Cargo.toml` — pg-core resolves from the root lockfile — and build.yml itself); `pg-compat-lint` covers the crate's fmt/clippy, which the per-crate matrices don't.
- Appending a field at the *end* of `Header` really is additive: the header is a length-prefixed region and `bincode` ignores trailing bytes, so published `pg-core` 0.6.1 still opens it. A field inserted anywhere else, a changed field type, or a reorder shifts every following byte and the containers stop opening, but *not* with a decode error: 0.6.1 reads a garbage length prefix, attempts a ~20 GiB allocation, and the process aborts (SIGABRT). Expect `reader died on signal 6 ... memory allocation of N bytes failed`, not a message naming the header. This is also why `pg-compat` opens each case in a child process (its `pg-compat-case` binary): an abort is not a panic, `catch_unwind` cannot contain it, and in one process the first broken case would take the run down before the others were tried. Don't reason about "additive" from the struct alone; run the compat gate.
- CI's `Format workspace` matrix runs `cargo fmt --manifest-path pg-<crate>/Cargo.toml --all -- --check` per crate over shared workspace files; always run `cargo fmt --all -- --check` from repo root before pushing, or one crate's drift fails the whole matrix.
- The Docker build (`Dockerfile`, `FROM rust:<version>-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image.
Expand Down
5 changes: 3 additions & 2 deletions pg-compat/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,12 @@ stream-multi-segment.bin/.plain

### Consuming the set from another job

One thing for [#261] to plan around. The proposed `wire-compat-rust` job seals
One thing for [#261] to plan around. The `wire-compat-rust` job seals

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

The fix is correct and I verified the end state rather than just the text: the artifact this PR's run actually produced is wire-compat-artifacts-a9138a074264b08478bfce3b78af017e257ed224, i.e. the head sha, matching both build.yml:153 and this sentence — so #261, written from here, will download something that exists.

Only the reflow is left ragged. This paragraph otherwise wraps at 74-76 columns, but dropping "proposed" left this line at 69 and the longer artifact name orphaned unconditionally therefore hard-fails onto a 36-column line 149. Re-wrap the paragraph.

and uploads only when the PR touches the wire surface; on every other PR it
reports success with no artifact attached. A `wire-compat-js` job with
`needs: wire-compat-rust` that downloads
Comment on lines +144 to 147

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Docs still name the old artifact, three lines below this one: wire-compat-artifacts-${{ github.sha }}.

The job now uploads wire-compat-artifacts-${{ github.event.pull_request.head.sha || github.sha }} (build.yml:153). On a pull_request event github.sha is the merge commit, not the head sha, and this very run proves it: the artifact landed as wire-compat-artifacts-62f2585093d5a679dfc144fc9046e24b9e9bc102 (head) while the PR's merge commit is 4bbbbdb. A wire-compat-js job written against this sentence therefore 404s on every PR, which is the silent-gate failure mode the head-sha change was made to avoid.

Mirror the head.sha || github.sha expression here. The advice that follows, about repeating the path condition on the download or sealing locally in the JS job, stays correct.

`wire-compat-artifacts-${{ github.sha }}` unconditionally therefore hard-fails
`wire-compat-artifacts-${{ github.event.pull_request.head.sha || github.sha }}`
unconditionally therefore hard-fails

@dobby-coder dobby-coder Bot Jul 28, 2026

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.

Re-wrap left ragged by the edit: this paragraph wraps at 69-79 columns on every other line (144-148, 150-151), but line 149 is 36 characters. Rewrapping 147-149 as `needs: wire-compat-rust` that downloads `wire-compat-artifacts-${{ github.event.pull_request.head.sha || github.sha }}` / unconditionally therefore hard-fails on unrelated PRs rather than restores it. Cosmetic — the content is correct, and I checked the end state rather than the prose: this PR's run uploaded wire-compat-artifacts-a9138a074264b08478bfce3b78af017e257ed224, the head sha this sentence names.

on unrelated PRs rather than skipping. Repeat the same path condition on the
download, or seal locally in the JS job instead of consuming the artifact.

Expand Down
Loading