Skip to content

Commit f7f5932

Browse files
ndycodeclaude
andauthored
fix(runtime): emit consistent token_invalidated body on upstream-401 path (#497)
* fix(runtime): emit consistent token_invalidated body on upstream-401 path The upstream-401 invalidation path forwarded the raw OpenAI body, so clients keying off error.code saw a stable "token_invalidated" code on the refresh-failure path but not here (Greptile P3 on #496). - Add buildTokenInvalidationBody(): wraps both paths in the same { error: { message, code: "token_invalidated" } } shape, preserving the upstream message when present and falling back to a stable message for non-JSON bodies (no markup echoed to clients). - Force content-type: application/json and drop content-length/-encoding via responseHeadersForClient since the body is rewritten. - Document that applyMonotonicAuthCooldown intentionally uses Date.now() (not the injected now()) to match the markAccountCoolingDown/nowMs() write domain, guarding against a regression that would defeat the monotonic cooldown race protection. - Strengthen existing upstream-401 and html-body tests to assert the client contract (code + message). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(runtime): route refresh-failure 401 through shared invalidation builder + unit-test branches Addresses two Greptile P2 comments on this PR: - The refresh-failure invalidation exit still hardcoded the body literals the new constants were meant to centralise, reopening the exact drift risk this PR closes. Route it through buildTokenInvalidationBody(""), which yields the same { error: { message: <fallback>, code: "token_invalidated" } } so both paths can no longer diverge. - Export buildTokenInvalidationBody and add 8 unit tests covering all message-extraction branches: empty input, top-level message, nested error.message, top-level-wins priority, blank-top-level -> nested fallback, non-JSON body, and no-usable-message fallback. Full suite: 4062 tests pass (4054 + 8). typecheck/lint/build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 4569197 commit f7f5932

2 files changed

Lines changed: 121 additions & 10 deletions

File tree

lib/runtime-rotation-proxy.ts

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,11 @@ function applyMonotonicAuthCooldown(
666666
cooldownMs: number,
667667
): void {
668668
const existing = accountManager.getAccountByIndex(account.index)?.coolingDownUntil ?? 0;
669+
// Intentionally Date.now(), not the proxy's injected now(): coolingDownUntil is
670+
// written by markAccountCoolingDown via nowMs() (== Date.now()), so both sides of
671+
// this comparison must live in the same real-wall-clock domain. Switching to the
672+
// injected now() would mis-compare an injected-clock value against a real-clock
673+
// deadline and silently defeat the monotonic guard.
669674
if (Date.now() + cooldownMs > existing) {
670675
accountManager.markAccountCoolingDown(account, cooldownMs, "auth-failure");
671676
}
@@ -843,6 +848,41 @@ async function readErrorBody(response: Response): Promise<string> {
843848
}
844849
}
845850

851+
const TOKEN_INVALIDATED_CODE = "token_invalidated";
852+
const TOKEN_INVALIDATED_FALLBACK_MESSAGE =
853+
"OAuth token has been invalidated. Please re-login.";
854+
855+
// Both invalidation exit paths (refresh-failure and upstream-401) must hand the
856+
// client the same machine-readable shape — { error: { message, code:
857+
// "token_invalidated" } } — so a consumer keying off error.code behaves
858+
// identically regardless of which vector fired. The upstream forwards a raw body
859+
// with no guaranteed code, so we wrap it here while preserving its human-readable
860+
// message when one is present.
861+
export function buildTokenInvalidationBody(upstreamBodyText: string): string {
862+
let message = TOKEN_INVALIDATED_FALLBACK_MESSAGE;
863+
const trimmed = upstreamBodyText.trim();
864+
if (trimmed) {
865+
try {
866+
const parsed = JSON.parse(trimmed) as unknown;
867+
if (isRecord(parsed)) {
868+
const direct = parsed.message;
869+
if (typeof direct === "string" && direct.trim()) {
870+
message = direct.trim();
871+
} else if (isRecord(parsed.error)) {
872+
const nested = parsed.error.message;
873+
if (typeof nested === "string" && nested.trim()) {
874+
message = nested.trim();
875+
}
876+
}
877+
}
878+
} catch {
879+
// Non-JSON upstream body (e.g. HTML error page): keep the stable fallback
880+
// message rather than echoing markup back to the client.
881+
}
882+
}
883+
return JSON.stringify({ error: { message, code: TOKEN_INVALIDATED_CODE } });
884+
}
885+
846886
function extractErrorCodeFromBody(bodyText: string): string | null {
847887
if (!bodyText.trim()) return null;
848888
try {
@@ -1516,14 +1556,10 @@ export async function startRuntimeRotationProxy(
15161556
// return auth error to client instead of rotating to the next account.
15171557
sessionAffinityStore?.forgetSession(context.sessionKey);
15181558
res.writeHead(HTTP_STATUS.UNAUTHORIZED, { "content-type": "application/json" });
1519-
res.end(
1520-
JSON.stringify({
1521-
error: {
1522-
message: "OAuth token has been invalidated. Please re-login.",
1523-
code: "token_invalidated",
1524-
},
1525-
}),
1526-
);
1559+
// Route through the shared builder so both invalidation exit paths stay
1560+
// in lockstep — empty input yields { error: { message: <fallback>,
1561+
// code: "token_invalidated" } }.
1562+
res.end(buildTokenInvalidationBody(""));
15271563
await usageRecorder.record({
15281564
outcome: "failure",
15291565
statusCode: HTTP_STATUS.UNAUTHORIZED,
@@ -1742,8 +1778,13 @@ export async function startRuntimeRotationProxy(
17421778
);
17431779
sessionAffinityStore?.forgetSession(context.sessionKey);
17441780
accountManager.saveToDiskDebounced();
1745-
res.writeHead(upstream.status, responseHeadersForClient(upstream.headers));
1746-
res.end(bodyText);
1781+
// Emit the same machine-readable shape as the refresh-failure path
1782+
// (code: "token_invalidated") instead of forwarding the raw upstream
1783+
// body, so the client contract is consistent across both vectors.
1784+
const clientHeaders = responseHeadersForClient(upstream.headers);
1785+
clientHeaders["content-type"] = "application/json";
1786+
res.writeHead(upstream.status, clientHeaders);
1787+
res.end(buildTokenInvalidationBody(bodyText));
17471788
await usageRecorder.record({
17481789
outcome: "failure",
17491790
statusCode: upstream.status,

test/runtime-rotation-proxy.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { AccountManager } from "../lib/accounts.js";
44
import { HTTP_STATUS, OPENAI_HEADERS } from "../lib/constants.js";
55
import {
66
startRuntimeRotationProxy,
7+
buildTokenInvalidationBody,
78
type RuntimeRotationProxyServer,
89
} from "../lib/runtime-rotation-proxy.js";
910
import { clearCircuitBreakers } from "../lib/circuit-breaker.js";
@@ -1857,6 +1858,13 @@ describe("runtime rotation proxy", () => {
18571858
const response = await postResponses(proxy, { model: "gpt-5-codex" });
18581859

18591860
expect(response.status).toBe(HTTP_STATUS.UNAUTHORIZED);
1861+
// upstream-401 invalidation path emits the same machine-readable contract as
1862+
// the refresh-failure path, preserving the upstream message
1863+
const body = (await response.json()) as { error: { message: string; code: string } };
1864+
expect(body.error.code).toBe("token_invalidated");
1865+
expect(body.error.message).toBe(
1866+
"Encountered invalidated oauth token for user, failing request",
1867+
);
18601868
expect(calls).toHaveLength(1);
18611869
expect(calls[0]?.headers.get(OPENAI_HEADERS.ACCOUNT_ID)).toBe("acc_1");
18621870
expect(accountManager.getAccountByIndex(0)?.cooldownReason).toBe("auth-failure");
@@ -2027,7 +2035,69 @@ describe("runtime rotation proxy", () => {
20272035
const response = await postResponses(proxy, { model: "gpt-5-codex" });
20282036

20292037
expect(response.status).toBe(HTTP_STATUS.UNAUTHORIZED);
2038+
// non-JSON upstream body falls back to the stable message rather than echoing
2039+
// markup back to the client, but still carries the consistent code
2040+
const body = (await response.json()) as { error: { message: string; code: string } };
2041+
expect(body.error.code).toBe("token_invalidated");
2042+
expect(body.error.message).toBe("OAuth token has been invalidated. Please re-login.");
20302043
expect(calls).toHaveLength(1);
20312044
expect(proxy.getStatus().rotations).toBe(0);
20322045
});
20332046
});
2047+
2048+
describe("buildTokenInvalidationBody", () => {
2049+
const FALLBACK = "OAuth token has been invalidated. Please re-login.";
2050+
const parse = (raw: string) =>
2051+
JSON.parse(raw) as { error: { message: string; code: string } };
2052+
2053+
it("always emits the token_invalidated code", () => {
2054+
expect(parse(buildTokenInvalidationBody("")).error.code).toBe("token_invalidated");
2055+
expect(
2056+
parse(buildTokenInvalidationBody(JSON.stringify({ message: "x" }))).error.code,
2057+
).toBe("token_invalidated");
2058+
});
2059+
2060+
it("uses the stable fallback message for empty input", () => {
2061+
expect(parse(buildTokenInvalidationBody("")).error.message).toBe(FALLBACK);
2062+
});
2063+
2064+
it("preserves a top-level message", () => {
2065+
const body = JSON.stringify({ message: "Encountered invalidated oauth token" });
2066+
expect(parse(buildTokenInvalidationBody(body)).error.message).toBe(
2067+
"Encountered invalidated oauth token",
2068+
);
2069+
});
2070+
2071+
it("preserves a nested error.message when no top-level message is present", () => {
2072+
const body = JSON.stringify({ error: { message: "nested invalidation detail" } });
2073+
expect(parse(buildTokenInvalidationBody(body)).error.message).toBe(
2074+
"nested invalidation detail",
2075+
);
2076+
});
2077+
2078+
it("prefers a top-level message over a nested error.message", () => {
2079+
const body = JSON.stringify({
2080+
message: "top-level wins",
2081+
error: { message: "nested loses" },
2082+
});
2083+
expect(parse(buildTokenInvalidationBody(body)).error.message).toBe("top-level wins");
2084+
});
2085+
2086+
it("falls back to nested error.message when top-level message is blank/whitespace", () => {
2087+
const body = JSON.stringify({
2088+
message: " ",
2089+
error: { message: "nested fallback" },
2090+
});
2091+
expect(parse(buildTokenInvalidationBody(body)).error.message).toBe("nested fallback");
2092+
});
2093+
2094+
it("falls back to the stable message for non-JSON bodies (no markup echoed)", () => {
2095+
const html = "<html><body>oauth token has been invalidated</body></html>";
2096+
expect(parse(buildTokenInvalidationBody(html)).error.message).toBe(FALLBACK);
2097+
});
2098+
2099+
it("falls back to the stable message when no usable message field exists", () => {
2100+
const body = JSON.stringify({ error: { code: "something_else" } });
2101+
expect(parse(buildTokenInvalidationBody(body)).error.message).toBe(FALLBACK);
2102+
});
2103+
});

0 commit comments

Comments
 (0)