Refuse the launch when omp names itself with a version we cannot read - #393
Merged
schickling merged 2 commits intoAug 30, 2026
Merged
Conversation
…rough `find_release` gave the provider's own `<provider>/…` token the last word, but only when that token parsed. When it did not, the loop fell through and kept scanning, so any other release in the banner could stand in for the one the provider had just named. `omp/18.1.0-rc1 18.0.9` admitted on the stray `18.0.9` and launched a provider that reported itself as an unverified 18.1 pre-release — the exact outcome the function's own doc comment promises to prevent. The label now decides outright: if what the provider named cannot be parsed, the answer is `None` and the caller fails closed. Both shipped banners are unaffected, which was checked rather than assumed — omp prints `omp/18.0.11`, which parses, and opencode prints a bare `1.18.25`, which never enters the labelled branch at all. Latent today because the real binaries print one token each, but DQ-OMP-5 is open on precisely the update banner that would add a second one, and the exact-version gate that used to mask this is gone: keying on the minor means a stray token only has to land in an admitted series, not match a version exactly. Found by independent verification of #370 as mutation S6: deleting the fall-through left the suite byte-identical, so nothing asserted it. Two tests now do, one on the parser and one driving the production gate through a fake provider. Both go red under S6 in either shape — restoring the `continue`, or deleting the labelled branch outright. agent-identity: dev3.dotfiles.omp.admission-18-0-9.worker agent-persona: worker agent-supervisor: dev3.dotfiles-lead agent-tool: Claude Code agent-tool-version: 2.1.251 agent-runtime: Claude Code 2.1.251 tooling-profile: dotfiles@6048b77
The rule the previous commit documented — the provider naming itself decides — is true of the FIRST such token only: `find_release` returns on it. Two disagreeing own labels are therefore resolved by order, while two disagreeing UNLABELLED releases fail closed. No provider prints two own labels, so this records the behaviour rather than changing it; making the labelled case fail closed too would be more principled and is a separate question. agent-identity: dev3.dotfiles.omp.admission-18-0-9.worker agent-persona: worker agent-supervisor: dev3.dotfiles-lead agent-tool: Claude Code agent-tool-version: 2.1.251 agent-runtime: Claude Code 2.1.251 tooling-profile: dotfiles@6048b77
schickling-assistant
marked this pull request as ready for review
August 30, 2026 18:42
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.
Problem
find_releasegives the provider's own<provider>/…token the last word — but only when thattoken parses. When it does not, the loop falls through and keeps scanning, so any other release
in the banner can stand in for the one the provider just named.
#370 is merged, so this is live in shipped st2, not a defect in a PR under review. Driving the
deployed binary (re-resolved at probe time, reporting
0.1.0+c4d6f7a— this PR's base) with aprovider whose banner is
omp/18.1.0-rc1 18.0.9:omp said it was
18.1.0-rc1. The gate admitted on the stray18.0.9and started it — the exactoutcome the function's own doc comment promises to prevent. The same probe reproduced it against
the preceding generation (
0.1.0+08b67b3) too, so it is not an artifact of one build.Goal
The provider's own label decides outright. If what it named cannot be parsed, the gate refuses
instead of reading some other token, and the omp wrapper fails closed rather than launching an
unverified provider.
After, same probe, this branch:
Decisions
returninstead ofcontinue, not a rescan. The alternative — keep scanning and let anagreeing unlabelled release stand in — would reintroduce the bug in a narrower form. The label is
the provider's own claim about itself; an unreadable one is information, not noise.
Both real banners checked, not assumed. The change is only safe because neither shipped
provider reaches the new refusal:
find_releaseomp/18.0.111.18.25Measured under the environment that actually applies. The gate runs
<omp> --versionbeforethe wrapper assembles its offline defaults, so it reads the banner under the ambient environment:
One token either way on this build, so the defect is latent today — measured, not asserted.
Fixed now rather than when it becomes reachable. Two things make "latent" a thin margin.
DQ-OMP-5 is open on precisely the update banner that would add a second token, and the ambient
call above is exactly the call it would pollute. And the exact-version gate used to mask this: a
stray token had to equal an admitted version exactly. #370 keyed on the minor, so a stray token
now only has to land anywhere in an admitted series — #370 widened the reachable form of this bug,
and this closes it.
Documented one asymmetry rather than changing it.
find_releasereturns on the FIRSTown-labelled token, so two disagreeing own labels resolve by order while two disagreeing
unlabelled releases fail closed. No provider prints two own labels. The second commit records
that in the doc comment and the spec; making the labelled case fail closed too would be more
principled and is a separate question.
Verification
The tests were not inert — that was the finding. This was found by independent verification of
#370 as mutation S6: deleting the labelled fall-through left the suite byte-identical, because
nothing asserted it. The two tests that looked like coverage did not reach it — one used a
parseable label, the other a foreign label.
Two tests now assert it: one on the parser, one driving the production gate
(
verify_supported_version) through a fake provider binary. Both were written first and observedred before the fix, and both go red again under S6 in either shape:
harness_versiontestomp_sessiontestcontinueBoth tests run in the CI sandbox — read from the run log, not inferred from a green tick,
because
nix flake checkruns only the hermetic portion of the suite and the production-path testspawns
installand execs a shell script:CI green on this exact head (
25d1854):check-x86_64-linuxandcheck-aarch64-darwinbothsuccess — run
33326134146.
Pre-flip deviation. Local
cargo testis green on this branch excepteval_run_e2e::canonical_agents_freeze_the_admitted_route_across_post_boot_catalog_mutation,which also fails on pristine
origin/main(c4d6f7a) in the same worktree — reproduced at themerge-base, not assumed. It is outside the hermetic set
nix flake checkruns, which is why main'sCI is green.
cargo fmt --checkreports diffs across ~37 files onmain, including lines in the two files thisPR touches that it does not modify.
flake.nixstates outright that fmt and clippy areintentionally not gated, so formatting was left alone.
Complexity
No new complexity: one branch in one function goes from "keep looking" to "answer now", and the
control flow gets simpler. No new abstraction, dependency, or module boundary. +59 / −9 across
three files, most of it tests and prose.
Concerns
The refusal is strictly stricter than before, so a provider that starts printing
<provider>/<something-unparseable>as its normal banner would now fail to launch where itpreviously launched on a stray token. That is the intended direction — failing closed on an
unreadable self-report is the point — but it is a behaviour change, and it would surface as a
launch refusal rather than a warning.
Friction & bottlenecks
Friction. Probing the gate end-to-end requires a throwaway catalog and a fake provider
executable, because a passing gate makes the wrapper actually launch the provider. There is no
read-only "would this version be admitted?" surface, so verifying gate behaviour against a real
binary is not something a reviewer can do casually. Logged rather than fixed here.
Follow-ups
MEASURED_CONTEXT_VERSIONSis["18.0.9", "18.0.3"]while the fleet ships 18.0.11. The launchgate admits it; the harness-context token arithmetic was never measured on it, and the pin test
only asserts measured ⊆ gate, so it stays green either way.
omp_session.rsstates these twoquestions must not be collapsed — flagged, not touched here, and tracked separately.
--versioncall,given DQ-OMP-5.
Not in this PR
The codex gate.
src/codex_app_server.rsstates that semantic-version proximity is notcompatibility evidence for that surface, which forbids the reasoning a version-series gate needs.
References
docs/vrs/06-omp-driver/spec.md— updated with which token the release is read from.docs/vrs/06-omp-driver/open-questions.md.Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile