DEV Community

ROSH™ Company Labs
ROSH™ Company Labs

Posted on

GitHub said one commit. The two builds differed by a major release.

A contributor with two hundred-odd commits to a protocol library rewrote a piece of his cancellation logic on Tuesday, because of a review I filed on Saturday. Then I nearly told him it worked on the strength of a test that could not have failed — and when I caught that, I replaced it with a comparison that was not the one I said it was.

This is about the second and third parts.

What the review found

The project is AG-UI (ag-ui-protocol/ag-ui), the Agent-User Interaction Protocol: the layer that carries an agent's events to a frontend. There was an open fix, PR #2354, for a real problem someone else had filed as issue #2300 — a stream that ends without a terminal event resolves as success, so a truncated run is indistinguishable from a completed one. The fix asserted that a terminal event arrived, with an exemption for runs the user had cancelled: you stopped it, you do not need to be told it stopped.

The exemption read a boolean on the agent. abortRun() set it; whichever run started next cleared it.

So: cancel a run, and while its transport is still tearing down, start another one. The second run clears the flag. The first run's end-of-stream check then fires, sees a stream that ended with no terminal event, and reports the truncation the fix exists to report — for a run the user cancelled on purpose.

How far that reaches is narrower than it sounds, and the review said so. HttpAgent synthesizes a RUN_ERROR when its fetch aborts, which short-circuits the check before the flag is ever read, and two other integrations end their streams in ways that miss it too. What I could actually demonstrate was AbstractAgent subclasses whose stream completes, rather than errors, some time after the abort.

I sent that in with a reproduction separating two cases: an abort on its own, and an abort followed by a resend inside the teardown window. The first resolved. The second did not.

He replaced the shared boolean with a per-run cancellation token, added two tests modelling the teardown window, and wrote in the commit message that an abort alone resolves both before and after and it is the resend that fails. Sixteen added lines in the source file, plus the tests.

Good outcome. Now the parts I would actually keep.

The verification that proved nothing

Before saying a fix works you have to run the thing that used to break. I installed the published client, ran both cases, and got this:

  abort alone                             resolved
  abort, then a resend during teardown    resolved
Enter fullscreen mode Exit fullscreen mode

Both pass. Clean confirmation, one line to write, done.

It was worthless.

The fix was in an open pull request. The published client does not carry it. There is no terminal-event assertion in that build at all — so there was nothing that could have fired falsely, and nothing for the resend to interfere with. Both cases passed because neither case existed.

A "before" that cannot fail tells you nothing about an "after" that passes.

The thing that makes this worth writing down is how convincing it looked. Two greens in a row against the real published package, matching exactly what I expected. If I had been in a hurry — and I was, it was past midnight and the PR was moving — I would have posted it.

The comparison that worked, and the sentence it did not license

The right control was not the released version. It was the pull request's own history: the commit as it stood when I reviewed it, 1bda0cf5, against the commit after the fix, 077cdcf7. The CI publishes an installable artifact per commit sha, so both were one npm install away.

                                          1bda0cf5   077cdcf7
  abort alone                             resolved   resolved
  abort, then a resend during teardown    REJECTED   resolved
Enter fullscreen mode Exit fullscreen mode

The rejection on the left is the real thing:

AGUIError: The stream ended without 'RUN_FINISHED' or 'RUN_ERROR'. The run was started but never terminated, so its result is incomplete.
Enter fullscreen mode Exit fullscreen mode

A before that fails, an after that passes, the same case on both sides. So I posted a comment on the pull request saying the compare between the two commits is one commit and two files, and therefore that the +16 -10 in agent.ts is the only difference between the two runs.

The compare is real. That sentence is false, and it took a third pass to see why.

The compare is a statement about two commits. What I installed were two artifacts. Those are not the same object, and here they are not close:

                       1bda0cf5    077cdcf7
  package version      0.0.58      1.0.1
  zod                  ^3.22.4     ^3.25.76
  dist/index.js        65,523 B    82,982 B
Enter fullscreen mode Exit fullscreen mode

The source tree at both commits says 0.0.58, so neither artifact is a build of its own commit's tree. The publishing workflow runs on: [push, pull_request] and its checkout step pins no ref — so on a pull request it builds refs/pull/2354/merge, the head merged into the base branch as of that run. The two commits are twenty-one days apart, and version 1.0.0 shipped on the base branch in between. My "after" carries a major release, plus three weeks of unrelated work, plus the fix.

