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

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.

Comments

[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.