From da1e5dd28daeff8b063d38d3ef19f95406659c1b Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 11:50:08 +0900 Subject: [PATCH 1/2] test(protocols): cover external image wire roundtrips --- .../000_plan.md | 9 +- .../020_wire_contract.md | 15 +++ structure/04_transports-and-sidecars.md | 2 +- .../chat-completions-endpoint.test.ts | 92 +++++++++++++- .../openai-responses-passthrough.test.ts | 118 ++++++++++++++++++ 5 files changed, 232 insertions(+), 4 deletions(-) diff --git a/devlog/_plan/260905_external_image_roundtrip/000_plan.md b/devlog/_plan/260905_external_image_roundtrip/000_plan.md index 8c3e09a673..6d76a4a18c 100644 --- a/devlog/_plan/260905_external_image_roundtrip/000_plan.md +++ b/devlog/_plan/260905_external_image_roundtrip/000_plan.md @@ -14,7 +14,8 @@ - Outcomes: DONE only with proof; external dependencies may be BLOCKED/NEEDS_HUMAN; unsafe expansion is UNSAFE. No implementation-success claim from docs-only work. - Scope: this managed checkout, read-only Aside official docs, GitHub stack/CI/admin - merge. Maximum four concurrent agents; reassess after 90 minutes; no token cap set. + merge. The user's follow-up permits unlimited useful parallel agents (subject to + actual tool capacity); reassess after 90 minutes; no token cap set. - Escalation: reclaim a lane after two distinct failed agents; any delegated writes must be planned with disjoint paths before B. No production credentials in artifacts. @@ -55,6 +56,12 @@ converter and protects the integrated contract independently of unit-level asser ## Continuity +wp1 outcome: commit `1f1daa368` implements 010 with ten converter regression cases; +draft PR #3586 targets dev. Independent patch reviewer inspected both changed files +and returned PASS. Standalone request JSON changed from image/detail missing (exit 1) +to both retained (exit 0); node TypeScript and privacy scan passed. Suites are CI-only, +not claimed green yet. wp2 inherits this verified converter and adds wire/HTTP evidence. + Roadmap audit: independent gpt-6-astra high reviewer returned GO-WITH-FIXES, two medium findings. Both folded: exact no-suite typecheck/push commands and actual Claude converter export. Direct node tsc exits 0. Standalone reproduction at diff --git a/devlog/_plan/260905_external_image_roundtrip/020_wire_contract.md b/devlog/_plan/260905_external_image_roundtrip/020_wire_contract.md index 7baf039379..4e9ae1c581 100644 --- a/devlog/_plan/260905_external_image_roundtrip/020_wire_contract.md +++ b/devlog/_plan/260905_external_image_roundtrip/020_wire_contract.md @@ -2,6 +2,17 @@ Depends on wp1 and its corrected Chat converter. One full PABCD cycle. +Delegated B lanes (user reconfirmed unlimited useful parallelism): worker A exclusively +edits tests/responses/openai-responses-passthrough.test.ts; worker B exclusively adds +the HTTP regression in tests/responses/chat-completions-endpoint.test.ts. Main owns +public docs, structure, devlog and git/CI. C reviewers are read-only and independent. +All lanes prohibit local test suites, services, config/auth and git/FSM mutation. + +User steering during B: all image representations must be audited before merge. This +cycle now publishes the wire-contract child; the original exact-head CI/merge/ancestry +criterion is unchanged and moves to appended wp3 after the expanded audit. No criterion +is dropped or marked met early. Only existing 020 implementation runs in this B. + ## MODIFY tests/responses/openai-responses-passthrough.test.ts Import real chatCompletionsToResponsesBody, anthropicToResponsesBody, @@ -38,6 +49,10 @@ existing Chat-to-Responses HTTP regression (line 2834). POST a user image with h detail and a paired tool screenshot to mock/grok-4.5; consume the stream and assert one captured /responses body with unchanged ordered image parts. This is real HTTP route proof in CI, not real-model OCR or canonical account authentication. +The manual HTTP probe showed that data-only mock Responses frames don't satisfy the +native event-name terminal observer. Add matching `event: response.output_text.delta` +and `event: response.completed` fields to mockDualWireUpstream's existing frames; +preserve all body assertions and require `[DONE]` on the new HTTP cases. ## MODIFY structure/04_transports-and-sidecars.md diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index 780e2bd08d..1ceaa12f7d 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -1427,7 +1427,7 @@ surface is listed here so a maintainer can find the owner without grepping: | Adapter execution support | `src/adapters/run-turn-queue.ts`, `src/adapters/tool-catalog-nudge.ts`, `src/adapters/identity.ts`, `src/adapters/image.ts`, `src/adapters/upstream-http-error.ts` | Shared machinery: turn ordering, tool-catalog nudging, client fingerprinting, image conversion, upstream error normalization. | | Cursor (beyond the sections above) | `src/adapters/cursor/live-transport.ts`, `src/adapters/cursor/http1-bidi.ts`, `src/adapters/cursor/live-models.ts`, `src/adapters/cursor/transport-retry.ts`, `src/adapters/cursor/mcp-manager.ts`, `src/adapters/cursor/thread-continuity.ts`, `src/adapters/cursor/checkpoint-store.ts` | Thread continuity is the point: a retry must not start a new Cursor thread, and a validated checkpoint must not rebuild the full root history. HTTP/2 remains the default; an explicit `http1.1`/`h1` pin maps the bidi run onto Cursor's `RunSSE` receive stream plus sequenced `BidiAppend` sends, and applies to live discovery too. | | Claude Messages | `src/server/claude-messages.ts` | Routed translation, a native Anthropic passthrough branch, and `count_tokens`. | -| Chat Completions inbound | `src/server/chat-completions.ts`, `src/chat/` | Inbound translation onto the same routing pipeline. | +| Chat Completions inbound | `src/server/chat-completions.ts`, `src/chat/` | Inbound translation onto the same routing pipeline. The content mapper preserves image URLs and supported detail, including screenshot-bearing tool results; target adapters own image placement on their wire. Image-free tool results stay strings. | | Hosted search relay | `src/server/search.ts` | Direct relay; distinct from the web-search sidecar loop below. | | Image/video generation loop | `src/images/loop.ts`, `src/images/plan.ts`, `src/images/fulfill.ts`, `src/images/xai-client.ts`, `src/images/xai-video-client.ts`, `src/images/artifacts.ts` | A provider-returned image URL is downloaded into a local artifact once, then served locally; warnings stay URL-free because provider CDN URLs may embed credentials. | | GitHub Copilot | `src/providers/xai-transport.ts` (`resolveProviderTransport`), `src/providers/github-copilot-transport.ts` | `resolveProviderTransport` selects the Copilot transport when the routed provider name is `github-copilot`; the Copilot module then resolves its headers and base URL, and the registry seeds the provider row and model fallback. | diff --git a/tests/responses/chat-completions-endpoint.test.ts b/tests/responses/chat-completions-endpoint.test.ts index 4a4a5a5a12..b622027a97 100644 --- a/tests/responses/chat-completions-endpoint.test.ts +++ b/tests/responses/chat-completions-endpoint.test.ts @@ -122,8 +122,8 @@ function mockDualWireUpstream() { if (url.pathname.endsWith("/responses")) { const frames = [ - `data: ${JSON.stringify({ type: "response.output_text.delta", delta: "ok" })}\n\n`, - `data: ${JSON.stringify({ + `event: response.output_text.delta\ndata: ${JSON.stringify({ type: "response.output_text.delta", delta: "ok" })}\n\n`, + `event: response.completed\ndata: ${JSON.stringify({ type: "response.completed", response: { id: "resp_1", @@ -2934,6 +2934,94 @@ test("inbound chat-completions honors the override when stripping sampling (#404 } }); +test.each([ + { model: "grok-4.5", pathname: "/v1/responses" }, + { model: "gemini-3-pro", pathname: "/v1/chat/completions" }, +])("inbound chat images and paired screenshots survive the $model wire", async ({ model, pathname }) => { + const png = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII="; + const screenshot = "https://example.com/tool-screenshot.png"; + const { server: upstream, captured } = mockDualWireUpstream(); + let server: ReturnType | undefined; + try { + saveConfig(dualWireConfig(`${upstream.url.toString().replace(/\/$/, "")}/v1`)); + server = startServer(0); + const response = await fetch(new URL("/v1/chat/completions", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + model: `mock/${model}`, + stream: true, + // Force the Chat sibling through Responses translation, not native Chat passthrough. + store: true, + messages: [ + { role: "user", content: [ + { type: "text", text: "Inspect this image." }, + { type: "image_url", image_url: { url: png, detail: "high" } }, + { type: "text", text: "Compare it with the screenshot." }, + ] }, + { role: "assistant", content: null, tool_calls: [ + { id: "call_screenshot", type: "function", function: { name: "screenshot", arguments: "{}" } }, + ] }, + { role: "tool", tool_call_id: "call_screenshot", content: [ + { type: "text", text: "Before screenshot." }, + { type: "image_url", image_url: { url: screenshot, detail: "low" } }, + { type: "text", text: "After screenshot." }, + { type: "image_url", image_url: { url: png, detail: "auto" } }, + ] }, + ], + }), + }); + expect(response.status).toBe(200); + expect(await response.text()).toContain("[DONE]"); + + expect(captured).toHaveLength(1); + expect(captured[0]!.pathname).toBe(pathname); + expect(captured[0]!.body.model).toBe(model); + if (pathname === "/v1/responses") { + expect(captured[0]!.body.input).toEqual([ + { type: "message", role: "user", content: [ + { type: "input_text", text: "Inspect this image." }, + { type: "input_image", image_url: png, detail: "high" }, + { type: "input_text", text: "Compare it with the screenshot." }, + ] }, + { type: "function_call", call_id: "call_screenshot", name: "screenshot", arguments: "{}" }, + { type: "function_call_output", call_id: "call_screenshot", output: [ + { type: "input_text", text: "Before screenshot." }, + { type: "input_image", image_url: screenshot, detail: "low" }, + { type: "input_text", text: "After screenshot." }, + { type: "input_image", image_url: png, detail: "auto" }, + ] }, + ]); + } else { + const messages = captured[0]!.body.messages as Array>; + const conversation = messages.filter(message => message.role !== "system"); + expect(conversation.map(message => message.role)).toEqual(["user", "assistant", "tool", "user"]); + expect(conversation[0]).toEqual({ role: "user", content: [ + { type: "text", text: "Inspect this image." }, + { type: "image_url", image_url: { url: png, detail: "high" } }, + { type: "text", text: "Compare it with the screenshot." }, + ] }); + expect(conversation[1]!.tool_calls).toEqual([ + { id: "call_screenshot", type: "function", function: { name: "screenshot", arguments: "{}" } }, + ]); + expect(conversation[2]).toEqual({ + role: "tool", tool_call_id: "call_screenshot", content: "Before screenshot.After screenshot.", + }); + expect(conversation[3]).toEqual({ role: "user", content: [ + { type: "text", text: "[ocx] image output from the preceding tool result(s):" }, + { type: "image_url", image_url: { url: screenshot, detail: "low" } }, + { type: "image_url", image_url: { url: png, detail: "auto" } }, + ] }); + } + } finally { + try { + await server?.stop(true); + } finally { + await upstream.stop(true); + } + } +}); + test("/v1/chat/completions non-OK upstream preserves top-level structured cyber_policy type", async () => { const secret = `blocked by upstream policy Authorization: ${["Bear", "er"].join("")} chathttpsecret123456`; const safeMessage = "blocked by upstream policy Authorization: Bearer [REDACTED]"; diff --git a/tests/responses/openai-responses-passthrough.test.ts b/tests/responses/openai-responses-passthrough.test.ts index 8f9648e951..578a0f1906 100644 --- a/tests/responses/openai-responses-passthrough.test.ts +++ b/tests/responses/openai-responses-passthrough.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "bun:test"; +import { createOpenAIChatAdapter } from "../../src/adapters/openai-chat"; import { createResponsesPassthroughAdapter as createResponsesPassthroughAdapterProduction } from "../../src/adapters/openai-responses"; import { openaiResponsesUrl } from "../../src/adapters/openai-responses-url"; +import { chatCompletionsToResponsesBody } from "../../src/chat/inbound"; +import { anthropicToResponsesBody } from "../../src/claude/inbound"; +import { parseRequest } from "../../src/responses/parser"; import { enrichProviderFromRegistry, providerConfigSeed } from "../../src/providers/derive"; import { getProviderRegistryEntry } from "../../src/providers/registry"; import { XAI_GROK_CLI_BASE_URL } from "../../src/providers/xai-transport"; @@ -25,6 +29,120 @@ const provider = { authMode: "forward" as const, }; +describe("external image wire matrix", () => { + // Same decodable 1x1 PNG as anthropic-image-normalize.test.ts; no fetch is needed. + const png = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg=="; + const dataUrl = `data:image/png;base64,${png}`; + const httpsUrl = "https://images.example/second.png"; + const keyed = { adapter: "openai-responses", baseUrl: "https://api.openai.com/v1", authMode: "key" as const, apiKey: "test-key" }; + const ingresses = [ + { + name: "Chat", convert: chatCompletionsToResponsesBody, + images: [ + { type: "image_url", image_url: { url: dataUrl, detail: "high" } }, + { type: "image_url", image_url: { url: httpsUrl, detail: "low" } }, + ], + call: { role: "assistant", tool_calls: [{ id: "call_image", type: "function", function: { name: "screenshot", arguments: "{}" } }] }, + responsesImages: [ + { type: "input_image", image_url: dataUrl, detail: "high" }, + { type: "input_image", image_url: httpsUrl, detail: "low" }, + ], + chatImages: [ + { type: "image_url", image_url: { url: dataUrl, detail: "high" } }, + { type: "image_url", image_url: { url: httpsUrl, detail: "low" } }, + ], + }, + { + name: "Claude", convert: anthropicToResponsesBody, + images: [ + { type: "image", source: { type: "base64", media_type: "image/png", data: png } }, + { type: "image", source: { type: "url", url: httpsUrl } }, + ], + call: { role: "assistant", content: [{ type: "tool_use", id: "call_image", name: "screenshot", input: {} }] }, + responsesImages: [ + { type: "input_image", image_url: dataUrl }, + { type: "input_image", image_url: httpsUrl }, + ], + chatImages: [ + { type: "image_url", image_url: { url: dataUrl } }, + { type: "image_url", image_url: { url: httpsUrl } }, + ], + }, + ]; + + for (const ingress of ingresses) { + for (const placement of ["user", "tool", "image-only tool"]) { + for (const target of ["API-key Responses", "ChatGPT forward", "Chat"]) { + test(`${ingress.name} ${placement} images -> ${target}`, async () => { + const isTool = placement !== "user"; + const imageOnly = placement === "image-only tool"; + const content = [...(imageOnly ? [] : [{ type: "text", text: "screenshot" }]), ...structuredClone(ingress.images)]; + const result = ingress.name === "Chat" + ? { role: "tool", tool_call_id: "call_image", content } + : { role: "user", content: [{ type: "tool_result", tool_use_id: "call_image", content }] }; + const raw = { + model: "test-model", stream: true, + messages: isTool + ? [structuredClone(ingress.call), result, { role: "user", content: "continue" }] + : [{ role: "user", content }], + }; + const original = structuredClone(raw); + const translated = ingress.convert(raw); + const translatedBefore = structuredClone(translated); + const parsed = parseRequest(translated); + const adapter = target === "Chat" + ? withTestTranslatorBudget(createOpenAIChatAdapter({ ...keyed, adapter: "openai-chat" })) + : createResponsesPassthroughAdapter(target === "ChatGPT forward" ? provider : keyed); + const request = await adapter.buildRequest(parsed, { headers: new Headers() }); + const body = JSON.parse(request.body) as { model: string; input?: unknown[]; messages?: unknown[] }; + expect(request.url).toBe(target === "ChatGPT forward" + ? "https://chatgpt.com/backend-api/codex/responses" + : target === "Chat" ? "https://api.openai.com/v1/chat/completions" : "https://api.openai.com/v1/responses"); + expect(body.model).toBe("test-model"); + // Expected payloads are hand-authored, never taken from translator/parser output. + if (target === "Chat") { + expect(body.messages).toEqual(isTool ? [ + { role: "assistant", content: "", tool_calls: [{ id: "call_image", type: "function", function: { name: "screenshot", arguments: "{}" } }] }, + { role: "tool", tool_call_id: "call_image", content: imageOnly ? "[image][image]" : "screenshot" }, + { role: "user", content: [{ type: "text", text: "[ocx] image output from the preceding tool result(s):" }, ...ingress.chatImages] }, + { role: "user", content: "continue" }, + ] : [{ role: "user", content: [{ type: "text", text: "screenshot" }, ...ingress.chatImages] }]); + } else { + const expectedContent = [...(imageOnly ? [] : [{ type: "input_text", text: "screenshot" }]), ...ingress.responsesImages]; + expect(body.input).toEqual(isTool ? [ + { type: "function_call", call_id: "call_image", name: "screenshot", arguments: "{}" }, + { type: "function_call_output", call_id: "call_image", output: expectedContent }, + { type: "message", role: "user", content: [{ type: "input_text", text: "continue" }] }, + ] : [{ type: "message", role: "user", content: expectedContent }]); + } + expect(raw).toEqual(original); + expect(translated).toEqual(translatedBefore); + }); + } + } + + test(`${ingress.name} orphan image-only output survives canonical forward repair`, async () => { + const content = structuredClone(ingress.images); + const raw = { model: "test-model", messages: [ingress.name === "Chat" + ? { role: "tool", tool_call_id: "call_orphan", content } + : { role: "user", content: [{ type: "tool_result", tool_use_id: "call_orphan", content }] }], + }; + const original = structuredClone(raw); + const translated = { ...ingress.convert(raw), previous_response_id: "resp_missing" }; + const translatedBefore = structuredClone(translated); + const request = await createResponsesPassthroughAdapter(provider).buildRequest(parseRequest(translated)); + const body = JSON.parse(request.body) as { previous_response_id?: string; input: unknown[] }; + expect(body.previous_response_id).toBeUndefined(); + expect(body.input).toEqual([{ + type: "message", role: "user", + content: [{ type: "input_text", text: "[tool output for call_orphan]" }, ...ingress.responsesImages], + }]); + expect(raw).toEqual(original); + expect(translated).toEqual(translatedBefore); + }); + } +}); + test("noncanonical forward providers cannot receive caller or runtime credentials", () => { const userInfoUrl = new URL("https://chatgpt.com/backend-api/codex"); userInfoUrl.username = "user"; From 5060ac8910eff1877bd2ee6bdcb2b6d063f26b79 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 12:24:06 +0900 Subject: [PATCH 2/2] test(state): isolate late-spill ordering from wall-clock reserve --- .../060_delivery.md | 39 +++++++++++++++++++ tests/responses/responses-state.test.ts | 9 ++++- 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 devlog/_plan/260905_external_image_roundtrip/060_delivery.md diff --git a/devlog/_plan/260905_external_image_roundtrip/060_delivery.md b/devlog/_plan/260905_external_image_roundtrip/060_delivery.md new file mode 100644 index 0000000000..79ad1d9e47 --- /dev/null +++ b/devlog/_plan/260905_external_image_roundtrip/060_delivery.md @@ -0,0 +1,39 @@ +# Full-format closeout + +Depends on030/040/050; wp6. No new runtime features. Main owns all external writes. + +## Diagnosed CI prerequisite (test-only, no production storage changes) + +PR3589 job101236091166 on7783355f9 fails the late-async-spill ordering test before +its overwrite assertions: outer fallback uses real Date.now despite frozen inner +clocks, so its80ms reserve can expire during real cleanup. Images aren't in this +isolated batch. Do not blindly retry or weaken budgets/assertions. + +MODIFY only tests/responses/responses-state.test.ts: import existing spyOn and +awaitResponseSpillPublicationTailForTests; after `await started` in this single +late-completion test, capture Date.now and spy it to return that captured value. +Keep all assertions and40ms real drain timer. In finally restore spy FIRST, then +release the blocked writer and await the existing publication-tail barrier. +Do not freeze timers or other deadline/exhaustion tests. Existing superseded flag, +file-identity and replay assertions prove ordering independently of clock progression. +Publish correction on layer2, cascade all own higher branches with explicit leases, +and re-run exact-head CI; no new production clock hook/export. Independent reviewer +must verify scope and teardown. The earlier failed CI is the red evidence. + +MODIFY003 audit table with each exact final disposition, test names and CI links; +MODIFY000 continuity with exact commit/PR/reviewer proof. Archive unit _plan -> _fin +only when it describes a public outcome. Tests/code may not be weakened for green CI. + +Before each merge: refresh exact head, base, full status rollup, reviewer comments, +worktree identity and source ancestry. Resolve actual failures; never assume flakes. +Document user-authorized admin approval bypass. Merge bottom-up, prefer merge commits, +retain parent branches, retarget child to dev only after parent is public. Verify CI +against the exact child head and current base; restack with lease if necessary. Fetch +origin/dev and prove every merge SHA ancestor. No release, deployment or10100 restart. + +Local suites remain prohibited. Inspect and stop actual local Bun suite processes as +authorized, not SSH commands merely mentioning a remote suite, dev servers or the proxy. +Success: c-all fully accounted + unchanged c2 CI/review/merge/ancestry criterion met. +Report remaining native file/remote URL/history limitations honestly, separate from +fixed silent losses. Report original ordinary-image OCR mismatch unproven if no new +evidence establishes its cause. Do not equate model tokens or a mock reply with OCR. diff --git a/tests/responses/responses-state.test.ts b/tests/responses/responses-state.test.ts index e4ae716a41..831a8df29c 100644 --- a/tests/responses/responses-state.test.ts +++ b/tests/responses/responses-state.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { BULK_DURABLE_IO_BUDGET_MS } from "../helpers/test-budget"; import { findDeadPid } from "../helpers/dead-pid"; import { @@ -30,6 +30,7 @@ import { evictOldestResponseContinuationForBudget, expandPreviousResponseInput, flushResponseState, + awaitResponseSpillPublicationTailForTests, markBodyNonPersistable, previousResponseConversationId, previousResponseProviderState, @@ -1513,13 +1514,19 @@ describe("Responses previous_response_id state", () => { let fallbackFile: string | undefined; let abandonedTempCount = -1; + // This checks late-write ordering, not elapsed time in the outer fallback reserve. + // Keep the real drain timer, but exclude unrelated cleanup wall time from that reserve. + const frozenNow = Date.now(); + const nowSpy = spyOn(Date, "now").mockReturnValue(frozenNow); try { await flushResponseState(); fallbackFile = spillFileNames(home)[0]; expect(fallbackFile).toBeDefined(); abandonedTempCount = spillTempNames(home).length; } finally { + nowSpy.mockRestore(); release(); + await awaitResponseSpillPublicationTailForTests(); } await hardened; await new Promise(resolve => setTimeout(resolve, 0));