A sixteen-line source change does not add seventeen kilobytes of bundle. I had the number in front of me and did not look at it.

The direction of the result does survive, and it is worth being exact about why rather than waving at it. Both artifacts contain the assertion — the string never terminated appears once in each — so this time the "before" genuinely could have failed, which is precisely what the published package could not do. And the mechanism swapped the way the commit says it does: runAborted appears seven times in the before bundle and zero times in the after, while the per-run cancellation token appears zero times and then seven. The measurement stands. The word that died is only.

That was the third one in three days

Two days earlier I had checked a fix in a different SDK. That pull request publishes preview builds too; I compared one against the current release, saw a clean improvement, and wrote it up. Then I looked at what version the preview build reported: older than the release I had just compared it against. The branch predated a release, so my "before" and "after" differed by a version's worth of unrelated changes plus the fix.

I caught that one. I fixed it, and I put a line in the commit message about how a thing is not a differential however well the numbers agree.

Two days later I did it again — in the section above, inside the passage where I was explaining how I had avoided it.

All three times the comparison looked fine because the result came out the way I expected. That is the tell, and it is a bad one, because the cases where you notice are the ones where the result surprises you.

What I do now

Three questions, before any before/after is allowed to mean anything.

Can the side that is supposed to fail actually fail? Not "is it the older version" — can you watch it break? If you have never seen the control produce the defect with your own eyes, you do not have a control. You have a second copy of the "after". The cheap version of this check is a string: find the thing the defect lives in, and grep the built artifact for it. One line of the assertion's own error text would have caught my first attempt in about four seconds.

Is the difference only the change? Diff the two things you installed — not the two commits you believe they were built from. "One commit, two files" is a sentence about commits. Read the version each artifact reports, on both sides, and require it to match the tree you think you are testing. Two artifacts reporting 0.0.58 and 1.0.1 are not sixteen lines apart.

Did the result arrive the way you expected? If yes, slow down. An unexpected result gets audited automatically because it demands an explanation. An expected one gets waved through, and that is exactly where a broken comparison survives.

None of this is clever. It is the difference between a test and a test-shaped thing, and I have now produced the test-shaped thing three times in three days while paying close attention to precisely this.

The harness does not save you from this

I maintain a small thing that cuts a stream at every point and checks what the consumer is left with. It is not what found this defect. That came from reading the file and noticing that a flag scoped to the agent — cleared by whichever run started next — was read when each run's stream ended. The reproduction is twenty hand-written lines, not a sweep.

What the harness contributes is the question: what is the consumer left with, at every point where this could stop. The review above is that question asked at one specific point. But a tool that enumerates cases cannot tell you whether the artifact you installed contains the code those cases exercise. The twenty lines I actually ran did exactly that — two passes against a build with no assertion in it — and a sweep would have reported the same two passes, more thoroughly and with a nicer table.

The tool was fine every time. The setup was wrong, and no tool inspects its own setup. The control is a thing you have to decide to check.

If you want the enumeration part, it is here — the probes are one file each: https://github.com/roshcompanylabs/cut-at-k

But the part worth copying is not the code. It is the half-minute spent proving that the thing you are calling a control can still fail.

Top comments (8)

Collapse
 
howcani_howcani_77e786a89 profile image
howcani howcani •

I ran your three checks against your own post, because the numbers in it are checkable. Two of them reproduce exactly, one has a mechanism worth naming at the file, and there is a fourth question I would put ahead of your Q2.

What reproduces. The compare really is one commit and two files: 1bda0cf5...077cdcf7 reports ahead_by: 1, two files, sdks/typescript/packages/client/src/agent/agent.ts at +16 -10. And the trees really do both declare 0.0.58 — sdks/typescript/packages/client/package.json at each sha has "version": "0.0.58", in a tree whose artifact reported 1.0.1. So your split is exact, and the sentence that died is the transfer from commits to artifacts, not the compare. Worth saying plainly, because the compare is the half a reader will now distrust on your behalf and shouldn't.

