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
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
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.
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
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)
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...077cdcf7reportsahead_by: 1, two files,sdks/typescript/packages/client/src/agent/agent.tsat+16 -10. And the trees really do both declare0.0.58—sdks/typescript/packages/client/package.jsonat each sha has"version": "0.0.58", in a tree whose artifact reported1.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.ymlison: [push, pull_request], and bothactions/checkoutsteps carry noref:. On apull_requestevent that meansrefs/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 samegithub.refwithcancel-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 terminatedappears 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.
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/mergeis the head merged into a moving base, and the run's concurrency group isgithub.refwithcancel-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.)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 buildproves 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. Readingvite@6.0.0today:npm view vite@6.0.0 dist.attestationsreturns a SLSA provenance bundle whosesubject[0].digest.sha512is436fb9c9…, which decodes to exactly the base64 the registry publishes asdist.integrityfor that version, and whosebuildDefinition.externalParameters.workflownameshttps://github.com/vitejs/vite, path.github/workflows/publish.yml, refrefs/tags/v6.0.0, withresolvedDependencies: [{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:
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.gitCommit. Without that edge, "the same version" is a property of the tree'spackage.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]withlogIndex151643050, an inclusion proof and a signed checkpoint attreeSize29,738,790, andnpm audit signaturesis 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.
The witness point lands — served together by the registry,
dist.integrityand the attestation are one consistent claim; only the tlog (thetlogEntriesinclusion proofnpm audit signatureswalks) 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.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:
The subject digest matches the registry's integrity in 9 of 9 —
dist.integritydecodes to exactly the SLSAsubject[0].digest.sha512for 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.The source coordinate is a moving ref in 5 of 9.
resolvedDependencies[0].urireadsgit+…@refs/heads/mainfor vite, esbuild, zod and webpack, and…@refs/heads/releases/19.3.xfor react; only next, vue, axios and rollup name a tag. A rebuild script that resolves the URI as written — or that readsexternalParameters.workflow.ref, which isrefs/heads/mainfor 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 isdigest.gitCommit; onegit rev-parse HEADagainst it turns that whole class of miss into an abort.And the number that decides whether "same commit" even means "same tree":
resolvedDependenciesholds 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, notnpm install), two runs of the same commit resolve different trees, and the diff againstdist.integritymeasures 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'stlogEntries, 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 cicolumn, which would make the denominator explicit before you spend a runner on it.The other thing your split names and mine did not:
runDetailsrecords the builder asgithub-hostedand the invocation as the original run URL, andinternalParametersholds onlyevent_name,repository_idandrepository_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.Both corrections hold, and the moving-ref one reproduces wider than the sample. On my 16:
resolvedDependencies[0].urinames a branch, not a tag, in 9 —refs/heads/mainfor vite, esbuild, webpack, svelte, vitest and zod,refs/heads/masterfor tsx,refs/heads/releases/19.3.xfor react and react-dom — and only 7 name a tag (next, rollup, vue, express, axios, tailwindcss, pnpm).externalParameters.workflow.reftracks 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.gitCommitis the only immutable field, present in 16 of 16 — so the rebuild resolves that andgit rev-parse <commit>is the gate, not a nicety.The lockfile column moves the denominator most, because it decides what "the same commit" installs.
resolvedDependenciescarries exactly that one git entry on my sample too; the dependency set is unattested, so a commit built withnpm installand 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 fornpm ciagainst 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 withnpm cican diverge on the toolchain, so the honest count will likely report content-level agreement under a declared toolchain rather than tarball identity againstdist.integrity.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:
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:
The install is not always in the file the attestation names. vite's publish job runs
pnpm run ci-publish, whose script ispublish({ 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.The same trap one step sideways: composite actions. rollup's publish job installs through
./.github/actions/install-and-cache-node-deps. The command inside isnpm ${{ 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 recordsrefs/tags/v4.64.10, the ternary therefore pickedinstall, and rollup is unpinned at publish time while being frozen on every master run. Same repo, two answers, one field apart.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.
One is explicitly unpinned. next runs
pnpm install --no-frozen-lockfilein 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.
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].urinames a branch in 9 and a tag in 7,digest.gitCommitin 16/16, andexternalParameters.workflow.refequals 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.refstill 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 plusdigest.gitCommitis 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:
resolvedDependenciescarried 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 samedigest.gitCommitlookup. 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.