Repository navigation
test(cross-version-wire): derive skew expectations from the baseline under test - #17178
Merged
Merged
Conversation
…under test The cross-version wire job pairs current code against whichever release tag is newest, so a hand-written "the old side does not have X" assertion expires by itself: v1.4.192 was the first tag containing the SnapshotStart `terminalOwner` field, and cutting it turned the new-client/old-server pairing red on unrelated pull requests with no code change anywhere. Read what each build publishes from that build. Each host is now paired against a client of its own version to produce a reference, and the skewed pairings are compared against that reference, so the expectation is whatever the release actually shipped. The same class of assertion in the agent-session suite — "the old build advertises no structured capability and registers no structured method" — becomes "each build's advertisement agrees with what it registers", and the "client too old to know this capability" is derived by removing the capability from the baseline's own list. The guard is unchanged in strength: a field the old host still publishes may not be dropped, skew may not change what a host puts on the wire, and a new pairing asserts the oracle still stalls when a peer cannot decode an opcode the other side sends.
Greptile SummaryThe PR replaces rolling-baseline assumptions with expectations derived from each build’s same-version behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts | Derives legacy capabilities from the baseline and adds per-method execution and result verification that addresses the prior review threads. |
| tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.ts | Replaces hard-coded rolling-baseline field assumptions with same-version reference comparisons and stronger compatibility guards. |
| tests/e2e/cross-version-wire/published-field-shape.ts | Adds reusable published-field shape comparison for additive wire evolution. |
| tests/e2e/cross-version-wire/terminal-skew-journey.ts | Expands journey recording so reference and skewed pairings can be compared without hard-coded release assumptions. |
| tests/e2e/cross-version-wire/versioned-agent-session-wire.ts | Extends the versioned-build harness to inspect and execute the structured surface in the selected checkout. |
| tests/e2e/cross-version-wire/versioned-terminal-wire.ts | Adds decoder capability manipulation used to prove incompatible terminal frames stall the harness. |
| docs/reference/remote-wire-compatibility.md | Documents why rolling baseline properties must be derived and when a pinned legacy release is appropriate. |
Sequence Diagram
sequenceDiagram
participant RC as Reference client
participant H as Host build
participant SC as Skewed client
RC->>H: Run same-version journey
H-->>RC: Reference publications
SC->>H: Run cross-version journey
H-->>SC: Skewed publications
Note over RC,SC: Compare skewed output with the publishing build's reference
SC->>H: Invoke declared agentSession methods
H-->>SC: Validate host call and declared result
Reviews (6): Last reviewed commit: "test(cross-version): make the release-sh..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughCross-version wire tests now derive expectations from same-version current and baseline journeys. New utilities compare published field sets and detect additions or removals. Agent-session tests derive legacy capabilities and registered methods from the baseline. Release loading uses Merge Risk: ⚪ Minimal · up to This PR updates cross-version compatibility tests and related documentation without changing production behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description provides a detailed explanation, rationale, before-and-after examples, testing results, and scope. However, it omits several required template sections, including the linked issue, explicit What Changed and Why headings, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist. Resolution Add the missing required template sections. Include a valid Linked Issue entry, explicit What Changed and Why sections, Visual Proof with N/A and a reason if applicable, Testing checkboxes and platform details, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist entries. Ensure the linked issue is included because the template requires one.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
brennanb2025
force-pushed
the
fix-cross-version-derived-expectations
branch
from
August 29, 2026 11:48
c43fabf to
c35a41a
Compare
… host gates The release-shaped checkout gate accepted any reply that was not method_not_found, so a registered-but-throwing handler passed it. The capability gate asserted a shared host spy had been called at all, so the second method mapped to that spy could stop reaching the host unnoticed.
Comment on lines
+380
to
+396
| it( | ||
| 'executes methods that a release-shaped checkout actually registers', | ||
| async () => { | ||
| const releasedCurrent = await loadAgentSessionWireBuild('HEAD') | ||
| expect(releasedCurrent.capabilities).toContain(STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY) | ||
| expect(releasedCurrent.methodNames).toContain('agentSession.createSupport') | ||
| const replies = await callBuild( | ||
| releasedCurrent, | ||
| 'agentSession.createSupport', | ||
| paramsFor('agentSession.createSupport'), | ||
| { clientKind: 'runtime', clientCapabilities: current.capabilities } | ||
| ) | ||
| expect(replies).toHaveLength(1) | ||
| // Anything weaker than the runtime's own answer passes a handler that is | ||
| // registered but unusable, which is the gap this test exists to close: | ||
| // any refusal other than `method_not_found` would satisfy it. | ||
| expect(replies[0]).toMatchObject({ ok: true, result: { supported: true } }) |
There was a problem hiding this comment.
When the rolling baseline registers structured methods other than agentSession.createSupport, the baseline loop accepts any reply except method_not_found, while this release-shaped execution check invokes only createSupport. An affected handler can therefore return structured_agent_session_unsupported, runtime_error, or another execution error without failing the compatibility suite, leaving registered methods unusable despite passing coverage.
…nt-session manifest The release-shaped checkout is the only place the "registered means usable" claim is executable today — the baseline release registers none of these methods — and it was exercising one of sixteen. A handler registered and returning an execution error passed the suite. - Declare each method's result in the manifest, so "answered" is the contract rather than "did not say method_not_found". - Give each build a seam to install a host into its own module slot; a release checkout has its own copy, so the working tree's host was never this dispatcher's, and every host-backed method answered structured_agent_session_unsupported — the capability gate's own words. - Run one execution contract over both skews instead of two divergent loops. - Pair the AI Vault never-called spy with a positive control; renaming the runtime method it watches left it green.
7 tasks done
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
…under test (stablyai#17178) * test(cross-version-wire): derive skew expectations from the baseline under test The cross-version wire job pairs current code against whichever release tag is newest, so a hand-written "the old side does not have X" assertion expires by itself: v1.4.192 was the first tag containing the SnapshotStart `terminalOwner` field, and cutting it turned the new-client/old-server pairing red on unrelated pull requests with no code change anywhere. Read what each build publishes from that build. Each host is now paired against a client of its own version to produce a reference, and the skewed pairings are compared against that reference, so the expectation is whatever the release actually shipped. The same class of assertion in the agent-session suite — "the old build advertises no structured capability and registers no structured method" — becomes "each build's advertisement agrees with what it registers", and the "client too old to know this capability" is derived by removing the capability from the baseline's own list. The guard is unchanged in strength: a field the old host still publishes may not be dropped, skew may not change what a host puts on the wire, and a new pairing asserts the oracle still stalls when a peer cannot decode an opcode the other side sends. * test(cross-version-wire): exercise release structured methods * test(cross-version-wire): load the registered method manifest * test(cross-version-wire): assert execution, not registration, on both host gates The release-shaped checkout gate accepted any reply that was not method_not_found, so a registered-but-throwing handler passed it. The capability gate asserted a shared host spy had been called at all, so the second method mapped to that spy could stop reaching the host unnoticed. * test(cross-version): make the release-shaped skew cover the whole agent-session manifest The release-shaped checkout is the only place the "registered means usable" claim is executable today — the baseline release registers none of these methods — and it was exercising one of sixteen. A handler registered and returning an execution error passed the suite. - Declare each method's result in the manifest, so "answered" is the contract rather than "did not say method_not_found". - Give each build a seam to install a host into its own module slot; a release checkout has its own copy, so the working tree's host was never this dispatcher's, and every host-backed method answered structured_agent_session_unsupported — the capability gate's own words. - Run one execution contract over both skews instead of two divergent loops. - Pair the AI Vault never-called spy with a positive control; renaming the runtime method it watches left it green. --------- Co-authored-by: Brennan Benson <brennanbenson@Brennans-MacBook-Pro.local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
The
cross-version wire compatibilityjob checks that a new Orca client and an old Orca server still understand each other. To get an "old server" it checks out the newest published release and runs it against your working tree.The test also contained a written-down list of things the old server doesn't have yet — for example, "an old server never puts
terminalOwneron a snapshot frame."That list has an expiry date nobody set.
terminalOwnermerged on 2026-08-26 in #16379. On 2026-08-29v1.4.192was cut, and it was the first tag containing that commit. From that moment "the newest release" did publishterminalOwner— so the test started failing, on whatever PR happened to be running, with nobody having changed a line of code.This PR stops writing the list down. Each build is now first paired with a client of its own version to find out what it actually publishes, and the skewed pairings are compared against that. "Old" now means old relative to the field, not old relative to today.
Before / after
Before — cutting a release could turn this job red on unrelated pull requests, because the test compared against "the newest release" and that release now contained the very field it asserted was absent:
After — the expectation comes from the baseline actually being tested, so only a real incompatibility fails it:
Same tree, same suite:
mainv1.4.192mainv1.4.191mainv1.4.184v1.4.192v1.4.191v1.4.184What replaced the written-down lists
Direction is now named in every comment, because this failure was misread as "a new client is sending an unknown field" when
snapshotStartsfiltersdirection === 'host-to-client'— the old server was publishing it.baselineReference— a baseline client against a baseline host. What that release publishes to its own client.currentReference— a current client against a current host. What this tree publishes to its own client.baselineReference— skew must not change what the old host publishes.currentReference— skew must not change what the new host publishes.published-field-shape.ts).The guard got stronger, not weaker: the old assertions could only say "these two names are absent", and were blind to any value the two pairings disagreed on.
Same shape elsewhere in the suite
cross-version-agent-session-wire.unit.test.tshad the same class, still latent only because no release has shippedagent-session.structured.v1yet:expect(baseline.capabilities).not.toContain(STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY)expect(baseline.methodNames.filter(name => name.startsWith('agentSession.'))).toEqual([])method_not_foundon the old dispatcherbaseline.capabilitiesas "a client that never asked for structured sessions"Those became: each build's advertised capability agrees with what it registers; the old dispatcher must always answer rather than fall silent, with
method_not_foundasserted for the methods that release genuinely lacks; and "a client too old to know this capability" is derived as the baseline's own list minus the capability.Simulating the next release shipping that surface (baseline advertises + registers it):
mainreddens 4 tests, this branch stays 10/10 green.cross-version-browser-placement.unit.test.tsis already immune — it pinsv1.4.184with a comment saying exactly why. Left alone, and the trade-off is now written down indocs/reference/remote-wire-compatibility.mdunder Never write down what the old side has.The guard still bites
A new permanent test pairs a build whose decoder cannot read the
Inputopcode — the shape a peer too old to know it has on the wire — and asserts the harness stalls and records the rejected frame, rather than passing.Mutation table, both baselines, mutation verified applied before each run:
displayMode(a field the old host still publishes)Each gate is 1-to-1: deleting the new-client/old-host comparator leaves M3 reddening only the other one, and vice versa; deleting additive-only leaves M1 reddening nothing.
Gates
pnpm tc(all projects) clean ·oxlint --deny-warningsclean ·check-changed-code-quality.mjs0 new findings ·git diff --checkclean · formatted withnpx oxfmt --writeon changed files only.release-checkout.unit.test.ts > keeps an import live while another colliding release label materializesfails pre-existing onmainin a local checkout (git rev-list --merges -1 HEADis empty onmain; CI runs on a merge ref). Verified by stashing this branch entirely and re-running. Not touched here.