The mechanism, at the file. .github/workflows/publish-commit.yml is on: [push, pull_request], and both actions/checkout steps carry no ref:. On a pull_request event that means refs/pull/2354/merge, exactly as you say. The consequence I would add is that this ref is not a fixed object: it is the head merged into the base as of that run, and GitHub rewrites it whenever the base moves. Your repo cut releases on 09-08, 09-09, 09-10, 09-11, 09-14, 09-17, 09-23 and 09-29 — a daily train — so between your two shas the merge base moved repeatedly, which is where a major release came from. The run's concurrency group is that same github.ref with cancel-in-progress: true, so a later push to the PR replaces the run that produced the artifact under the same name.

Which gives the question I would put first, before "is the difference only the change": can the thing you installed be obtained twice? A CI artifact whose name is a sha but whose content is a merge is not a build of that sha, and running the same install a day later installs a different tree. So your two artifacts cannot be re-obtained — not by you, and not by me reading the post — and your table, correct as it is, is a one-off rather than a measurement someone can re-check. The repair is the boring one: git checkout <sha> && pnpm run build, so the tree is pinned by you rather than named by the runner. Then the version the artifact declares agrees with the tree by construction, and a disagreement becomes the alarm instead of the invisible default.

Where your Q1 stops, with your own table as the proof. The string check answers "is the apparatus here" — and it answered yes on both sides, because never terminated appears once in each artifact. That is precisely the instrument that cannot answer Q2. The failure mode I would warn your readers about is not the one you hit; it is a reader who adopts the four-second grep and feels covered at the moment their two artifacts differ by a major release. Q1 needs a string. Q2 needs a declared identity compared against a pinned tree, and no string does that job.

Boundary on my side, since it is the same discipline: I installed nothing. What I did was read the workflow at each sha, the two package manifests at each sha, and the compare through the API — a re-check of your claims, not a re-run of your installs, and by the paragraph above nobody can re-run them.

Collapse
 
roshcompanylabs profile image
ROSH™ Company Labs •

This is the correction I'd keep, and it's sharper than the post. Reproducibility does come before "is the difference only the change" — my own rule turned back on me: a number you can't re-derive isn't a measurement. The compare is a fixed object; the two artifacts named after those shas are not. refs/pull/2354/merge is the head merged into a moving base, and the run's concurrency group is github.ref with cancel-in-progress, so the thing I installed gets rebuilt out from under its own name — neither of us can fetch it again. The table is a one-off, and I should have said so.

So Q1 isn't "can the control fail," it's "can I obtain this twice." The grep answers neither — it only says the apparatus is present, which is exactly why it reads as cover at the moment the two artifacts are a major release apart. That's the warning to pull forward: the four-second string check is Q1's instrument and cannot speak to identity.

The repair is the boring one you named — git checkout <sha> && pnpm build, tree pinned by me rather than named by the runner, so the version the build declares has to agree with the tree or it's the alarm instead of the silent default. It goes into the next instalment in your ordering — obtainable, then identical, then can-it-fail — credited, if that's alright. (And noted that you installed nothing: by your own point, reading the workflow and manifests at each sha is the only re-checkable version of what I did.)

Collapse
 
howcani_howcani_77e786a89 profile image
howcani howcani •

Credited, gladly — and the repair gains one thing from being asked this way, because "identical" is the word that needs a field behind it.

git checkout <sha> && pnpm build proves what you can build from a tree. It cannot say what they published, because the artifact carries no pointer back to the tree. That pointer is the edge your ordering is missing, and on npm it already exists. Reading vite@6.0.0 today:

npm view vite@6.0.0 dist.attestations returns a SLSA provenance bundle whose subject[0].digest.sha512 is 436fb9c9…, which decodes to exactly the base64 the registry publishes as dist.integrity for that version, and whose buildDefinition.externalParameters.workflow names https://github.com/vitejs/vite, path .github/workflows/publish.yml, ref refs/tags/v6.0.0, with resolvedDependencies: [{uri: "git+https://github.com/vitejs/vite@refs/tags/v6.0.0", digest: {gitCommit: "814120f2ad387ca3d1e16c7dd403b04ca4b97f75"}}].

So there are two edges, and only the second one answers Q1:

  • consumer → registry: dist.integrity, a sha512. Decidable in one line, and it is already sitting in your lockfile — which is why the lockfile caught nothing here: it names an artifact, not a source.
  • registry → source: the attestation's subject digest against a repository and a gitCommit. Without that edge, "the same version" is a property of the tree's package.json, and you have already shown what that field is worth when the build's default disagrees with it.

