[hotfix] v0.9.3 prefill safety net + Codex/Ollama silent redirects (A/8)#572
Merged
jalehman merged 3 commits intoMay 3, 2026
Conversation
Three regressions introduced inside v0.9.3 itself, each a self-contained hunk: * assemble: restore PR Martian-Engineering#504 reference-inequality contract. The no-user-turn fallback added by PR Martian-Engineering#502 returned `params.messages` by reference, defeating the `installContextEngineLoopHook` identity check installed by PR Martian-Engineering#504 (`assembled.messages !== sourceMessages`). The hook then fell through to raw sourceMessages (still ending with assistant), re-introducing the prefill-rejection bug fixed by safeFallback in the other early-return paths. Use safeFallback() here too. Closes Martian-Engineering#559 (sub-fix A1). * assembler: strip assistant messages whose only blocks are blank text (`[{type:"text", text:""}]`). PR Martian-Engineering#506 added isThinkingOnlyContent for the Bedrock empty-content case, but blank-text blocks pass that filter and Bedrock still rejects with `The text field in the ContentBlock object at messages.N.content.0 is blank`. New isBlankContent helper mirrors the thinking-only shape and is added to the cleanedEntries filter. Closes Martian-Engineering#559 (sub-fix B5). * plugin: respect explicit `https://api.openai.com/v1` for openai-codex. PR Martian-Engineering#549's OPENAI_CODEX_NATIVE_BASE_URLS would rewrite an explicitly-configured api.openai.com/v1 to chatgpt.com/backend-api/codex whenever api was openai-codex-responses. That broke paid OpenAI API-key users who chose the endpoint deliberately — their key cannot authenticate against the ChatGPT backend. shouldUseNativeCodexBaseUrl now accepts an isExplicitlyConfigured flag (set when configuredBaseUrl is non-undefined) and skips the rewrite for that case; the native rewrite still applies when baseUrl is empty or already a ChatGPT Codex variant. Closes Martian-Engineering#560 (sub-fix A2). * plugin: remove silent ollama localhost fallback. PR Martian-Engineering#546's `inferBaseUrlFromProvider['ollama']` mapping silently routed cloud ollama configs (`https://ollama.com`, per Martian-Engineering#480) to localhost. Drop the entry so any ollama user must configure an explicit baseUrl; failing with an empty baseUrl is loudly diagnosable, while a wrong localhost connect is not. Closes Martian-Engineering#560 (sub-fix A3). Sub-fix A4 (Martian-Engineering#561's bootstrap raw-hash anchor) was deferred from this PR after a deeper audit: the original recommendation conflicts with the heartbeat-prune append-only fast path because the same \`lastProcessedEntryHash\` field serves both DB-tail integrity (post- prune, post-externalization) and raw-transcript checkpoint roles. Will be addressed alongside the bootstrap dedup migration in a follow-up. Tests updated for each sub-fix: - engine.test.ts: assembler regression test for blank-text blocks; existing no-user-turn fallback test rewritten to assert reference-inequality. - index-complete-provider-config.test.ts: new test for explicit api.openai.com/v1 preservation; existing ollama test updated and paired with a new cloud-ollama explicit-baseUrl test. Refs Martian-Engineering#554, Martian-Engineering#496, Martian-Engineering#499, Martian-Engineering#505, Martian-Engineering#509, Martian-Engineering#519, Martian-Engineering#480, Martian-Engineering#436, Martian-Engineering#508, Martian-Engineering#541, Martian-Engineering#251.
There was a problem hiding this comment.
Pull request overview
Hotfix PR addressing v0.9.3 regressions that caused silent breakage in (a) context assembly prefill safety nets and (b) provider baseUrl routing for Codex and Ollama, with targeted regression tests.
Changes:
- Assembly: ensure the “no user turn” fallback uses
safeFallback()(new array + trailing-assistant stripping) and filter assistant messages with blank-text blocks during assembly. - Provider routing: prevent rewriting an explicitly configured
https://api.openai.com/v1Codex baseUrl to the ChatGPT Codex backend; remove Ollama’s implicit localhost baseUrl inference. - Tests/changeset: add regression coverage for the above and document the hotfix.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/engine.ts |
Fixes assemble fallback to use safeFallback() to preserve the reference-inequality contract relied on by the gateway hook. |
src/assembler.ts |
Adds isBlankContent and filters assistant messages with blank/whitespace-only text blocks to avoid Bedrock rejections. |
src/plugin/index.ts |
Adjusts Codex native baseUrl rewrite logic to respect explicitly configured baseUrls; removes inferred Ollama localhost baseUrl. |
test/engine.test.ts |
Updates regression assertions for fallback reference-inequality and adds a blank-text content filtering test. |
test/index-complete-provider-config.test.ts |
Adds/updates tests ensuring explicit Codex/Ollama baseUrls are preserved and Ollama no longer defaults to localhost. |
.changeset/hotfix-v093-prefill-providers.md |
Patch changeset describing the bundled hotfix items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The previous comment claimed "forces an explicit baseUrl" but `resolveProviderModelBaseUrl` still passes `""` through to the dispatcher when no other source yields a value. Reword to reflect actual behavior: returning undefined drops the inferred default and the empty-baseUrl error is the deliberate clearer-failure path, not a configuration enforcement. Addresses Martian-Engineering#572 (review).
Merged
This was referenced May 3, 2026
This was referenced May 3, 2026
Merged
4 tasks
jalehman
pushed a commit
that referenced
this pull request
May 11, 2026
…y assistant (#606) The assemble pass's `cleanedEntries` filter only guarded `assistant` messages against empty/blank content. An empty `user` or `toolResult` content array produced upstream during incremental compaction could survive the cleaned-tail filter and reach Bedrock Converse, which rejects it with the literal wording: The content field in the Message object at messages.N is empty. Add a ContentBlock object to the content field and try again. A direct probe against Bedrock confirms this exact wording is reproducible only when `content === []`; bare strings or non-empty arrays produce a different (`SerializationException`) error. The asymmetry between the assistant guard and the missing user/toolResult guard explains why the 0.9.3 prefill safety net (#572) and the 0.9.4 blank-text guard did not fully close the validation surface. Extract a unified `isEmptyMessageContent` helper that drops `undefined`, `null`, empty-string, whitespace-only string, and empty-array content for any role, while preserving the existing assistant-only thinking-only and blank-text guards. Dropping a `toolResult` here is safe — `sanitizeToolUseResultPairing` runs immediately below and re-pairs missing results with a synthetic `[lossless-claw] missing tool result …` placeholder. Add `test/assembler-empty-content.test.ts` covering 28 cases: universal empty handling across all three roles, non-empty preservation, the assistant-only thinking/blank-text guards, and a direct regression for the production failure shape. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Closes #559, closes #560.
Three regressions introduced inside v0.9.3 itself, each a self-contained hunk. All four sub-fixes ship together because they share a single hotfix-shaped theme — silent breakage from changes that landed in v0.9.3 — and a reviewer should look at all of them at once to confirm the scope is regression-only.
(Sub-fix A4 from #561 — the bootstrap raw-hash anchor — was deferred from this PR. The original audit recommendation conflicts with the heartbeat-prune append-only fast path because the same
lastProcessedEntryHashfield serves two roles: DB-tail integrity check (post-prune, post-externalization) and raw-transcript checkpoint. Doing it right needs the dedup work in a follow-up.)Sub-fixes
[assembly]reference-inequality contract on no-user-turn fallback (#559 A1)PR #502's no-user-turn guard returned
{ messages: params.messages, estimatedTokens: 0 }by reference. PR #504 (merged ~3 hours apart) added the contract that all fallback paths must return a new array viasafeFallback(), soinstallContextEngineLoopHook'sassembled.messages !== sourceMessagescheck treats the result as assembled context. #502's guard violated that contract — when it fired, the gateway hook fell through to rawsourceMessages(still ending with assistant), re-introducing exactly the prefill-rejection bug fixed by #504 (#499, #505).Fix: replace
return { messages: params.messages, estimatedTokens: 0 }withreturn safeFallback();(helper already in scope).[assembly]blank-text-block survival incleanedEntries(#559 B5)PR #506 added
isThinkingOnlyContentto the assembler'scleanedEntriesfilter to fix Bedrock empty-content rejection from thinking-only messages (#519, #509). Blank text blocks[{type:"text", text:""}]aren't thinking-only, so they pass the filter — and Bedrock still rejects them withThe text field in the ContentBlock object at messages.N.content.0 is blank.Fix: new
isBlankContenthelper mirroring theisThinkingOnlyContentshape; added to thecleanedEntriesfilter and the non-array-content check (which now also rejects whitespace-only strings).[provider]paid OpenAI API-key Codex redirect (#560 A2)PR #549's
OPENAI_CODEX_NATIVE_BASE_URLSrewrites baseUrls tohttps://chatgpt.com/backend-api/codexwhenever api isopenai-codex-responses. The native-rewrite list includeshttps://api.openai.com/v1— so users on a paid OpenAI API key who configuredmodels.providers.openai-codex.baseUrl: https://api.openai.com/v1get silently redirected to the ChatGPT backend, which their API key cannot authenticate against. This is exactly the audience PR #500 (Truck0ff) targeted with per-model api overrides.Fix:
shouldUseNativeCodexBaseUrlnow accepts anisExplicitlyConfiguredflag;resolveProviderModelBaseUrlsets it whenconfiguredBaseUrlis non-undefined. When explicit AND normalized baseUrl equalsOPENAI_API_BASE_URL, return false (don't rewrite). The native rewrite still applies when baseUrl is empty (default) or already a ChatGPT Codex variant.[provider]ollama localhost fallback breaks cloud configs (#560 A3)PR #546 added
ollama: http://localhost:11434toinferBaseUrlFromProvider. Cloud-only ollama users (https://ollama.com, per #480) without a model-catalog entry now get rewritten to localhost — guaranteed connection refused.Fix: remove the ollama entry from
inferBaseUrlFromProvider. Cloud and self-hosted setups both rely on explicit baseUrl; an empty baseUrl produces a clearer error than a silent localhost connect. Local users typically already configure baseUrl explicitly, but if not, a missing-baseUrl error is a far better failure mode than a silent wrong-target connection.Tests
test/engine.test.ts: existingcold-cache new sessiontest rewritten to assert reference-inequality (not.toBe(liveMessages)+toStrictEqual(liveMessages)); new testfilters assistant messages with blank-text content during assemblycovering A1's blank-text gap; one regression test per sub-fix.test/index-complete-provider-config.test.ts: new testpreserves an explicit api.openai.com/v1 baseUrl for paid OpenAI API-key Codex users; existing ollama test updated (now assertsbaseUrl: ""); new testpreserves a user-configured ollama cloud baseUrl instead of overriding to localhost.Total: 836 tests pass (was 833 on baseline, +3 regression tests).
Test plan
npm ci && npm testclean on the branch (45 files, 836 tests, 0 failures)npm run buildclean (660kb dist/index.js)result.messages !== liveMessages(was===previously)[{type:"text",text:""}]; confirm filtered from assembled outputopenai-codexwith explicithttps://api.openai.com/v1; confirmcompleteSimplereceives that exact baseUrl (was rewritten tochatgpt.com/backend-api/codexon v0.9.3)ollamawithhttps://ollama.com; confirmcompleteSimplereceiveshttps://ollama.com(was rewritten tolocalhost:11434on v0.9.3)Cross-links
completeSimplefails withConnection errorfor OpenClawclaude-cliSonnet transport #508, openai-codex/* summary calls return 404 Not Found from responses-API path (regression after #273) #541, summaryApi / API format override for LLM summarization calls #251 (provider/auth chain)