fix(auth-broker): retain cache resilience on server errors
This commit is contained in:
@@ -146,7 +146,7 @@ The 15 s client window deliberately sits below the broker’s 5 min server cache
|
||||
|
||||
Freshness is anchored to the broker-stamped `snapshot.generatedAt`, not local write time. Default TTL is 1 h (`OMP_AUTH_BROKER_SNAPSHOT_TTL_MS`); `0` disables cache reads and writes. A fresh cache is revalidated against a reachable broker with a 500 ms startup budget, so an imported, revoked, or rotated credential is visible to one-shot commands immediately. If revalidation fails because the broker is unavailable or slow, `omp` starts from the cache and `RemoteAuthCredentialStore` continues normal SSE / long-poll synchronization in the background. Expired OAuth access tokens still refresh through `POST /v1/credential/:id/refresh`.
|
||||
|
||||
If the broker is down at boot and a fresh cache exists, startup succeeds from the cached snapshot. Authentication and other HTTP failures are not masked by the cache. If the cache is missing, expired, corrupt, written for a different URL, or encrypted with a different token, startup falls back to the live fetch and fails if the broker is unreachable.
|
||||
If the broker is down at boot and a fresh cache exists, startup succeeds from the cached snapshot. Authentication failures (401/403) are not masked by the cache; transient server errors fall back to it. If the cache is missing, expired, corrupt, written for a different URL, or encrypted with a different token, startup falls back to the live fetch and fails if the broker is unreachable.
|
||||
|
||||
## Client account pools (routing, not authorization)
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fresh encrypted auth-broker snapshot caches are revalidated within a short startup budget, so one-shot clients see newly imported or revoked credentials immediately when the broker is reachable while retaining cache fallback for transport and server failures.
|
||||
- Fixed OpenAI Responses native history replay sending output-only `status` fields back as input, preventing `input[N].status` failures in long-running sessions. ([#6513](https://github.com/can1357/oh-my-pi/pull/6513) by [@Ant39140](https://github.com/Ant39140))
|
||||
- Cursor no longer discards a local tool result when the transport fails mid-execution. The provider waits for in-flight exec dispatches before pushing `done`, but the error path skipped that wait, so a handler decoded from the last chunk landed its result after the Agent had already finalized the call from the terminal error and cleared its buffer — losing the real outcome of a tool that may already have run side effects. Both exits now drain the same barrier.
|
||||
- Cursor exec handlers returning the bare-result form no longer record a failed call as successful. When an SDK handler returns only a protocol result (no paired `toolResult`), the synthesized transcript entry was always `"Tool produced no transcript result"` with `isError: false`, even for a `rejected` or `error` result — so Cursor saw a failure while the rebuilt transcript showed success. The synthesized entry now derives its state and message from the result's own oneof variant — including MCP, where an application-level tool failure rides inside the `success` variant as `is_error` rather than as a separate variant.
|
||||
|
||||
@@ -431,7 +431,15 @@ export class AuthBrokerClient {
|
||||
signal,
|
||||
});
|
||||
if (!response.ok && response.status !== 304) {
|
||||
const text = await response.text();
|
||||
let text = "";
|
||||
try {
|
||||
text = await response.text();
|
||||
} catch (cause) {
|
||||
throw new AuthBrokerError(`Auth broker request failed: ${response.status} ${response.statusText}`, {
|
||||
status: response.status,
|
||||
cause,
|
||||
});
|
||||
}
|
||||
throw new AuthBrokerError(`Auth broker request failed: ${response.status} ${response.statusText}`, {
|
||||
status: response.status,
|
||||
body: text,
|
||||
@@ -442,6 +450,7 @@ export class AuthBrokerClient {
|
||||
lastError = error;
|
||||
// Caller-driven abort wins over retry — the caller said stop.
|
||||
if (opts.signal?.aborted) {
|
||||
if (error instanceof AuthBrokerError && error.status !== undefined) throw error;
|
||||
throw new AuthBrokerError("Auth broker request aborted", { cause: opts.signal.reason });
|
||||
}
|
||||
if (error instanceof AuthBrokerError && error.status !== undefined) {
|
||||
|
||||
@@ -276,13 +276,14 @@ export async function discoverAuthStorage(options: DiscoverAuthStorageOptions =
|
||||
signal: cachedSnapshot ? AbortSignal.timeout(SNAPSHOT_CACHE_REVALIDATION_TIMEOUT_MS) : undefined,
|
||||
});
|
||||
if (initialResult.status !== 200)
|
||||
throw new AIError.AuthBrokerError("Auth broker returned no initial snapshot", {
|
||||
throw new AuthBrokerError("Auth broker returned no initial snapshot", {
|
||||
status: initialResult.status,
|
||||
});
|
||||
initialSnapshot = initialResult.snapshot;
|
||||
persist?.(initialSnapshot);
|
||||
} catch (error) {
|
||||
if (!cachedSnapshot || (error instanceof AuthBrokerError && error.status !== undefined)) throw error;
|
||||
if (!cachedSnapshot || (error instanceof AuthBrokerError && [401, 403].includes(error.status ?? 0)))
|
||||
throw error;
|
||||
}
|
||||
const store = new RemoteAuthCredentialStore({
|
||||
client,
|
||||
|
||||
@@ -91,6 +91,26 @@ describe("auth-broker wire surface", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("preserves an HTTP rejection when the caller aborts while reading its body", async () => {
|
||||
const client = new AuthBrokerClient({
|
||||
url: "http://broker.invalid",
|
||||
token,
|
||||
maxRetries: 0,
|
||||
fetchImpl: (async (_input, init) => {
|
||||
const signal = init?.signal;
|
||||
const body = new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
controller.enqueue(new TextEncoder().encode("forbidden"));
|
||||
signal?.addEventListener("abort", () => controller.error(signal.reason), { once: true });
|
||||
},
|
||||
});
|
||||
return new Response(body, { status: 401 });
|
||||
}) as typeof fetch,
|
||||
});
|
||||
|
||||
await expect(client.fetchSnapshot({ signal: AbortSignal.timeout(10) })).rejects.toMatchObject({ status: 401 });
|
||||
});
|
||||
|
||||
test("GET /v1/snapshot returns generation headers and 304 for unchanged long-poll", async () => {
|
||||
const res = await fetch(`${handle!.url}/v1/snapshot`, {
|
||||
headers: { Authorization: `Bearer ${token}` },
|
||||
|
||||
@@ -141,6 +141,59 @@ describe("discoverAuthStorage auth-broker snapshot cache", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("boots from a fresh cache when revalidation returns a server error", async () => {
|
||||
const cachePath = path.join(tempDir, "snapshot.enc");
|
||||
const server = Bun.serve({
|
||||
port: 0,
|
||||
fetch: () => new Response("temporarily unavailable", { status: 503 }),
|
||||
});
|
||||
const url = server.url.toString();
|
||||
let storage: AuthStorage | undefined;
|
||||
try {
|
||||
process.env.OMP_AUTH_BROKER_URL = url;
|
||||
process.env.OMP_AUTH_BROKER_TOKEN = TOKEN;
|
||||
process.env.OMP_AUTH_BROKER_SNAPSHOT_CACHE = cachePath;
|
||||
process.env.OMP_AUTH_BROKER_SNAPSHOT_TTL_MS = "3600000";
|
||||
await writeAuthBrokerSnapshotCache({
|
||||
path: cachePath,
|
||||
token: TOKEN,
|
||||
url,
|
||||
snapshot: makeSnapshot(Date.now()),
|
||||
});
|
||||
|
||||
storage = await discoverAuthStorage(tempDir);
|
||||
expect(await storage.getApiKey(PROVIDER)).toBe("cached-api-key");
|
||||
} finally {
|
||||
storage?.close();
|
||||
server.stop(true);
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects a fresh cache when the broker rejects its bearer token", async () => {
|
||||
const cachePath = path.join(tempDir, "snapshot.enc");
|
||||
const server = Bun.serve({
|
||||
port: 0,
|
||||
fetch: () => new Response("unauthorized", { status: 401 }),
|
||||
});
|
||||
const url = server.url.toString();
|
||||
try {
|
||||
process.env.OMP_AUTH_BROKER_URL = url;
|
||||
process.env.OMP_AUTH_BROKER_TOKEN = TOKEN;
|
||||
process.env.OMP_AUTH_BROKER_SNAPSHOT_CACHE = cachePath;
|
||||
process.env.OMP_AUTH_BROKER_SNAPSHOT_TTL_MS = "3600000";
|
||||
await writeAuthBrokerSnapshotCache({
|
||||
path: cachePath,
|
||||
token: TOKEN,
|
||||
url,
|
||||
snapshot: makeSnapshot(Date.now()),
|
||||
});
|
||||
|
||||
await expect(discoverAuthStorage(tempDir)).rejects.toMatchObject({ status: 401 });
|
||||
} finally {
|
||||
server.stop(true);
|
||||
}
|
||||
});
|
||||
|
||||
test("prefers a reachable broker snapshot over a fresh cached snapshot", async () => {
|
||||
const cachePath = path.join(tempDir, "snapshot.enc");
|
||||
const brokerStore = await SqliteAuthCredentialStore.open(path.join(tempDir, "broker.db"));
|
||||
|
||||
Reference in New Issue
Block a user