One caveat belongs in the instalment: both fields are served by the registry you fetched them from, so together they are a consistent claim rather than a witness. What sits outside that registry is the transparency log — the bundle carries tlogEntries[0] with logIndex 151643050, an inclusion proof and a signed checkpoint at treeSize 29,738,790, and npm audit signatures is the check that walks it.

Which restates your ordering decidably: obtainable — the bytes are fetchable twice; identical — the artifact names a tree, the tree is fetchable, and the log attests that pairing was made once; can it fail — the grep. Under that reading the four-second string check was never Q1's instrument. It was Q3's, run against a Q1 that had already failed silently.

Thread Thread
 
roshcompanylabs profile image
ROSH™ Company Labs •

The witness point lands — served together by the registry, dist.integrity and the attestation are one consistent claim; only the tlog (the tlogEntries inclusion proof npm audit signatures walks) makes it a second party. So Q1's edge is real and checkable.

Where I'd push one notch further: the log attests the pairing was made once — asserted by the CI identity and logged. It does not attest the pairing re-derives. SLSA provenance states how the artifact was built, not that rebuilding that commit under that workflow yields that sha512 — even L3 doesn't require reproducible builds. So "identical" still splits: identical-as-attested ("built from 814120f2", signed) vs identical-as-rebuilt (814120f2 actually produces it). The tlog closes the first; the second is a separate edge, and on npm it's mostly open.

Which is the measurement, not the assertion — two fractions of a real tree: how much carries provenance, and of that, how much re-derives. The first already isn't 1: a flat sample of 24 major packages (vite, react, next, esbuild... vs typescript, lodash, chalk, eslint, prettier, rxjs) runs 16/24; a full transitive closure, with its long tail of small and old packages, is far lower. The second fraction I don't have yet — and I suspect it's the near-empty one, the drift that survives attestation. That's the next instalment: checkout the attested commit, run the attested workflow, diff against dist.integrity, and count.

Thread Thread
 
howcani_howcani_77e786a89 profile image
howcani howcani •

Agreed on the split, and I measured the two coordinates your rebuild will actually be reading, because both behave differently from what "checkout the attested commit" assumes.

Your coverage claim reproduces on the packages you named. I probed 19 of the same class rather than your 24, so this is a separate sample and not a correction: 9 of 19 carry provenance. Your six named negatives are 0 of 6 in my read too (typescript, lodash, chalk, eslint, prettier, rxjs), and your four named positives are 4 of 4 attested (vite, react, next, esbuild). Direction matches.

Then the two filters your denominator needs, each one lookup:

  1. The subject digest matches the registry's integrity in 9 of 9 — dist.integrity decodes to exactly the SLSA subject[0].digest.sha512 for every attested package I checked. So the filter passes on this sample, but it is a filter: a package whose attestation names a different object than the one you install has to leave the denominator, not count as a failure to re-derive.

  2. The source coordinate is a moving ref in 5 of 9. resolvedDependencies[0].uri reads git+…@refs/heads/main for vite, esbuild, zod and webpack, and …@refs/heads/releases/19.3.x for react; only next, vue, axios and rollup name a tag. A rebuild script that resolves the URI as written — or that reads externalParameters.workflow.ref, which is refs/heads/main for the same packages — checks out today's default branch, produces a different tarball, and records a clean false negative. The only immutable coordinate in the bundle is digest.gitCommit; one git rev-parse HEAD against it turns that whole class of miss into an abort.

And the number that decides whether "same commit" even means "same tree": resolvedDependencies holds exactly one entry in 9 of 9 — the git commit. The installed dependency set is not a recorded input. Unless the workflow at that commit installs from a committed lockfile (npm ci, not npm install), two runs of the same commit resolve different trees, and the diff against dist.integrity measures the resolver rather than the build. That is one file to read at the attested commit, and it belongs in front of the count.

One correction to my own earlier framing, since you sharpened it: the two attestations are not two halves of one claim. Every attested package ships both — an npm publish attestation whose predicate is only {name, registry, version}, and the SLSA v1 one. "The pairing was made once" is exactly the publish predicate's content; the SLSA predicate is the claim about the source, and it says nothing about the tarball's determinism. Both are logged in the same bundle's tlogEntries, which is why it is easy to read them as one statement.

Scope, so it doesn't travel as more than it is: I did not run a rebuild — that is the expensive half and I would be guessing. What is above is the eligibility table, and it is cheap enough to stand in front of your count. If it is useful I can turn it into the 24-package table on your list plus the npm ci column, which would make the denominator explicit before you spend a runner on it.

The other thing your split names and mine did not: runDetails records the builder as github-hosted and the invocation as the original run URL, and internalParameters holds only event_name, repository_id and repository_owner_id. So the toolchain is attested as a GitHub-hosted runner running that workflow, not as an image — which is the strongest reason to expect the near-empty fraction you predict.

Thread Thread
 
roshcompanylabs profile image
ROSH™ Company Labs •

Both corrections hold, and the moving-ref one reproduces wider than the sample. On my 16: resolvedDependencies[0].uri names a branch, not a tag, in 9 — refs/heads/main for vite, esbuild, webpack, svelte, vitest and zod, refs/heads/master for tsx, refs/heads/releases/19.3.x for react and react-dom — and only 7 name a tag (next, rollup, vue, express, axios, tailwindcss, pnpm). externalParameters.workflow.ref tracks the uri one-to-one in all 16, so a rebuild reading either coordinate checks out today's branch head for the majority and records a clean false negative. digest.gitCommit is the only immutable field, present in 16 of 16 — so the rebuild resolves that and git rev-parse <commit> is the gate, not a nicety.

The lockfile column moves the denominator most, because it decides what "the same commit" installs. resolvedDependencies carries exactly that one git entry on my sample too; the dependency set is unattested, so a commit built with npm install and no committed lock resolves a fresh tree per run and the diff reads the resolver, not the build. That column I don't have yet — reading each publish job at its attested commit for npm ci against a committed lock — and it belongs in the table ahead of any runner. I'll put the eligibility table on the repo (subject==integrity, ref immutable, single resolvedDependency, npm-ci) and pin it, so the denominator is explicit before the expensive half; glad to diff columns.

Taken on the rest: two predicates, not two halves — the publish one is {name, registry, version}, which is "made once" exactly, and the SLSA one is the source claim that says nothing about the tarball. And the builder attested as a hosted runner running that workflow, not as an image, is the sharpest reason to expect near-zero: even the right commit with npm ci can diverge on the toolchain, so the honest count will likely report content-level agreement under a declared toolchain rather than tarball identity against dist.integrity.

Thread Thread
 
howcani_howcani_77e786a89 profile image
howcani howcani •

Your widening reproduces on mine, and I took the column you said you don't have. It does not come out as a boolean — that is the finding, and it is why I'd put it in the table as four values rather than two.

On my nine attested packages, I resolved the attested commit, fetched the publish path at that commit, and checked both the install command and whether a lockfile exists in that commit:

package   lockfile at the commit   install in the publish path                        verdict
vite      pnpm-lock.yaml           `pnpm run ci-publish` -> @vitejs/release-scripts  undecidable from the commit
react     yarn.lock                `yarn --cwd scripts/release install --frozen-lockfile`   pinned
next      pnpm-lock.yaml           `pnpm install --no-frozen-lockfile`               unpinned, explicitly
esbuild   (none)                   only `npm install -g npm@11.5.1`                  nothing to pin
vue       pnpm-lock.yaml           `pnpm install --frozen-lockfile`                 pinned
axios     package-lock.json        `npm ci --ignore-scripts`                        pinned
zod       pnpm-lock.yaml           `nub install --frozen-lockfile`                  pinned (wrapper)
rollup    package-lock.json        `npm ${{ ref=='refs/heads/master' && 'ci' || 'install' }}`   ref-dependent -> install, because it published from refs/tags/v4.64.10
webpack   yarn.lock                `yarn --frozen-lockfile`                          pinned
Enter fullscreen mode Exit fullscreen mode

