fix(core): preserve request fidelity and provider errors
This commit is contained in:
Patrick Wozniak
@@ -0,0 +1,196 @@
|
||||
import assert from "node:assert/strict"
|
||||
import { after, before, beforeEach, describe, it } from "node:test"
|
||||
|
||||
import {
|
||||
commandCodeErrorMessage,
|
||||
normalizeCommandCodeErrorMessage,
|
||||
normalizeCommandCodeMessage,
|
||||
} from "../src/overflow.ts"
|
||||
import {
|
||||
collectEvents,
|
||||
createTestDeps,
|
||||
makeContext,
|
||||
makeModel,
|
||||
startMockCommandCodeServer,
|
||||
type MockCommandCodeServer,
|
||||
} from "./helpers.ts"
|
||||
|
||||
let server: MockCommandCodeServer
|
||||
|
||||
before(async () => {
|
||||
server = await startMockCommandCodeServer()
|
||||
})
|
||||
|
||||
after(async () => {
|
||||
await server.close()
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
server.reset()
|
||||
})
|
||||
|
||||
describe("Command Code overflow normalization", () => {
|
||||
it("normalizes Command Code context errors to pi's generic overflow marker", () => {
|
||||
const normalized = normalizeCommandCodeErrorMessage("Prompt token limit exceeded")
|
||||
|
||||
assert.equal(normalized, "context_length_exceeded: Prompt token limit exceeded")
|
||||
})
|
||||
|
||||
it("is idempotent and leaves unrelated, rate-limit, and capacity errors unchanged", () => {
|
||||
assert.equal(
|
||||
normalizeCommandCodeErrorMessage("context_length_exceeded: Prompt token limit exceeded"),
|
||||
undefined,
|
||||
)
|
||||
assert.equal(normalizeCommandCodeErrorMessage("OpenAI request failed"), undefined)
|
||||
assert.equal(
|
||||
normalizeCommandCodeErrorMessage("Prompt token limit exceeded due to rate limit"),
|
||||
undefined,
|
||||
)
|
||||
assert.equal(
|
||||
normalizeCommandCodeErrorMessage("Command Code API error 429: context window exceeded"),
|
||||
undefined,
|
||||
)
|
||||
assert.equal(
|
||||
normalizeCommandCodeErrorMessage("context window exceeded: status: 429"),
|
||||
undefined,
|
||||
)
|
||||
assert.equal(
|
||||
normalizeCommandCodeErrorMessage("The input is too long"),
|
||||
"context_length_exceeded: The input is too long",
|
||||
)
|
||||
assert.equal(
|
||||
normalizeCommandCodeErrorMessage("Input exceeds context limit"),
|
||||
"context_length_exceeded: Input exceeds context limit",
|
||||
)
|
||||
assert.equal(
|
||||
normalizeCommandCodeErrorMessage("Context window exceeded: provider capacity reached"),
|
||||
undefined,
|
||||
)
|
||||
})
|
||||
|
||||
it("scopes finalized message normalization to Command Code", () => {
|
||||
const message = {
|
||||
role: "assistant" as const,
|
||||
provider: "commandcode",
|
||||
stopReason: "error" as const,
|
||||
errorMessage: "model context window exceeded",
|
||||
}
|
||||
|
||||
const normalized = normalizeCommandCodeMessage(message)
|
||||
assert.equal(
|
||||
normalized?.message.errorMessage,
|
||||
"context_length_exceeded: model context window exceeded",
|
||||
)
|
||||
assert.equal(normalizeCommandCodeMessage({ ...message, provider: "openai" }), undefined)
|
||||
assert.equal(normalizeCommandCodeMessage({ ...message, stopReason: "stop" }), undefined)
|
||||
})
|
||||
|
||||
it("extracts nested stream error messages without exposing credentials", () => {
|
||||
assert.equal(
|
||||
commandCodeErrorMessage({
|
||||
error: { details: { errorMessage: "context window exceeded" } },
|
||||
}),
|
||||
"context window exceeded",
|
||||
)
|
||||
})
|
||||
|
||||
it("redacts secrets from finalized provider errors", async () => {
|
||||
server.mockResponse({
|
||||
type: "error",
|
||||
status: 400,
|
||||
body: "api_key=user_secret_value",
|
||||
})
|
||||
const { streamCommandCode } = createTestDeps({ apiBase: server.baseUrl() })
|
||||
|
||||
const events = await collectEvents(
|
||||
streamCommandCode(makeModel(), makeContext(), { apiKey: "mock-key" }),
|
||||
)
|
||||
const error = events.at(-1)
|
||||
assert.equal(error?.type, "error")
|
||||
if (error?.type !== "error") throw new Error("expected error")
|
||||
assert.doesNotMatch(error.error.errorMessage ?? "", /user_secret_value/)
|
||||
assert.match(error.error.errorMessage ?? "", /api_key=\[redacted\]/)
|
||||
})
|
||||
|
||||
it("normalizes HTTP error bodies containing nested context errors", async () => {
|
||||
server.mockResponse({
|
||||
type: "error",
|
||||
status: 400,
|
||||
body: JSON.stringify({ error: { message: "Prompt token limit exceeded" } }),
|
||||
})
|
||||
const { streamCommandCode } = createTestDeps({ apiBase: server.baseUrl() })
|
||||
|
||||
const events = await collectEvents(
|
||||
streamCommandCode(makeModel(), makeContext(), { apiKey: "mock-key" }),
|
||||
)
|
||||
const error = events.at(-1)
|
||||
|
||||
assert.equal(error?.type, "error")
|
||||
if (error?.type !== "error") throw new Error("expected error")
|
||||
const normalized = normalizeCommandCodeMessage(error.error)
|
||||
assert.match(normalized?.message.errorMessage ?? "", /^context_length_exceeded:/)
|
||||
})
|
||||
|
||||
it("does not normalize an HTTP rate-limit response that mentions context", async () => {
|
||||
server.mockResponse({
|
||||
type: "error",
|
||||
status: 429,
|
||||
body: JSON.stringify({ error: { message: "context window exceeded" } }),
|
||||
})
|
||||
const { streamCommandCode } = createTestDeps({ apiBase: server.baseUrl() })
|
||||
|
||||
const events = await collectEvents(
|
||||
streamCommandCode(makeModel(), makeContext(), { apiKey: "mock-key" }),
|
||||
)
|
||||
const error = events.at(-1)
|
||||
|
||||
assert.equal(error?.type, "error")
|
||||
if (error?.type !== "error") throw new Error("expected error")
|
||||
assert.equal(normalizeCommandCodeMessage(error.error), undefined)
|
||||
})
|
||||
|
||||
it("normalizes nested stream error events", async () => {
|
||||
server.mockResponse({
|
||||
type: "success",
|
||||
events: [
|
||||
JSON.stringify({
|
||||
type: "error",
|
||||
error: { details: { message: "model context window exceeded" } },
|
||||
}),
|
||||
],
|
||||
})
|
||||
const { streamCommandCode } = createTestDeps({ apiBase: server.baseUrl() })
|
||||
|
||||
const events = await collectEvents(
|
||||
streamCommandCode(makeModel(), makeContext(), { apiKey: "mock-key" }),
|
||||
)
|
||||
const error = events.at(-1)
|
||||
|
||||
assert.equal(error?.type, "error")
|
||||
if (error?.type !== "error") throw new Error("expected error")
|
||||
const normalized = normalizeCommandCodeMessage(error.error)
|
||||
assert.equal(
|
||||
normalized?.message.errorMessage,
|
||||
"context_length_exceeded: model context window exceeded",
|
||||
)
|
||||
|
||||
server.mockResponse({
|
||||
type: "success",
|
||||
events: [
|
||||
JSON.stringify({
|
||||
type: "error",
|
||||
error: { message: "context window exceeded", status: 429 },
|
||||
}),
|
||||
],
|
||||
})
|
||||
const retryEvents = await collectEvents(
|
||||
createTestDeps({ apiBase: server.baseUrl() }).streamCommandCode(makeModel(), makeContext(), {
|
||||
apiKey: "mock-key",
|
||||
}),
|
||||
)
|
||||
const retryError = retryEvents.at(-1)
|
||||
assert.equal(retryError?.type, "error")
|
||||
if (retryError?.type !== "error") throw new Error("expected error")
|
||||
assert.equal(normalizeCommandCodeMessage(retryError.error), undefined)
|
||||
})
|
||||
})
|
||||
@@ -10,6 +10,8 @@ import { join } from "node:path"
|
||||
import { describe, it } from "node:test"
|
||||
|
||||
import {
|
||||
assertTextOnlyMessages,
|
||||
COMMAND_CODE_INPUT_TYPES,
|
||||
getApiKey,
|
||||
getEnvironmentInfo,
|
||||
mapFinishReason,
|
||||
@@ -20,6 +22,7 @@ import {
|
||||
toJsonSchema,
|
||||
toolsToJson,
|
||||
} from "../src/core.ts"
|
||||
import { redactCommandCodeErrorText } from "../src/overflow.ts"
|
||||
|
||||
import { objectAt } from "./helpers.ts"
|
||||
|
||||
@@ -90,6 +93,20 @@ describe("getApiKey()", () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe("error redaction", () => {
|
||||
it("redacts bearer, credential, and query-string secrets", () => {
|
||||
const redacted = redactCommandCodeErrorText(
|
||||
"Bearer user_secret_value api_key=user_secret_value https://example.test/x?token=user_secret_value",
|
||||
)
|
||||
assert.doesNotMatch(redacted, /user_secret_value/)
|
||||
assert.match(redacted, /Bearer \[redacted\]/)
|
||||
assert.doesNotMatch(
|
||||
redactCommandCodeErrorText("provider returned sk-test-secret-value-1234567890"),
|
||||
/sk-test-secret-value/,
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe("projectSlugFromPath()", () => {
|
||||
it("matches the official CLI-style slug from an absolute working directory", () => {
|
||||
assert.equal(
|
||||
@@ -100,13 +117,42 @@ describe("projectSlugFromPath()", () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe("text-only image handling", () => {
|
||||
it("does not advertise image input capability", () => {
|
||||
assert.deepEqual(COMMAND_CODE_INPUT_TYPES, ["text"])
|
||||
})
|
||||
|
||||
it("rejects image content instead of dropping it", () => {
|
||||
assert.throws(
|
||||
() =>
|
||||
assertTextOnlyMessages([
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "image", data: "base64-data", mimeType: "image/png" }],
|
||||
},
|
||||
]),
|
||||
/does not support image content.*refusing to send it/i,
|
||||
)
|
||||
assert.throws(
|
||||
() =>
|
||||
assertTextOnlyMessages([
|
||||
{
|
||||
role: "toolResult",
|
||||
toolCallId: "c1",
|
||||
content: [{ type: "image", data: "base64-data", mimeType: "image/png" }],
|
||||
},
|
||||
]),
|
||||
/does not support image content.*refusing to send it/i,
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe("textContent()", () => {
|
||||
it("extracts and joins text blocks", () => {
|
||||
assert.equal(
|
||||
textContent({
|
||||
content: [
|
||||
{ type: "text", text: "hello" },
|
||||
{ type: "image", data: "x" },
|
||||
{ type: "text", text: "world" },
|
||||
],
|
||||
}),
|
||||
@@ -114,6 +160,20 @@ describe("textContent()", () => {
|
||||
)
|
||||
})
|
||||
|
||||
it("rejects mixed text and image content instead of dropping the image", () => {
|
||||
assert.throws(
|
||||
() =>
|
||||
textContent({
|
||||
content: [
|
||||
{ type: "text", text: "hello" },
|
||||
{ type: "image", data: "x", mimeType: "image/png" },
|
||||
{ type: "text", text: "world" },
|
||||
],
|
||||
}),
|
||||
/does not support image content.*refusing to send it/i,
|
||||
)
|
||||
})
|
||||
|
||||
it("handles empty or missing content", () => {
|
||||
assert.equal(textContent({ content: [] }), "")
|
||||
assert.equal(textContent({}), "")
|
||||
@@ -177,6 +237,201 @@ describe("toJsonSchema()", () => {
|
||||
)
|
||||
assert.deepEqual(toJsonSchema(undefined), {})
|
||||
assert.deepEqual(toJsonSchema({ kind: "wat" }), {})
|
||||
assert.deepEqual(toJsonSchema({ type: "wat", description: "not a schema" }), {})
|
||||
assert.deepEqual(toJsonSchema({}), {})
|
||||
assert.equal(toJsonSchema(true), true)
|
||||
})
|
||||
|
||||
it("preserves complete JSON Schema metadata and nested schemas", () => {
|
||||
assert.deepEqual(
|
||||
toJsonSchema({
|
||||
type: "object",
|
||||
description: "Search options",
|
||||
properties: {
|
||||
query: {
|
||||
type: "string",
|
||||
description: "Text to search for",
|
||||
minLength: 2,
|
||||
maxLength: 50,
|
||||
pattern: "^[a-z]+$",
|
||||
default: "pi",
|
||||
},
|
||||
limit: {
|
||||
type: "integer",
|
||||
minimum: 1,
|
||||
maximum: 100,
|
||||
exclusiveMinimum: 0,
|
||||
multipleOf: 1,
|
||||
default: 10,
|
||||
},
|
||||
tags: {
|
||||
type: "array",
|
||||
minItems: 1,
|
||||
maxItems: 3,
|
||||
uniqueItems: true,
|
||||
items: {
|
||||
type: "object",
|
||||
properties: { name: { type: "string" } },
|
||||
required: ["name"],
|
||||
additionalProperties: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
required: ["query", "limit"],
|
||||
additionalProperties: false,
|
||||
}),
|
||||
{
|
||||
type: "object",
|
||||
description: "Search options",
|
||||
properties: {
|
||||
query: {
|
||||
type: "string",
|
||||
description: "Text to search for",
|
||||
minLength: 2,
|
||||
maxLength: 50,
|
||||
pattern: "^[a-z]+$",
|
||||
default: "pi",
|
||||
},
|
||||
limit: {
|
||||
type: "integer",
|
||||
minimum: 1,
|
||||
maximum: 100,
|
||||
exclusiveMinimum: 0,
|
||||
multipleOf: 1,
|
||||
default: 10,
|
||||
},
|
||||
tags: {
|
||||
type: "array",
|
||||
minItems: 1,
|
||||
maxItems: 3,
|
||||
uniqueItems: true,
|
||||
items: {
|
||||
type: "object",
|
||||
properties: { name: { type: "string" } },
|
||||
required: ["name"],
|
||||
additionalProperties: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
required: ["query", "limit"],
|
||||
additionalProperties: false,
|
||||
},
|
||||
)
|
||||
})
|
||||
|
||||
it("preserves JSON Schema composition and nullable forms", () => {
|
||||
assert.deepEqual(
|
||||
toJsonSchema({
|
||||
anyOf: [{ type: "string" }, { type: "number" }],
|
||||
oneOf: [{ const: "a" }, { const: "b" }],
|
||||
allOf: [{ minLength: 1 }, { maxLength: 10 }],
|
||||
nullable: true,
|
||||
}),
|
||||
{
|
||||
anyOf: [{ type: "string" }, { type: "number" }, { type: "null" }],
|
||||
oneOf: [{ const: "a" }, { const: "b" }],
|
||||
allOf: [{ minLength: 1 }, { maxLength: 10 }],
|
||||
},
|
||||
)
|
||||
assert.deepEqual(toJsonSchema({ type: ["string", "null"] }), {
|
||||
type: ["string", "null"],
|
||||
})
|
||||
assert.deepEqual(toJsonSchema({ type: "string", nullable: true }), {
|
||||
type: ["string", "null"],
|
||||
})
|
||||
})
|
||||
|
||||
it("preserves dangerous schema property names", () => {
|
||||
const inputProperties: Record<string, unknown> = {
|
||||
constructor: { type: "number" },
|
||||
}
|
||||
Object.defineProperty(inputProperties, "__proto__", {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
value: { type: "string" },
|
||||
writable: true,
|
||||
})
|
||||
const schema = toJsonSchema({
|
||||
type: "object",
|
||||
properties: inputProperties,
|
||||
required: ["__proto__", "constructor"],
|
||||
})
|
||||
assert.ok(schema && typeof schema === "object" && !Array.isArray(schema))
|
||||
if (!schema || typeof schema !== "object" || Array.isArray(schema)) {
|
||||
throw new Error("expected object schema")
|
||||
}
|
||||
const outputProperties: unknown = Object.getOwnPropertyDescriptor(schema, "properties")?.value
|
||||
assert.ok(outputProperties && typeof outputProperties === "object")
|
||||
if (!outputProperties || typeof outputProperties !== "object") {
|
||||
throw new Error("expected object properties")
|
||||
}
|
||||
assert.ok(Object.prototype.hasOwnProperty.call(outputProperties, "__proto__"))
|
||||
assert.deepEqual(Object.getOwnPropertyDescriptor(outputProperties, "__proto__")?.value, {
|
||||
type: "string",
|
||||
})
|
||||
assert.deepEqual(Object.getOwnPropertyDescriptor(outputProperties, "constructor")?.value, {
|
||||
type: "number",
|
||||
})
|
||||
})
|
||||
|
||||
it("converts legacy shapes without collapsing unions", () => {
|
||||
assert.deepEqual(
|
||||
toJsonSchema({
|
||||
kind: "Object",
|
||||
description: "Legacy options",
|
||||
properties: {
|
||||
mode: {
|
||||
kind: "union",
|
||||
variants: [
|
||||
{ kind: "string", enum: ["fast", "safe"] },
|
||||
{ kind: "string", enum: ["debug"] },
|
||||
],
|
||||
},
|
||||
count: { kind: "Number", minimum: 1, optional: true },
|
||||
nested: {
|
||||
kind: "Array",
|
||||
element: { kind: "object", properties: { value: { kind: "boolean" } } },
|
||||
},
|
||||
},
|
||||
optional: ["count"],
|
||||
additionalProperties: false,
|
||||
}),
|
||||
{
|
||||
type: "object",
|
||||
description: "Legacy options",
|
||||
properties: {
|
||||
mode: {
|
||||
anyOf: [
|
||||
{ type: "string", enum: ["fast", "safe"] },
|
||||
{ type: "string", enum: ["debug"] },
|
||||
],
|
||||
},
|
||||
count: { type: "number", minimum: 1 },
|
||||
nested: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "object",
|
||||
properties: { value: { type: "boolean" } },
|
||||
required: ["value"],
|
||||
},
|
||||
},
|
||||
},
|
||||
required: ["mode", "nested"],
|
||||
additionalProperties: false,
|
||||
},
|
||||
)
|
||||
assert.deepEqual(
|
||||
toJsonSchema({
|
||||
kind: "intersect",
|
||||
variants: [{ kind: "object", properties: { a: { kind: "string" } } }, { kind: "number" }],
|
||||
}),
|
||||
{
|
||||
allOf: [
|
||||
{ type: "object", properties: { a: { type: "string" } }, required: ["a"] },
|
||||
{ type: "number" },
|
||||
],
|
||||
},
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -354,6 +354,34 @@ describe("streamCommandCode — successful streams", () => {
|
||||
})
|
||||
|
||||
describe("streamCommandCode — request serialization", () => {
|
||||
it("rejects image input before sending a lossy request", async () => {
|
||||
const { streamCommandCode } = createTestDeps({ apiBase: server.baseUrl() })
|
||||
const events = await collectEvents(
|
||||
streamCommandCode(
|
||||
makeModel(),
|
||||
makeContext({
|
||||
messages: [
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "image", data: "base64-data", mimeType: "image/png" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
{ apiKey: "mock-key" },
|
||||
),
|
||||
)
|
||||
|
||||
assert.deepEqual(eventTypes(events), ["start", "error"])
|
||||
const lastEvent = events.at(-1)
|
||||
assert.equal(lastEvent?.type, "error")
|
||||
if (lastEvent?.type === "error") {
|
||||
assert.match(
|
||||
lastEvent.error.errorMessage ?? "",
|
||||
/does not support image content.*refusing to send it/i,
|
||||
)
|
||||
}
|
||||
assert.equal(server.requestCount(), 0)
|
||||
})
|
||||
it("sends the expected request body and default headers", async () => {
|
||||
server.mockResponse({
|
||||
type: "success",
|
||||
@@ -394,6 +422,7 @@ describe("streamCommandCode — request serialization", () => {
|
||||
assert.equal(objectAt(body, ["params", "model"]), "deepseek/deepseek-v4-flash")
|
||||
assert.equal(objectAt(body, ["params", "stream"]), true)
|
||||
assert.equal(objectAt(body, ["params", "max_tokens"]), 500)
|
||||
assert.equal(objectAt(body, ["params", "reasoning_effort"]), undefined)
|
||||
assert.equal(objectAt(body, ["params", "temperature"]), 0.3)
|
||||
assert.equal(objectAt(body, ["params", "system"]), "You are a test assistant.")
|
||||
assert.equal(objectAt(body, ["memory"]), null)
|
||||
@@ -552,6 +581,51 @@ describe("streamCommandCode — request serialization", () => {
|
||||
)
|
||||
})
|
||||
|
||||
it("times out a hung onResponse callback", async () => {
|
||||
server.mockResponse({
|
||||
type: "success",
|
||||
events: [JSON.stringify({ type: "finish", finishReason: "stop" })],
|
||||
})
|
||||
const { streamCommandCode } = createTestDeps({ apiBase: server.baseUrl() })
|
||||
const started = Date.now()
|
||||
const events = await collectEvents(
|
||||
streamCommandCode(makeModel(), makeContext(), {
|
||||
apiKey: "mock-key",
|
||||
timeoutMs: 25,
|
||||
onResponse: async () => new Promise<void>(() => {}),
|
||||
}),
|
||||
1_000,
|
||||
)
|
||||
|
||||
assert.ok(Date.now() - started < 500)
|
||||
assert.deepEqual(eventTypes(events), ["start", "error"])
|
||||
const error = events.at(-1)
|
||||
assert.equal(error?.type, "error")
|
||||
if (error?.type !== "error") throw new Error("expected error")
|
||||
assert.match(error.error.errorMessage ?? "", /timed out after 25ms/)
|
||||
})
|
||||
|
||||
it("times out a hung onPayload callback", async () => {
|
||||
const { streamCommandCode } = createTestDeps({ apiBase: server.baseUrl() })
|
||||
const started = Date.now()
|
||||
const events = await collectEvents(
|
||||
streamCommandCode(makeModel(), makeContext(), {
|
||||
apiKey: "mock-key",
|
||||
timeoutMs: 25,
|
||||
onPayload: async () => new Promise<unknown>(() => {}),
|
||||
}),
|
||||
1_000,
|
||||
)
|
||||
|
||||
assert.ok(Date.now() - started < 500)
|
||||
assert.deepEqual(eventTypes(events), ["start", "error"])
|
||||
const error = events.at(-1)
|
||||
assert.equal(error?.type, "error")
|
||||
if (error?.type !== "error") throw new Error("expected error")
|
||||
assert.match(error.error.errorMessage ?? "", /timed out after 25ms/)
|
||||
assert.equal(server.requestCount(), 0)
|
||||
})
|
||||
|
||||
it("runs onPayload and onResponse hooks", async () => {
|
||||
server.mockResponse({
|
||||
type: "success",
|
||||
|
||||
Reference in New Issue
Block a user