Uh oh!
There was an error while loading. Please reload this page.
Codex's openai preset carries the x-hypaware-upstream rung, and the preset-slot race gets a regression test - #537
Conversation
… pinned by tests `registerUpstreamPreset` is a name-keyed last-write-wins `Map.set`, so two adapters that both register `openai` share one slot and the plugin that activates last owns it. Codex's registration declared neither a `match()` nor the `x-hypaware-upstream` rung, so if it ever won that slot a steered `openai-completions` turn (whose shadow base URL is the bare gateway origin, making the path `/chat/completions`) would match no upstream and the gateway would answer 404, killing the user's turn. LLP 0157 R5 forbids that. The safety net today is that `toposort()` breaks ties alphabetically and `@hypaware/codex` sorts before `@hypaware/openclaw`, so codex always activates first and never wins the contested slot. That is accidental and was untested. Two halves: - `matchOpenaiUpstream()` gives Codex's `openai` registration the same two rungs any copy of it needs: the `x-hypaware-upstream` header, then the `/v1` path anchor reproduced byte-for-byte from the `path_prefix` a `match()` supersedes. No traffic that exists today sends that header, so every current route is untouched. - `test/plugins/gateway-openai-upstream-slot.test.js` pins the real boot order off the shipped manifests via `resolveDependencies()`, and pins the compiled routing table built from the plugins' real `activate()` calls in both activation orders. The registration deliberately keeps the default priority rather than the `priority: 100` the triage suggested. `hyp init` writes gateway upstreams with no priority, so on an install that declares `anthropic` in config and leaves `openai` to Codex's preset, a priority of 100 sorts `openai` above `anthropic` and its `/v1` anchor then swallows `/v1/messages`. A test pins that ordering so the hazard cannot be reintroduced silently. Co-Authored-By: Claude <noreply@anthropic.com>
The reintroduction guard read Codex's own preset, so it could only catch `priority: 100` coming back in Codex's file. The constant is hazardous wherever it is written: a sibling adapter that registers `openai` activates later, wins the name-keyed slot, and reintroduces the misroute with the guard still green. Assert against whichever registration survived, under every activation order. Also pin the two bytes either side of the `/v1` segment boundary (`/v1/` and `/v1x`), since a `match()` supersedes the `path_prefix` it replaced and nothing else holds it to that shape. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Jul 31, 2026
REVIEW round 1: approve, with one finding fixed in-branchVerdict: approve once Baseline: 1. The |
| mutation | result |
|---|---|
remove match: matchOpenaiUpstream | 2 fail, both "a steered openai turn is routable" |
reverse toposort()'s ready.sort() tie-break (dep_graph.js:248,259) | 2 fail, both activation-order tests |
add priority: 100 back | 1 fail, the config-anthropic outrank test |
FINDING (moderate, fixed): the reintroduction guard was keyed to the plugin, not the slot
test/plugins/gateway-openai-upstream-slot.test.js:154 read presetsFromActivate([CODEX]), so it could only catch priority: 100 returning in Codex's file. But the whole premise of this PR is that openai is one name-keyed last-write-wins slot that a sibling adapter can win. The constant is hazardous wherever it is written, and the tier-2 routing tests that do compile the surviving table run with no config upstreams, the one shape where priority: 100 is harmless. So the guard had a hole exactly where the known live hazard sits.
I demonstrated it: I locally added #510's openaiUpstreamPreset() (priority: 100, /v1-wide match) to the OpenClaw plugin. OpenClaw activates last, wins the slot, and the misroute is back, and the test suite stayed green, 8/8. The defect #510 is currently neutral:stuck on would have walked straight through this PR's pin.
Fixed in 52ea8f9: the assertion now runs against whichever registration won the slot, across [CODEX], [CODEX, OPENCLAW], and [OPENCLAW, CODEX], and also asserts /v1/messages/count_tokens and a header-signature-carrying /v1/messages. Under the same simulated #510 reintroduction it now fails on the codex then openclaw order. Also added /v1/ and /v1x to the ROUTES table: the two bytes either side of the segment boundary, which nothing else held the hand-written anchor to.
5. @ref annotations resolve, and earn their place
LLP 0157#adapter-rework and #requirements both exist. #adapter-rework reads "Upstream routing is per request, selected by the x-hypaware-upstream metadata the steering plugin attaches, via the gateway's existing header match functions", which is exactly what the code does, and it tells a reader why a Codex plugin carries a header rung nothing currently sends, which the code and filename cannot. The R5 [constrained-by] ref is a mild extension (R5 is written about turns the plugin cannot steer, not steered-but-unroutable ones) but "the user's turn MUST NOT fail because of capture" is literal R5 text and applies directly. The file-level @ref in the test sits above a blank line, which matches established house practice (usage-policy-fold.test.js:24, query-local-only-visibility.test.js:9, and others).
Also verified the comment's claim that the header never leaves the gateway: forwardHeaders drops everything matching x-hypaware- after the recorder has captured the original headers (proxy.js:385). True as written.
6. House rules
Clean. No em dashes, no semicolon-terminated code (the one ; in the diff is prose inside a comment), JSDoc only, @import at file top, no @typedef, no inline import('...') types, no ./types.d.ts specifiers. typecheck passes, which is what proves the AiGatewayRouteInput import resolves.
Notes for the eventual #510 merge-base pass (no action here)
- No textual conflict.OpenClaw full capture (LLP 0157-0162) #510 touches neither
codex/src/index.jsnor the new test file;git merge-treeof this head againstorigin/pr-510produces no conflict markers. - There is a semantic one.OpenClaw full capture (LLP 0157-0162) #510's
openaiUpstreamPreset()has amatch()semantically identical to this PR's (itsisOpenaiPathandsteersToare line-for-line equivalent). The only difference ispriority: 100. Since OpenClaw activates after Codex, OpenClaw full capture (LLP 0157-0162) #510's copy wins the slot and this PR's deliberate priority-0 choice is silently overwritten. Dropping that constant from OpenClaw full capture (LLP 0157-0162) #510 is what makes the merge safe, and after52ea8f9the merged tree will say so in red rather than silently. - Steering is subordinate to the table sort, not absolute. A steered
openairequest on/v1/messagesstill resolves toanthropic(priority 100 preset sorts first), and on/backend-api/codex/*tochatgpt. This is the fail-closed direction and the bare-origin path the seam actually uses is unaffected, but it does mean LLP 0157's shadow base URL must stay at the bare gateway origin, since a/v1-rooted shadow URL would be swallowed byanthropicon any install that declares it at/v1. - A steered request also overrides an operator's catch-all config upstream (
config-catchallcells movelocal -> openai). Correct for an explicit upstream-naming header, but worth stating in LLP 0157 so an operator pointing everything at a local proxy is not surprised.
The tier-2 no-change tests walk `ROUTES` with a `for` loop, so an emptied table routed nothing and both tests still passed. Verified: with `ROUTES` set to `[]` on 52ea8f9 the file was green 10/10. Pin the coverage the table claims (a route per upstream, a 404 case, and the `/v1` boundary plus the bare-origin path) separately from walking it, so thinning the table reddens instead of quietly disarming the guard. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Jul 31, 2026
REVIEW round 2: approve, with one finding fixed in-branchVerdict: approve once Baseline: 1. The reintroduction guard genuinely reddens now (the check this round existed for)I did not take round 1's fix on trust. I re-ran its own experiment: added PR #510's
The failure is So the guard is a guard, not the appearance of one. That was the open question and it is closed. 2. Guard coverage: every way a sibling can win the slot today is coveredI enumerated the shapes a competing
Registering later in activation order is covered directly: the guard now runs all three of Residual gap, informational, not fixed: 3. The two new |
mutation to matchOpenaiUpstream | result |
|---|---|
widen to path.startsWith('/v1') | 2 fail (/v1x and /v1foo become openai) |
narrow to path.startsWith('/v1/') | 2 fail (/v1 itself stops routing) |
So /v1/ must match and /v1x must not, and both are now pinned. Round 1's 132,895-path equivalence proof said the anchor was right; these two cells are what keeps it right.
4. The header value matches what the counterparty actually sends
Round 1's differential asserted the rung fires on x-hypaware-upstream: openai, but assumed that string. I checked the writing side. resolveSteering returns requestMeta: { 'x-hypaware-upstream': candidate.provider } only after gating candidate.provider === CANONICAL_PROVIDER_FOR_SHAPE['openai-completions'], which is the literal 'openai' (openclaw-steering-plugin/src/steering.js:28-30,137-152 on #510's branch). So this PR's exact-match === 'openai' agrees with the only producer, and the 404 this PR exists to remove is genuinely removed rather than removed for a header nobody sends.
5. npm test picks the file up
Confirmed by name in the full run, not inferred: subtests 2756-2765 at 52ea8f9, all passing, inside the test/**/*.test.js glob npm test is scoped to.
FINDING (low, fixed): the route table could be disarmed by deleting it
test/plugins/gateway-openai-upstream-slot.test.js:113 walks ROUTES with a bare for loop and asserts nothing outside it. An emptied or thinned table therefore routes nothing and both "existing routes are unchanged" tests pass having proved nothing. I verified rather than asserted this: with const ROUTES = [] at 52ea8f9, the file is green 10/10.
That matters more here than in an ordinary test, because this file's entire job is to be the thing that stops a /v1-wide openai preset from swallowing /v1/messages, and a table that quietly shrinks in a future refactor takes the guard with it. Everything else in the file already has this covered - bundledManifests() asserts a non-empty discovery, the permutation loop asserts each manifest was found, the order loops are literals - so ROUTES was the one remaining hole.
Fixed in 8d79e9f: a separate test pins the coverage the table claims (a route for each of openai, chatgpt, anthropic and the 404 case, plus the /v1, /v1/, /v1x boundary and the bare-origin /chat/completions), independently of walking it. Under the same ROUTES = [] mutation the file now fails.
6. House rules
Clean at this head. No em dashes, no statement-terminating semicolons (the three ; in the diff are comment prose, a template literal, and a for header), JSDoc only, @import at file top, no @typedef, no inline import('...') types, repo-root-anchored .js specifiers. npm run typecheck passes.
Standing notes for the #510 merge-base pass (no action here)
Unchanged from round 1, plus one addition: #510 registers an openai upstream preset but its hypaware.plugin.json still declares gateway_upstream: { name: "anthropic", ... } only. Nothing enforces that block, so it is documentation drift rather than a defect, but it is why a manifest-derived slot-writer detector would not work.
philcunliffe
commented
Jul 31, 2026
TRIAGE rung (LLP 0017): review rounds are exhausted, so before parking this PR I re-read every finding both rounds left unresolved at the current head ( Re-verified, not retaken on faith:
The one residual: the reintroduction guard's third-writer gap (
Filed #538 ( |
Uh oh!
There was an error while loading. Please reload this page.
Item 1 (T6) of #536: the
openaiupstream preset slot race between@hypaware/codexand@hypaware/openclaw.The chain, re-verified
Triage's reasoning holds. I re-derived it from the code rather than taking it on faith:
registerUpstreamPresetis a name-keyed last-write-winsMap.set(hypaware-core/plugins-workspace/ai-gateway/src/api.js:55). Two plugins registeringopenaishare one slot; the one that activates last owns it.toposort()is Kahn's with an alphabetical tie-break (src/core/dep_graph.js:248,259).@hypaware/codexand@hypaware/openclawdeclare onlyrequires.capabilities, with no edge between them: the only bundled manifests with arequires.pluginsblock are@hypaware/ai-gateway-graphand@hypaware/hermes, and neither names either adapter.resolveDependencies()over the ai-gateway/codex/openclaw trio returns[ai-gateway, codex, openclaw]in all 6 input permutations.reload()never adds or removes plugins (src/core/daemon/runtime.js), so there is no way to activate codex after an already-running openclaw without a full restart, which re-derives the same order.So the 404 is unreachable today, and only because of an alphabetical accident that nothing declared and nothing tested.
What changed
Half 1, the tests.
test/plugins/gateway-openai-upstream-slot.test.js, two tiers that fail for different reasons on purpose:resolveDependencies()over manifests read off disk bydiscoverBundledPlugins(), and asserts codex resolves before openclaw, in the full bundled set and in every input permutation of the trio. A plugin rename, a newrequires.pluginsedge, or a change to the tie-break reddens this instead of silently 404-ing users.activate()against one gateway API in both activation orders, compiles the surviving preset table, and asserts a route matrix covering/v1/chat/completions,/v1/responses,/v1,/backend-api/codex/responses,/v1/messages,/v1/messages/count_tokens, the anthropic header signature off-path, the/v1fooprefix look-alike, and unsteered/chat/completions. The existing shape test (openclaw-projector.test.js) compares literal copies and never calls Codex's registration, which is why two review rounds could not see this.Half 2, the hardening. Codex's
openairegistration getsmatch: matchOpenaiUpstream, two rungs:x-hypaware-upstream: openai(LLP 0157 #adapter-rework: "upstream routing is per request, selected by thex-hypaware-upstreammetadata ... via the gateway's existing header match functions")./v1path anchor, reproducingpathMatchesPrefix(path, '/v1')exactly, because a preset that declaresmatch()never falls back to itspath_prefix.No behaviour change for existing traffic
The full route matrix above is asserted identical in both activation orders, and the same matrix run against the pre-change tree produces the same answers. Exactly one cell moves, and it is the defect:
/v1/chat/completions/v1/responses/v1/messages/backend-api/codex/responses/chat/completions(unsteered)/chat/completions+x-hypaware-upstream: openaiClaude and Codex traffic never sends
x-hypaware-upstream, so rung 1 is inert for every route that exists today.Deliberate deviation: no
priority: 100The triage suggested
priority: 100alongside thematch(). I did not add it, and there is a test pinning that decision, because it is not behaviour-neutral:composePickerConfigwriteshyp init's gateway upstreams straight from the pickercomposeblocks, which carry nopriority(so they compile at 0), and a config upstream beats a preset of the same name. On an install that declaresanthropicin config and leavesopenaito Codex's preset,priority: 100sortsopenaiaboveanthropic, andopenai's/v1anchor then swallows/v1/messages:Priority is not needed for the safety property either: with the header rung on both copies, the steered turn routes whichever registration wins the slot, so the 404 is gone regardless of activation order. The registrations now differ only in
priority, and that difference changes no route in any table where theanthropicside is itself a preset (both copies of it arepriority: 100).Fail-then-pass evidence
New file green, then reddened three ways:
pass 6, fail 2- both "a steered openai turn is routable ..." cases fail, in both activation orders.ready.sort()changed toready.sort().reverse()indep_graph.js:pass 6, fail 2- both activation-order cases fail.priority: 100added back to Codex'sopenaipreset:pass 7, fail 1- the config-anthropic ordering case fails.With the fix in place:
pass 8, fail 0.npm test: 3118 pass, 8 fail, all 8 the knowntest/core/leave-command.test.jsbaseline (#512, held in #527).npm run typecheck: clean.Interference with #510
No textual conflict.
integration/openclaw-full-capturetouches no file underhypaware-core/plugins-workspace/codex/, and none of the files it touches overlap the two files here.A merge-base pass is needed for three semantic reasons, all on #510's side:
openclaw/src/projector.js's comment onopenaiUpstreamPreset()says closing this race "needs the rung on Codex's registration too, which is a change to Codex's routing and so is not made here." That is now done; the comment should be rewritten to say the two registrations differ only inpriority.test/plugins/openclaw-projector.test.js's "the openai upstream preset matches the shape of the codex plugin registration" assertspriority === 100against literals and never reads Codex's registration, so its name will be wrong once merged. The tests here are the drift detector it was trying to be.openaiUpstreamPreset()carriespriority: 100, and openclaw activates second, so it wins the slot on a real boot. That reintroduces exactly the/v1/messageshazard shown above on an install with a config-declaredanthropicand no configopenai. Droppingpriority: 100from OpenClaw full capture (LLP 0157-0162) #510's copy makes the two registrations identical and removes the hazard.Items 2 and 3 of #536 (OpenClaw settlement as the only
.hypignoregate, and the stalewrite_homepermission) are untouched here: both live in code #510 rewrites, and item 2 is explicitly deferred to a live acceptance run.Fixes#536