Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions devlog/_plan/260907_axis1_bugfixes/021_source_review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# wp1 source review

Three bounded patches implemented with regression coverage. Hooke independently passed the physical-response quota observer wiring; Tesla independently passed quota/recovery security and source review with zero blockers. Version comparator and status/doctor projections inspected by main. All source workers report no local suite/typecheck/build execution.

Quota source: #3809, Éverton Toffanetto; Co-authored-by included in f215f79b4. Version report: garysassano; Reported-by included in f91e3953a. Recovery report: Hu9956; Reported-by included in recovery commit.

Source-only checks: git diff --check and documentation fence/whitespace inspection. These do not prove runtime correctness. wp2 final cumulative hosted CI is still mandatory. Final CI dispatch includes Windows because ordinary PR workflow omits it. No release/deploy workflow will be dispatched.
14 changes: 12 additions & 2 deletions docs-site/src/content/docs/reference/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,17 @@ response is not cacheable. Post-commit and 5xx errors keep the no-resend path.

When encrypted agent-task recovery refuses a routed task, its existing 400 error
can include a bounded `recovery_reason`: `unsupported_envelope`,
`admission_denied`, `recovery_unavailable`, `caller_cancelled`, or `input_changed`.
The field is omitted when no classified recovery result exists.
`admission_denied`, `recovery_unavailable`, `caller_cancelled`, `input_changed`,
`recovery_http_rejected`, `recovery_timeout`, `recovery_aborted`,
`recovery_transport_error`, or `recovery_invalid_output`.
HTTP rejection requires an observed non-success response. Invalid output includes
invalid UTF-8, oversized bodies, malformed or incomplete recovery streams, and
invalid or conflicting assignments. A caller's cancellation takes precedence over
an owned deadline, which takes precedence over decode/transport failures.
`recovery_aborted` describes a shared recovery cancelled independently of that caller.
Shared-flight waiters receive the same underlying failure unless individually cancelled;
only successful plaintext is cached. Diagnostics contain no upstream error or payload text.
The field is omitted when no classified recovery result exists, and existing combo
branches that return the original target failure keep that response.
`recovery_unavailable` includes cache/singleflight capacity and does not prove an
upstream request was attempted. No retry or broader envelope acceptance is enabled.
1 change: 1 addition & 0 deletions scripts/test-layout/layout.json
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@
"aside-profiles-routes.test.ts": "server",
"aside-profiles.test.ts": "clients",
"aside-profile-paths.test.ts": "clients",
"aside-profile-identity.test.ts": "clients",
"aside-profile-sync-owner.test.ts": "clients",
"assert-mergeable-review.test.ts": "ci-workflows",
"auto-compact-budget.test.ts": "providers",
Expand Down
15 changes: 8 additions & 7 deletions src/clients/aside-profiles.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { lstatSync, readFileSync, readlinkSync, realpathSync, statSync, type Stats } from "node:fs";
import { lstatSync, readFileSync, readlinkSync, realpathSync, statSync, type BigIntStats } from "node:fs";
import { homedir } from "node:os";
import { basename, dirname, isAbsolute, join, resolve } from "node:path";
import type { IntegrationIO } from "../integrations/config-io";
Expand All @@ -14,7 +14,7 @@ export interface AsideProfile {
}

const MAX_PROFILES = 128;
const MAX_MANIFEST_BYTES = 4 * 1024 * 1024;
const MAX_MANIFEST_BYTES = 4n * 1024n * 1024n;
const MAX_LEAF_LINKS = 40;

