Skip to content

Commit e446dc3

Browse files
committed
fix remaining runtime review findings
1 parent c2f867a commit e446dc3

7 files changed

Lines changed: 81 additions & 17 deletions

lib/auto-update-checker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ function taskkillProcessTree(pid: number): Promise<boolean> {
542542
"/d",
543543
"/s",
544544
"/c",
545-
`taskkill /PID ${pid} /T /F >NUL 2>NUL`,
545+
`taskkill /PID ${pid} /T /F`,
546546
],
547547
{
548548
stdio: "ignore",

lib/request/fetch-helpers.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,13 +329,12 @@ export function isWorkspaceDisabledError(
329329
bodyText: string,
330330
): boolean {
331331
const normalizedCode = typeof code === "string" ? code.trim().toLowerCase() : "";
332-
const haystack = `${normalizedCode} ${bodyText}`.toLowerCase();
333-
const normalizedTokens = normalizedCode
334-
.split(/[^a-z0-9_]+/i)
335-
.map((token) => token.trim())
336-
.filter((token) => token.length > 0);
337332

338333
if (status === 402) {
334+
const normalizedTokens = normalizedCode
335+
.split(/[^a-z0-9_]+/i)
336+
.map((token) => token.trim())
337+
.filter((token) => token.length > 0);
339338
return (
340339
normalizedTokens.includes("deactivated_workspace") ||
341340
/\bdeactivated_workspace\b/i.test(bodyText)
@@ -346,6 +345,12 @@ export function isWorkspaceDisabledError(
346345
return false;
347346
}
348347

348+
const haystack = `${normalizedCode} ${bodyText}`.toLowerCase();
349+
const normalizedTokens = normalizedCode
350+
.split(/[^a-z0-9_]+/i)
351+
.map((token) => token.trim())
352+
.filter((token) => token.length > 0);
353+
349354
const disabledPatterns = [
350355
/workspace.*(?:disabled|expired|deactivated|terminated)/i,
351356
/account\s+(?:has\s+been|is)\s+(?:disabled|expired|deactivated|terminated|closed)/i,

lib/runtime-rotation-proxy.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,19 +1086,24 @@ export async function startRuntimeRotationProxy(
10861086
const bodyText = await readErrorBody(upstream);
10871087
const errorCode = extractErrorCodeFromBody(bodyText);
10881088
if (isWorkspaceDisabledError(upstream.status, errorCode, bodyText)) {
1089+
const accountWasEnabled =
1090+
accountManager.getAccountByIndex(refreshed.account.index)?.enabled !==
1091+
false;
10891092
accountManager.refundToken(
10901093
refreshed.account,
10911094
context.family,
10921095
context.model,
10931096
);
1094-
accountManager.recordFailure(
1095-
refreshed.account,
1096-
context.family,
1097-
context.model,
1098-
);
1099-
accountManager.setAccountEnabled(refreshed.account.index, false);
1097+
if (accountWasEnabled) {
1098+
accountManager.recordFailure(
1099+
refreshed.account,
1100+
context.family,
1101+
context.model,
1102+
);
1103+
accountManager.setAccountEnabled(refreshed.account.index, false);
1104+
accountManager.saveToDiskDebounced();
1105+
}
11001106
sessionAffinityStore?.forgetSession(context.sessionKey);
1101-
accountManager.saveToDiskDebounced();
11021107
exhaustionReason = "deactivated";
11031108
status.retries += 1;
11041109
status.rotations += 1;

scripts/test-model-matrix.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ function runQuietWindowsTaskkill(pid) {
153153
"/d",
154154
"/s",
155155
"/c",
156-
`taskkill /F /T /PID ${pid} >NUL 2>NUL`,
156+
`taskkill /F /T /PID ${pid}`,
157157
]);
158158
}
159159

test/auto-update-checker.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ describe("auto-update-checker", () => {
894894
"/d",
895895
"/s",
896896
"/c",
897-
"taskkill /PID 1234 /T /F >NUL 2>NUL",
897+
"taskkill /PID 1234 /T /F",
898898
],
899899
expect.objectContaining({ stdio: "ignore", windowsHide: true }),
900900
);

test/runtime-rotation-proxy.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,60 @@ describe("runtime rotation proxy", () => {
862862
});
863863
});
864864

865+
it("records a concurrent deactivation failure once for the account", async () => {
866+
const now = Date.now();
867+
const accountManager = new AccountManager(undefined, createStorage(now, 1));
868+
const recordFailureSpy = vi.spyOn(accountManager, "recordFailure");
869+
let disabledCalls = 0;
870+
let releaseDisabledCalls: (() => void) | null = null;
871+
const allDisabledCallsArrived = new Promise<void>((resolve) => {
872+
releaseDisabledCalls = resolve;
873+
});
874+
const { calls, fetchImpl } = createRecordingFetch(async (call) => {
875+
if (call.headers.get(OPENAI_HEADERS.ACCOUNT_ID) === "acc_1") {
876+
disabledCalls += 1;
877+
if (disabledCalls === 2) releaseDisabledCalls?.();
878+
await allDisabledCallsArrived;
879+
return new Response(
880+
JSON.stringify({ error: { code: "deactivated_workspace" } }),
881+
{
882+
status: 402,
883+
headers: { "content-type": "application/json" },
884+
},
885+
);
886+
}
887+
return textEventStream("data: recovered\n\n");
888+
});
889+
const proxy = await startProxy({ accountManager, fetchImpl });
890+
const body = {
891+
model: "gpt-5-codex",
892+
stream: true,
893+
metadata: { session_id: "thread-concurrent-deactivated" },
894+
};
895+
896+
const responses = await Promise.all([postResponses(proxy, body), postResponses(proxy, body)]);
897+
const payloads = (await Promise.all(responses.map((response) => response.json()))) as Array<{
898+
error: { reason: string };
899+
}>;
900+
901+
expect(responses.map((response) => response.status)).toEqual([
902+
HTTP_STATUS.SERVICE_UNAVAILABLE,
903+
HTTP_STATUS.SERVICE_UNAVAILABLE,
904+
]);
905+
expect(payloads.map((payload) => payload.error.reason)).toEqual([
906+
"deactivated",
907+
"deactivated",
908+
]);
909+
expect(calls.map((call) => call.headers.get(OPENAI_HEADERS.ACCOUNT_ID))).toEqual([
910+
"acc_1",
911+
"acc_1",
912+
]);
913+
expect(
914+
recordFailureSpy.mock.calls.filter(([account]) => account.index === 0),
915+
).toHaveLength(1);
916+
expect(accountManager.getAccountByIndex(0)?.enabled).toBe(false);
917+
});
918+
865919
it("returns pool exhaustion after all accounts are deactivated", async () => {
866920
const now = Date.now();
867921
const accountManager = new AccountManager(undefined, createStorage(now, 6));

test/test-model-matrix-script.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,13 +247,13 @@ describe("test-model-matrix script helpers", () => {
247247
expect(spawnSync).toHaveBeenNthCalledWith(
248248
1,
249249
"cmd.exe",
250-
["/d", "/s", "/c", "taskkill /F /T /PID 1001 >NUL 2>NUL"],
250+
["/d", "/s", "/c", "taskkill /F /T /PID 1001"],
251251
expect.objectContaining({ windowsHide: true, stdio: "ignore" }),
252252
);
253253
expect(spawnSync).toHaveBeenNthCalledWith(
254254
2,
255255
"cmd.exe",
256-
["/d", "/s", "/c", "taskkill /F /T /PID 2002 >NUL 2>NUL"],
256+
["/d", "/s", "/c", "taskkill /F /T /PID 2002"],
257257
expect.objectContaining({ windowsHide: true, stdio: "ignore" }),
258258
);
259259
} finally {

0 commit comments

Comments
 (0)