mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 22:50:51 +00:00
merge(FIX-D): v1.20.2 audit fixes
This commit is contained in:
commit
9c3b119695
9 files changed
+276
-16
No files matched your search
@@ -567,6 +567,10 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}. Fetch
|
||||
user: userPrompt,
|
||||
temperature: AI_BUDGETS.coach.temperature,
|
||||
maxTokens: AI_BUDGETS.coach.maxTokens,
|
||||
// v1.20.1 — thread the request's abort signal so a mid-generation
|
||||
// client disconnect tears the upstream provider call down instead of
|
||||
// paying the full token cost into a closed connection.
|
||||
signal: request.signal,
|
||||
}),
|
||||
});
|
||||
result = fallback.result;
|
||||
|
||||
@@ -290,4 +290,72 @@ describe("OpenAIClient", () => {
|
||||
image_url: { url: "data:image/png;base64,AAAA" },
|
||||
});
|
||||
});
|
||||
|
||||
it("tags an empty-content reply with sentinel httpStatus 0 + kind for the chain classifier", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: () =>
|
||||
Promise.resolve({
|
||||
choices: [{ message: { content: "" } }],
|
||||
usage: { total_tokens: 5 },
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
const client = new OpenAIClient({
|
||||
apiKey: "sk-test",
|
||||
model: "gpt-4o-mini",
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
});
|
||||
|
||||
await client
|
||||
.generateCompletion(singleUserTurn({ system: "s", user: "u" }))
|
||||
.then(
|
||||
() => {
|
||||
throw new Error("expected an empty-content throw");
|
||||
},
|
||||
(err: unknown) => {
|
||||
// The throw distinguishes an empty 200-OK reply from ECONNRESET:
|
||||
// sentinel httpStatus 0 (still classified as a hard failure, so the
|
||||
// cascade is unchanged) + a `kind` discriminator for observability.
|
||||
expect((err as Error).message).toContain("empty content");
|
||||
expect((err as { httpStatus?: number }).httpStatus).toBe(0);
|
||||
expect((err as { kind?: string }).kind).toBe("empty_response");
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("threads the caller's abort signal onto the upstream fetch", async () => {
|
||||
const mockFetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: () =>
|
||||
Promise.resolve({
|
||||
choices: [{ message: { content: "ok" } }],
|
||||
usage: { total_tokens: 1 },
|
||||
}),
|
||||
});
|
||||
vi.stubGlobal("fetch", mockFetch);
|
||||
|
||||
const client = new OpenAIClient({
|
||||
apiKey: "sk-test",
|
||||
model: "gpt-4o-mini",
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
});
|
||||
|
||||
const ctrl = new AbortController();
|
||||
await client.generateCompletion(
|
||||
singleUserTurn({ system: "s", user: "u", signal: ctrl.signal }),
|
||||
);
|
||||
|
||||
// safeFetch composes the caller signal with the timeout via AbortSignal.any,
|
||||
// so a signal lands on the dispatched fetch init.
|
||||
const init = mockFetch.mock.calls[0][1];
|
||||
expect(init.signal).toBeInstanceOf(AbortSignal);
|
||||
|
||||
// Aborting the caller's controller propagates to the composed signal.
|
||||
ctrl.abort();
|
||||
expect(init.signal.aborted).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -259,7 +259,9 @@ export class AnthropicClient implements AIProvider {
|
||||
// 60 s ceiling — see openai-client.ts for the rationale.
|
||||
// v1.11.2 — base URL is user/admin-overridable; pin the connect-time DNS
|
||||
// check so a private/metadata address is rejected (SSRF/rebinding).
|
||||
{ timeoutMs: 60_000, requirePublicHost: true },
|
||||
// v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so
|
||||
// a mid-generation abort tears the upstream call down early.
|
||||
{ timeoutMs: 60_000, requirePublicHost: true, signal: params.signal },
|
||||
);
|
||||
|
||||
if (!res.ok) {
|
||||
@@ -318,7 +320,16 @@ export class AnthropicClient implements AIProvider {
|
||||
// A tool_use-only reply carries no text — that is valid (F1). Only an empty
|
||||
// reply with neither text NOR a tool call is an error.
|
||||
if (!rawText && !toolCalls) {
|
||||
throw new Error("Anthropic returned empty content");
|
||||
// v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell
|
||||
// an empty 200-OK reply apart from a transport failure. Cascade unchanged
|
||||
// (`status <= 0` is already a hard failure).
|
||||
const err = new Error("Anthropic returned empty content");
|
||||
Object.assign(err, {
|
||||
httpStatus: 0,
|
||||
kind: "empty_response",
|
||||
upstream: "anthropic",
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
// When we prefilled the assistant turn with `{`, the model continues
|
||||
|
||||
@@ -467,7 +467,9 @@ export class CodexClient implements AIProvider {
|
||||
// clients use so a long generation is not clipped by the 15 s
|
||||
// default while still bounding a tar-pit upstream.
|
||||
},
|
||||
{ timeoutMs: 60_000 },
|
||||
// v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so
|
||||
// a mid-generation abort tears the upstream call down early.
|
||||
{ timeoutMs: 60_000, signal: params.signal },
|
||||
);
|
||||
}
|
||||
|
||||
@@ -492,7 +494,15 @@ export class CodexClient implements AIProvider {
|
||||
requestedSlug: string,
|
||||
): Promise<CompletionResult> {
|
||||
if (!res.body) {
|
||||
throw new Error("Codex returned no response body");
|
||||
// v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell
|
||||
// a stream-level failure apart from a connect-time transport error.
|
||||
const err = new Error("Codex returned no response body");
|
||||
Object.assign(err, {
|
||||
httpStatus: 0,
|
||||
kind: "stream_failed",
|
||||
upstream: "codex",
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
const reader = res.body.getReader();
|
||||
@@ -595,6 +605,10 @@ export class CodexClient implements AIProvider {
|
||||
const e = new Error(message);
|
||||
Object.assign(e, {
|
||||
upstream: "codex",
|
||||
// v1.20.1 — sentinel httpStatus + kind so the chain classifier
|
||||
// distinguishes a stream-level failure from a transport error.
|
||||
httpStatus: 0,
|
||||
kind: "stream_failed",
|
||||
errorCode: err?.code ?? null,
|
||||
model: serverModel ?? requestedSlug,
|
||||
});
|
||||
@@ -604,13 +618,25 @@ export class CodexClient implements AIProvider {
|
||||
const reason =
|
||||
parsed.response?.incomplete_details?.reason ??
|
||||
"incomplete response";
|
||||
throw new Error(`Codex stream incomplete: ${reason}`);
|
||||
const e = new Error(`Codex stream incomplete: ${reason}`);
|
||||
Object.assign(e, {
|
||||
httpStatus: 0,
|
||||
kind: "stream_failed",
|
||||
upstream: "codex",
|
||||
});
|
||||
throw e;
|
||||
}
|
||||
case "error":
|
||||
case "response.error": {
|
||||
const message =
|
||||
parsed.error?.message ?? "Codex stream returned an error event";
|
||||
throw new Error(message);
|
||||
const e = new Error(message);
|
||||
Object.assign(e, {
|
||||
httpStatus: 0,
|
||||
kind: "stream_failed",
|
||||
upstream: "codex",
|
||||
});
|
||||
throw e;
|
||||
}
|
||||
default:
|
||||
// Unknown event types are tolerated and ignored, same
|
||||
@@ -627,7 +653,16 @@ export class CodexClient implements AIProvider {
|
||||
// A function-call-only reply carries no text — valid (F1). Only an empty
|
||||
// reply with neither text NOR a tool call is an error.
|
||||
if (!content && toolCalls.length === 0) {
|
||||
throw new Error("Codex returned empty content");
|
||||
// v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell
|
||||
// an empty reply apart from a transport failure. Cascade unchanged
|
||||
// (`status <= 0` is already a hard failure).
|
||||
const err = new Error("Codex returned empty content");
|
||||
Object.assign(err, {
|
||||
httpStatus: 0,
|
||||
kind: "empty_response",
|
||||
upstream: "codex",
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -125,7 +125,13 @@ export class LocalOpenAICompatibleClient implements AIProvider {
|
||||
...(params.responseFormat === "json" ? { format: "json" } : {}),
|
||||
}),
|
||||
},
|
||||
{ timeoutMs: 60_000, requirePublicHost: !allowPrivate },
|
||||
// v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so
|
||||
// a mid-generation abort tears the upstream call down early.
|
||||
{
|
||||
timeoutMs: 60_000,
|
||||
requirePublicHost: !allowPrivate,
|
||||
signal: params.signal,
|
||||
},
|
||||
);
|
||||
|
||||
if (!res.ok) {
|
||||
@@ -155,7 +161,16 @@ export class LocalOpenAICompatibleClient implements AIProvider {
|
||||
|
||||
const content = json.choices?.[0]?.message?.content;
|
||||
if (!content) {
|
||||
throw new Error("Local AI returned empty content");
|
||||
// v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell
|
||||
// an empty 200-OK reply apart from a transport failure. Cascade unchanged
|
||||
// (`status <= 0` is already a hard failure).
|
||||
const err = new Error("Local AI returned empty content");
|
||||
Object.assign(err, {
|
||||
httpStatus: 0,
|
||||
kind: "empty_response",
|
||||
upstream: "local",
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -90,7 +90,9 @@ export class OpenAIClient implements AIProvider {
|
||||
// v1.11.2 — the base URL is user/admin-overridable (BYO gateway), so pin
|
||||
// the connect-time DNS check: a base URL resolving to a private/metadata
|
||||
// address is rejected, closing the SSRF/rebinding surface.
|
||||
{ timeoutMs: 60_000, requirePublicHost: true },
|
||||
// v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so
|
||||
// a mid-generation abort tears the upstream call down early.
|
||||
{ timeoutMs: 60_000, requirePublicHost: true, signal: params.signal },
|
||||
);
|
||||
|
||||
if (!res.ok) {
|
||||
@@ -124,7 +126,19 @@ export class OpenAIClient implements AIProvider {
|
||||
// A reply with tool calls and no prose is valid (F1 tool loop); only an
|
||||
// empty reply with neither content NOR tool calls is an error.
|
||||
if (!content && !toolCalls) {
|
||||
throw new Error("OpenAI returned empty content");
|
||||
// v1.20.1 — tag the empty-reply throw with a sentinel `httpStatus: 0` +
|
||||
// `kind` so the provider-chain classifier can tell an empty 200-OK reply
|
||||
// apart from a genuine transport failure (ECONNRESET, DNS, timeout) in
|
||||
// observability. The cascade is unchanged: `isHardProviderFailure`
|
||||
// already treats `status <= 0` as a hard failure and walks to the next
|
||||
// provider.
|
||||
const err = new Error("OpenAI returned empty content");
|
||||
Object.assign(err, {
|
||||
httpStatus: 0,
|
||||
kind: "empty_response",
|
||||
upstream: "openai",
|
||||
});
|
||||
throw err;
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -271,6 +271,15 @@ export interface CompletionParams {
|
||||
* Mapped per provider; omitted from the wire when unset.
|
||||
*/
|
||||
toolChoice?: "auto" | "none";
|
||||
/**
|
||||
* v1.20.1 — optional caller-owned cancellation signal. Threaded onto the
|
||||
* client's `safeFetch` so a mid-generation client disconnect (the Coach SSE
|
||||
* route wires `request.signal` here) tears the upstream provider request
|
||||
* down instead of running it to completion and paying the full token cost
|
||||
* into a closed connection. Composed with the per-client timeout inside
|
||||
* `safeFetch`. Omitted → behaviour is unchanged (timeout-only).
|
||||
*/
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
export interface CompletionResult {
|
||||
@@ -313,6 +322,8 @@ export function singleUserTurn(p: {
|
||||
documents?: CompletionDocument[];
|
||||
tools?: AiToolDef[];
|
||||
toolChoice?: "auto" | "none";
|
||||
/** v1.20.1 — caller-owned cancel signal threaded to the client fetch. */
|
||||
signal?: AbortSignal;
|
||||
}): CompletionParams {
|
||||
const images = p.images ?? [];
|
||||
const documents = p.documents ?? [];
|
||||
@@ -349,6 +360,7 @@ export function singleUserTurn(p: {
|
||||
responseFormat: p.responseFormat,
|
||||
tools: p.tools,
|
||||
toolChoice: p.toolChoice,
|
||||
signal: p.signal,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -502,6 +502,96 @@ describe("dispatchNotification — Telegram hard reject", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("dispatchNotification — config parse/decrypt failure is a hard reject", () => {
|
||||
it("malformed channel config auto-disables on first occurrence (not after 5 transient ticks)", async () => {
|
||||
const channel = makeChannel({
|
||||
type: "NTFY",
|
||||
// Not valid JSON → JSON.parse throws in sendToChannel.
|
||||
config: "{ this is not json",
|
||||
});
|
||||
vi.mocked(prisma.notificationChannel.findMany).mockResolvedValueOnce([
|
||||
channel,
|
||||
] as never);
|
||||
|
||||
await dispatchNotification({
|
||||
eventType: "SYSTEM_ALERT",
|
||||
userId: "u-1",
|
||||
title: "t",
|
||||
message: "m",
|
||||
});
|
||||
|
||||
// The sender is never reached on a parse failure.
|
||||
expect(sendViaNtfyMock).not.toHaveBeenCalled();
|
||||
|
||||
// Channel auto-disabled immediately with the precise parse reason —
|
||||
// NOT the misleading `give_up_after_5_failures`.
|
||||
const updateCalls = (
|
||||
prisma.notificationChannel.update as unknown as ReturnType<typeof vi.fn>
|
||||
).mock.calls;
|
||||
const disablePayload = updateCalls.find(
|
||||
(c) => (c[0] as { data: { enabled?: boolean } }).data?.enabled === false,
|
||||
);
|
||||
expect(disablePayload).toBeTruthy();
|
||||
expect(
|
||||
(disablePayload?.[0] as { data: { disabledReason?: string } }).data
|
||||
.disabledReason,
|
||||
).toBe("ntfy_config_parse_failed");
|
||||
|
||||
// It is logged as a hard reject, not a transient give-up.
|
||||
expect(auditLog).toHaveBeenCalledWith(
|
||||
"notification.channel.auto_disabled",
|
||||
expect.objectContaining({
|
||||
details: expect.objectContaining({
|
||||
reason: "ntfy_config_parse_failed",
|
||||
kind: "hard_reject",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("a genuine transient failure still takes the transient path (stays enabled, backoff set)", async () => {
|
||||
const channel = makeChannel({
|
||||
type: "NTFY",
|
||||
config: JSON.stringify({ serverUrl: "https://ntfy.example", topic: "t" }),
|
||||
consecutiveFailures: 0,
|
||||
});
|
||||
vi.mocked(prisma.notificationChannel.findMany).mockResolvedValueOnce([
|
||||
channel,
|
||||
] as never);
|
||||
sendViaNtfyMock.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
hardReject: false,
|
||||
statusCode: 503,
|
||||
reason: "ntfy_503",
|
||||
});
|
||||
(prisma.notificationChannel.update as unknown as ReturnType<typeof vi.fn>)
|
||||
.mockResolvedValueOnce({ consecutiveFailures: 1 })
|
||||
.mockResolvedValue({});
|
||||
|
||||
await dispatchNotification({
|
||||
eventType: "SYSTEM_ALERT",
|
||||
userId: "u-1",
|
||||
title: "t",
|
||||
message: "m",
|
||||
});
|
||||
|
||||
const updateCalls = (
|
||||
prisma.notificationChannel.update as unknown as ReturnType<typeof vi.fn>
|
||||
).mock.calls;
|
||||
// No enabled=false update → channel stays enabled, backoff scheduled.
|
||||
const disablePayload = updateCalls.find(
|
||||
(c) => (c[0] as { data: { enabled?: boolean } }).data?.enabled === false,
|
||||
);
|
||||
expect(disablePayload).toBeUndefined();
|
||||
const cooldownPayload = updateCalls.find(
|
||||
(c) =>
|
||||
(c[0] as { data: { nextRetryAt?: Date } }).data?.nextRetryAt instanceof
|
||||
Date,
|
||||
);
|
||||
expect(cooldownPayload).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("dispatchNotification — reminders reach Web Push without APNs", () => {
|
||||
it("a PWA-only self-hoster (WEB_PUSH channel, no APNS) still gets MEDICATION_REMINDER", async () => {
|
||||
const channel = makeChannel({ type: "WEB_PUSH" });
|
||||
|
||||
@@ -263,9 +263,16 @@ async function sendToChannel(
|
||||
decrypted = decrypt(encryptedConfig);
|
||||
} catch {
|
||||
getEvent()?.addWarning(`Failed to decrypt ${type} channel config`);
|
||||
// A config that can't be decrypted is a structurally-permanent failure,
|
||||
// not a transient blip: the ciphertext / key pairing won't fix itself on
|
||||
// a retry. Hard-reject so the channel auto-disables on first occurrence
|
||||
// with an accurate reason, rather than burning the 5-failure transient
|
||||
// budget and ending on the misleading `give_up_after_5_failures`. During
|
||||
// a key-rotation gap every channel would otherwise auto-disable for the
|
||||
// wrong reason.
|
||||
return {
|
||||
ok: false,
|
||||
hardReject: false,
|
||||
hardReject: true,
|
||||
reason: `${type.toLowerCase()}_config_decrypt_failed`,
|
||||
};
|
||||
}
|
||||
@@ -277,9 +284,10 @@ async function sendToChannel(
|
||||
config = JSON.parse(decrypted) as TelegramChannelConfig;
|
||||
} catch {
|
||||
getEvent()?.addWarning("Failed to parse Telegram channel config");
|
||||
// Malformed config decodes the same on every retry — permanent.
|
||||
return {
|
||||
ok: false,
|
||||
hardReject: false,
|
||||
hardReject: true,
|
||||
reason: "telegram_config_parse_failed",
|
||||
};
|
||||
}
|
||||
@@ -291,9 +299,10 @@ async function sendToChannel(
|
||||
config = JSON.parse(decrypted) as NtfyChannelConfig;
|
||||
} catch {
|
||||
getEvent()?.addWarning("Failed to parse ntfy channel config");
|
||||
// Malformed config decodes the same on every retry — permanent.
|
||||
return {
|
||||
ok: false,
|
||||
hardReject: false,
|
||||
hardReject: true,
|
||||
reason: "ntfy_config_parse_failed",
|
||||
};
|
||||
}
|
||||
@@ -305,9 +314,10 @@ async function sendToChannel(
|
||||
config = JSON.parse(decrypted) as WebhookChannelConfig;
|
||||
} catch {
|
||||
getEvent()?.addWarning("Failed to parse webhook channel config");
|
||||
// Malformed config decodes the same on every retry — permanent.
|
||||
return {
|
||||
ok: false,
|
||||
hardReject: false,
|
||||
hardReject: true,
|
||||
reason: "webhook_config_parse_failed",
|
||||
};
|
||||
}
|
||||
@@ -319,9 +329,10 @@ async function sendToChannel(
|
||||
config = JSON.parse(decrypted) as EmailChannelConfig;
|
||||
} catch {
|
||||
getEvent()?.addWarning("Failed to parse email channel config");
|
||||
// Malformed config decodes the same on every retry — permanent.
|
||||
return {
|
||||
ok: false,
|
||||
hardReject: false,
|
||||
hardReject: true,
|
||||
reason: "email_config_parse_failed",
|
||||
};
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user