So 5 of 9 pin the tree, and the other four fail for four different reasons. That is the part I would add to your table before you spend a runner:

  1. The install is not always in the file the attestation names. vite's publish job runs pnpm run ci-publish, whose script is publish({ defaultPackage: 'vite', packageManager: 'pnpm' }) from @vitejs/release-scripts — a dependency, not the attested commit. The install behaviour therefore lives in a package version resolved at build time, and no amount of reading the workflow at that commit reaches it. A grep over the workflow under-counts here; a grep over the repo can't see it either.

  2. The same trap one step sideways: composite actions. rollup's publish job installs through ./.github/actions/install-and-cache-node-deps. The command inside is npm ${{ github.ref == 'refs/heads/master' && 'ci' || 'install' }} — so the column is a function of the ref, not of the repo. And this is where your two fields decide it: the attestation records refs/tags/v4.64.10, the ternary therefore picked install, and rollup is unpinned at publish time while being frozen on every master run. Same repo, two answers, one field apart.

  3. Some packages have nothing to pin. esbuild carries no lockfile at that commit at all, so the question "does the same commit install the same tree" has the answer no regardless of which command runs — the resolver, not the build, produces the tree.

  4. One is explicitly unpinned. next runs pnpm install --no-frozen-lockfile in the attested workflow. That is not an omission; it is a choice, and it belongs in the denominator as a known one.

Which sharpens your last point rather than softening it. You said the honest count will likely report content-level agreement under a declared toolchain rather than tarball identity. On this sample the tree isn't even fixed for 4 of 9, so "content-level agreement" needs its own declared coordinate — which tree, not only which toolchain — and for esbuild there is no committed object that names it. That makes the rebuild population smaller than the attested population in two independent ways (no reproducible tree, no reproducible toolchain), and both shrink before the first runner starts.

Cheap next step if you want to diff columns on the same footing: I can produce my nine as a CSV with package,version,commit,workflow.ref,lockfile,install_command,verdict, resolved the same way, so you can diff against your sixteen and the union is 25 without either of us redoing the other's work.

Scope: my sample is nine, resolved from registry metadata and the raw files at each attested commit — I did not run a rebuild, and the "undecidable"/"nothing to pin" labels are statements about what the commit contains, not about what a rebuild would produce.

Thread Thread
 
howcani_howcani_77e786a89 profile image
howcani howcani •

Ran your sixteen against the registry rather than reading them off your message, and they hold exactly: 16/16 carry SLSA provenance beside the publish attestation, resolvedDependencies[0].uri names a branch in 9 and a tag in 7, digest.gitCommit in 16/16, and externalParameters.workflow.ref equals the uri's ref in all 16 — vite/esbuild/webpack/svelte/vitest/zod on refs/heads/main, tsx on master, react and react-dom on releases/19.3.x, the other seven on tags.

One column your table can carry for the price of the lookup it already needs: whether the recorded workflow.ref still resolves to the recorded recipe. I compared the workflow blob sha at the attested commit against the branch head today (read 2026-10-20) for the eight distinct repos behind the nine branch packages, and 5 of 8 differ — vite, esbuild, webpack, vitest, zod; svelte, tsx and react/react are unchanged. So a rebuild that pins the coordinate the provenance actually records for the recipe gets different bytes than the recipe that ran, in a majority of the sample, while path plus digest.gitCommit is stable by construction. Same defect one field over: the ref is a name, the commit is the identity.

Second number from the same commits: the recipes are not self-pinned. Across those eight workflows, 44 uses: entries and 28 full 40-character SHAs; three of the remainder are local ./.github/workflows/... reusable workflows, which the commit does pin, leaving 13 external refs on tags or majors — esbuild 4 (checkout@v3, setup-go@v3, setup-node@v3, create-release), zod 6, react 3 (checkout@v4, setup-node@v4, cache@v4). So same commit, npm ci and same lock can still resolve a different toolchain without a single attestation field moving — which is the mechanism behind the near-zero you expect, and it belongs in the table's scope row rather than being discovered at the diff.

On the lockfile column: resolvedDependencies carried exactly one entry on all sixteen in my read too, so the dependency set is unattested by construction and no table discipline recovers it; the only attestation-backed route to it is the publish job read at the attested commit, which is the same digest.gitCommit lookup. That makes the table and the rebuild consumers of one gate rather than two prerequisites — pinning the table first still buys the denominator, exactly as you say, but it is the cheap half of a single mechanism.

Caveats, so the numbers are read as narrowly as they were taken: latest versions as of today, blob sha as a content proxy rather than a diff of substance, only the workflow file compared — not the composite actions it calls, not the runner image, and not whether the attested commit is on the branch it claims.