function refuse(message: string): never {
Expand All @@ -30,9 +30,10 @@ function object(value: unknown): value is Record<string, unknown> {
return value !== null && typeof value === "object" && !Array.isArray(value);
}

function inspect(path: string, follow = false): Stats | null {
function inspect(path: string, follow = false): BigIntStats | null {
try {
return follow ? statSync(path) : lstatSync(path);
// File IDs can exceed Number's exact integer range; never round identities.
return follow ? statSync(path, { bigint: true }) : lstatSync(path, { bigint: true });
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
return refuse("a filesystem boundary could not be inspected.");
Expand Down Expand Up @@ -110,10 +111,10 @@ export function listAsideProfiles(env: NodeJS.ProcessEnv = process.env, home: st
return readProfiles(root);
}

type DirectoryIdentity = { path: string; dev: number; ino: number };
type DirectoryIdentity = { path: string; dev: bigint; ino: bigint };
type Boundary = Array<DirectoryIdentity | null>;

function sameIdentity(a: Pick<Stats, "dev" | "ino">, b: Pick<Stats, "dev" | "ino">): boolean {
function sameIdentity(a: Pick<BigIntStats, "dev" | "ino">, b: Pick<BigIntStats, "dev" | "ino">): boolean {
return a.dev === b.dev && a.ino === b.ino;
}

Expand Down Expand Up @@ -162,7 +163,7 @@ function boundary(profile: AsideProfile, profiles: AsideProfile[], mutation: boo
}
if (absent) return identities;
const leaf = inspect(profile.configPath);
if (leaf && (leaf.isSymbolicLink() || !leaf.isFile() || leaf.nlink > 1)) {
if (leaf && (leaf.isSymbolicLink() || !leaf.isFile() || leaf.nlink > 1n)) {
refuse("the model catalog is a link, shared file or non-regular file.");
}
if (leaf && canonical(profile.configPath) !== join(parent!, "models.json")) {
Expand Down
29 changes: 22 additions & 7 deletions src/lib/bounded-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,28 @@ export async function readBoundedResponseBytes(
}
}

function decodeUtf8(chunks: readonly Uint8Array[], fatal: boolean): string {
// Mark only exceptions thrown by our decoder, preserving their identity and TypeError contract.
// Timeout-path flushing may fail too; retain that origin so callers do not lose the deadline.
const decodeFailures = new WeakMap<object, "invalid_utf8" | "timeout">();

export function boundedBodyDecodeFailure(error: unknown): "invalid_utf8" | "timeout" | undefined {
return error !== null && typeof error === "object" ? decodeFailures.get(error) : undefined;
}

function decodeUtf8(chunks: readonly Uint8Array[], fatal: boolean, timedOut = false): string {
const decoder = new TextDecoder("utf-8", { fatal });
let text = "";
for (const chunk of chunks) text += decoder.decode(chunk, { stream: true });
// Flush an incomplete trailing UTF-8 sequence deterministically.
text += decoder.decode();
return text;
try {
let text = "";
for (const chunk of chunks) text += decoder.decode(chunk, { stream: true });
// Flush an incomplete trailing UTF-8 sequence deterministically.
text += decoder.decode();
return text;
} catch (error) {
if (error !== null && typeof error === "object") {
decodeFailures.set(error, timedOut ? "timeout" : "invalid_utf8");
}
throw error;
}
}

/**
Expand Down Expand Up @@ -297,7 +312,7 @@ export async function readBoundedResponseBody(
"TimeoutError",
);
return {
text: decodeUtf8([retained.subarray(0, retainedBytes)], options.fatalUtf8 === true),
text: decodeUtf8([retained.subarray(0, retainedBytes)], options.fatalUtf8 === true, true),
truncated: true,
timedOut: true,
totalTimedOut: outcome === TOTAL_TIMEOUT,
Expand Down
57 changes: 43 additions & 14 deletions src/server/responses/agent-task-recovery-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@ const MAX_CACHE_BYTES = 8 * 1024 * 1024;
const MAX_CONCURRENT_RECOVERIES = 32;
const CACHE_TTL_MS = 15 * 60 * 1000;

export type AgentTaskRecoveryResolutionFailureReason =
| "recovery_unavailable"
| "caller_cancelled"
| "recovery_http_rejected"
| "recovery_timeout"
| "recovery_aborted"
| "recovery_transport_error"
| "recovery_invalid_output";

/** Shared flights carry bounded failures; only successful plaintext enters the cache. */
export type AgentTaskRecoveryResolution =
| { readonly recovered: true; readonly assignment: string }
| { readonly recovered: false; readonly reason: AgentTaskRecoveryResolutionFailureReason };

interface RecoveryCacheEntry {
assignment: string;
bytes: number;
Expand All @@ -11,7 +25,7 @@ interface RecoveryCacheEntry {

interface RecoveryFlight {
controller: AbortController;
promise: Promise<string | null>;
promise: Promise<AgentTaskRecoveryResolution>;
waiters: number;
settled: boolean;
}
Expand Down Expand Up @@ -63,7 +77,7 @@ function insertRecoveryCacheEntry(key: string, assignment: string, maxEntries: n
function startRecoveryFlight(
key: string,
maxEntries: number,
request: (signal: AbortSignal) => Promise<string | null>,
request: (signal: AbortSignal) => Promise<AgentTaskRecoveryResolution>,
): RecoveryFlight | null {
const active = RECOVERY_FLIGHTS.get(key);
if (active) return active;
Expand All @@ -72,15 +86,15 @@ function startRecoveryFlight(
const controller = new AbortController();
const flight: RecoveryFlight = {
controller,
promise: Promise.resolve(null),
promise: Promise.resolve({ recovered: false, reason: "recovery_unavailable" }),
waiters: 0,
settled: false,
};
flight.promise = request(controller.signal)
.then((assignment) => {
if (!assignment || controller.signal.aborted) return null;
insertRecoveryCacheEntry(key, assignment, maxEntries);
return assignment;
.then((result): AgentTaskRecoveryResolution => {
if (controller.signal.aborted) return { recovered: false, reason: "recovery_aborted" };
if (result.recovered) insertRecoveryCacheEntry(key, result.assignment, maxEntries);
return result;
})
.finally(() => {
flight.settled = true;
Expand All @@ -93,14 +107,14 @@ function startRecoveryFlight(
async function waitForRecoveryFlight(
flight: RecoveryFlight,
abortSignal?: AbortSignal,
): Promise<string | null> {
if (abortSignal?.aborted) return null;
): Promise<AgentTaskRecoveryResolution> {
if (abortSignal?.aborted) return { recovered: false, reason: "caller_cancelled" };
flight.waiters += 1;
let onAbort: (() => void) | undefined;
try {
if (!abortSignal) return await flight.promise;
const cancelled = new Promise<null>((resolve) => {
onAbort = () => resolve(null);
const cancelled = new Promise<AgentTaskRecoveryResolution>((resolve) => {
onAbort = () => resolve({ recovered: false, reason: "caller_cancelled" });
abortSignal.addEventListener("abort", onAbort, { once: true });
if (abortSignal.aborted) onAbort();
});
Expand All @@ -120,12 +134,27 @@ export async function resolveCachedAgentTaskRecovery(
request: (signal: AbortSignal) => Promise<string | null>,
abortSignal?: AbortSignal,
): Promise<string | null> {
if (abortSignal?.aborted) return null;
const result = await resolveCachedAgentTaskRecoveryWithResult(key, maxEntries, async signal => {
const assignment = await request(signal);
return assignment
? { recovered: true, assignment }
: { recovered: false, reason: "recovery_unavailable" };
}, abortSignal);
return result.recovered ? result.assignment : null;
}

export async function resolveCachedAgentTaskRecoveryWithResult(
key: string,
maxEntries: number,
request: (signal: AbortSignal) => Promise<AgentTaskRecoveryResolution>,
abortSignal?: AbortSignal,
): Promise<AgentTaskRecoveryResolution> {
if (abortSignal?.aborted) return { recovered: false, reason: "caller_cancelled" };
sweepRecoveryCache(Date.now(), maxEntries);
const cached = RECOVERY_CACHE.get(key)?.assignment;
if (cached) return cached;
if (cached) return { recovered: true, assignment: cached };
const flight = startRecoveryFlight(key, maxEntries, request);
return flight ? waitForRecoveryFlight(flight, abortSignal) : null;
return flight ? waitForRecoveryFlight(flight, abortSignal) : { recovered: false, reason: "recovery_unavailable" };
}

export function discardCachedAgentTaskRecovery(key: string): void {
Expand Down
44 changes: 28 additions & 16 deletions src/server/responses/agent-task-recovery.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { createHash, createHmac, randomBytes } from "node:crypto";
import { decodeJwtPayload, extractAccountId } from "../../oauth/chatgpt";
import type { OcxConfig } from "../../types";
import { readBoundedResponseBody } from "../../lib/bounded-body";
import { boundedBodyDecodeFailure, readBoundedResponseBody } from "../../lib/bounded-body";
import { isApiAuthRequired, isProxyAdmissionSecret } from "../auth-cors";
import { structurallyValidFernetTokens } from "./encrypted-payload";
import {
cachedAgentTaskRecovery,
discardCachedAgentTaskRecovery,
resetAgentTaskRecoveryCache,
resolveCachedAgentTaskRecovery,
resolveCachedAgentTaskRecoveryWithResult,
type AgentTaskRecoveryResolution,
type AgentTaskRecoveryResolutionFailureReason,
} from "./agent-task-recovery-cache";

/** Experimental opt-in normalization through ChatGPT's fixed Codex endpoint. */
Expand Down Expand Up @@ -44,9 +46,8 @@ export interface AgentTaskRecoveryOptions {
export type AgentTaskRecoveryFailureReason =
| "unsupported_envelope"
| "admission_denied"
// Includes cache capacity rejection; does not imply an upstream request was attempted.
| "recovery_unavailable"
| "caller_cancelled"
// recovery_unavailable includes capacity rejection, which does not imply an upstream attempt.
| AgentTaskRecoveryResolutionFailureReason
| "input_changed";

export type AgentTaskRecoveryResult =
Expand Down Expand Up @@ -436,7 +437,7 @@ async function requestRecovery(
envelope: AgentEnvelope,
options: AgentTaskRecoveryOptions,
abortSignal?: AbortSignal,
): Promise<string | null> {
): Promise<AgentTaskRecoveryResolution> {
const controller = new AbortController();
const timeout = setTimeout(
() => controller.abort(new DOMException("Agent task recovery timed out", "TimeoutError")),
Expand All @@ -454,8 +455,11 @@ async function requestRecovery(
redirect: "error",
});
if (!response.ok) {
try { await response.body?.cancel(); } catch { /* already closed */ }
return null;
// A rejected or never-settling cancellation must not extend the recovery deadline.
try { void response.body?.cancel().catch(() => undefined); } catch { /* already closed */ }
if (abortSignal?.aborted) return { recovered: false, reason: "recovery_aborted" };
if (controller.signal.aborted) return { recovered: false, reason: "recovery_timeout" };
return { recovered: false, reason: "recovery_http_rejected" };
}
const body = await readBoundedResponseBody(response, {
signal,
Expand All @@ -465,10 +469,18 @@ async function requestRecovery(
inactivityTimeoutMs: options.timeoutMs ?? 45_000,
firstByteTimeoutMs: options.timeoutMs ?? 45_000,
});
if (body.truncated || body.oversized || body.timedOut || !body.displaySafe) return null;
return assignmentFromRecoverySse(body.text, envelope);
} catch {
return null;
if (abortSignal?.aborted) return { recovered: false, reason: "recovery_aborted" };
if (controller.signal.aborted || body.timedOut) return { recovered: false, reason: "recovery_timeout" };
if (body.truncated || body.oversized || !body.displaySafe) return { recovered: false, reason: "recovery_invalid_output" };
const assignment = assignmentFromRecoverySse(body.text, envelope);
return assignment === null
? { recovered: false, reason: "recovery_invalid_output" }
: { recovered: true, assignment };
} catch (error) {
if (abortSignal?.aborted) return { recovered: false, reason: "recovery_aborted" };
const decodeFailure = boundedBodyDecodeFailure(error);
if (controller.signal.aborted || decodeFailure === "timeout") return { recovered: false, reason: "recovery_timeout" };
return { recovered: false, reason: decodeFailure === "invalid_utf8" ? "recovery_invalid_output" : "recovery_transport_error" };
} finally {
clearTimeout(timeout);
}
Expand Down Expand Up @@ -497,23 +509,23 @@ export async function recoverEncryptedAgentTaskWithResult(
const admitted = admittedRecovery(req, input, config, context.parentThreadId);
if (!admitted.admitted) return { recovered: false, reason: admitted.reason };
const { admission, cacheKey, envelope } = admitted.recovery;
const assignment = await resolveCachedAgentTaskRecovery(
const result = await resolveCachedAgentTaskRecoveryWithResult(
cacheKey,
options.cacheEntries ?? 200,
signal => requestRecovery(admission, envelope, options, signal),
context.abortSignal,
);
if (!assignment) {
if (!result.recovered) {
return {
recovered: false,
reason: context.abortSignal?.aborted ? "caller_cancelled" : "recovery_unavailable",
reason: context.abortSignal?.aborted ? "caller_cancelled" : result.reason,
};
}
if (context.abortSignal?.aborted) {
discardCachedAgentTaskRecovery(cacheKey);
return { recovered: false, reason: "caller_cancelled" };
}
if (!injectAssignment(input, envelope, assignment)) {
if (!injectAssignment(input, envelope, result.assignment)) {
discardCachedAgentTaskRecovery(cacheKey);
return { recovered: false, reason: "input_changed" };
}
Expand Down
14 changes: 12 additions & 2 deletions structure/04_transports-and-sidecars.md
Original file line number Diff line number Diff line change
Expand Up @@ -1719,7 +1719,17 @@ response is not cacheable. Post-commit and 5xx errors keep the no-resend path.

When encrypted agent-task recovery refuses a routed task, its existing 400 error
can include a bounded `recovery_reason`: `unsupported_envelope`,
`admission_denied`, `recovery_unavailable`, `caller_cancelled`, or `input_changed`.
The field is omitted when no classified recovery result exists.
`admission_denied`, `recovery_unavailable`, `caller_cancelled`, `input_changed`,
`recovery_http_rejected`, `recovery_timeout`, `recovery_aborted`,
`recovery_transport_error`, or `recovery_invalid_output`.
HTTP rejection requires an observed non-success response. Invalid output includes
invalid UTF-8, oversized bodies, malformed or incomplete recovery streams, and
invalid or conflicting assignments. A caller's cancellation takes precedence over
an owned deadline, which takes precedence over decode/transport failures.
`recovery_aborted` describes a shared recovery cancelled independently of that caller.
Shared-flight waiters receive the same underlying failure unless individually cancelled;
only successful plaintext is cached. Diagnostics contain no upstream error or payload text.
The field is omitted when no classified recovery result exists, and existing combo
branches that return the original target failure keep that response.
`recovery_unavailable` includes cache/singleflight capacity and does not prove an
upstream request was attempted. No retry or broader envelope acceptance is enabled.
Loading
Loading