rollup: integrate open fix PRs into a single review branch

lib/accounts.ts

@@ -866,6 +866,16 @@ export class AccountManager {
 	): void {
 		account.lastSwitchReason = reason;
 		this.currentAccountIndexByFamily[family] = account.index;
+		// HI-02: keep cursorByFamily in lockstep so subsequent round-robin
+		// passes resume AFTER the just-switched account, matching the
+		// convention used in getCurrentOrNextForFamilyHybrid / the
+		// getCurrentOrNextForFamily inner loop. Without this, the cursor
+		// still points at the pre-switch position and the next selection
+		// can re-pick or skip the freshly marked account.
+		const count = this.accounts.length;
+		if (count > 0) {
+			this.cursorByFamily[family] = (account.index + 1) % count;
+		}
 	}
 
 	/**
@@ -900,6 +910,13 @@ export class AccountManager {
 		return withRoutingMutex(this.routingMutexMode, () => {
 			account.lastSwitchReason = reason;
 			this.currentAccountIndexByFamily[family] = account.index;
+			// HI-02: keep cursorByFamily in lockstep with the active-index
+			// mutation so the mutex-serialized variant preserves the same
+			// round-robin invariant as the legacy `markSwitched` path.
+			const count = this.accounts.length;
+			if (count > 0) {
+				this.cursorByFamily[family] = (account.index + 1) % count;
+			}
 			const trackerKey = getRuntimeTrackerKey(account);
 			const healthTracker = getHealthTracker();
 			const tokenTracker = getTokenTracker();
@@ -1316,16 +1333,38 @@ export class AccountManager {
 
 		// Snapshot family pointers before splice so we can distinguish "was
 		// pointing at the removed account" from "was pointing past it".
-		const priorCursor: Record<ModelFamily, number> = {} as Record<ModelFamily, number>;
-		const priorActive: Record<ModelFamily, number> = {} as Record<ModelFamily, number>;
+		const priorCursor: Record<ModelFamily, number> = {} as Record<
+			ModelFamily,
+			number
+		>;
+		const priorActive: Record<ModelFamily, number> = {} as Record<
+			ModelFamily,
+			number
+		>;
 		for (const family of MODEL_FAMILIES) {
 			priorCursor[family] = this.cursorByFamily[family];
 			priorActive[family] = this.currentAccountIndexByFamily[family];
 		}
 
 		this.accounts.splice(idx, 1);
+		// Clear numeric-keyed tracker state in the shifted range. After reindex,
+		// any refresh-only account that moved from N to N-1 must not inherit the
+		// stale health/token entries that used to belong to the old numeric slot.
+		getHealthTracker().clearNumericKeysAtOrAbove(idx);
+		getTokenTracker().clearNumericKeysAtOrAbove(idx);
 		this.accounts.forEach((acc, index) => {
 			acc.index = index;
+			// Invalidate the cached runtime tracker key when it was keyed by
+			// numeric index (fallback path in getRuntimeAccountIdentityKey).
+			// After the splice+reindex above, a remaining account that was at
+			// index N (e.g. 3) may now live at index N-1 (e.g. 2); if we keep
+			// the previously cached numeric key, rotation/health/token state
+			// queries would consult the stale position and mismatch the
+			// current one. Identity-based string keys remain stable because
+			// accountId/email are not affected by array position changes.
+			if (typeof acc._runtimeTrackerKey === "number") {
+				acc._runtimeTrackerKey = undefined;
+			}
 		});
 
 		if (this.accounts.length === 0) {

lib/auth/auth.ts

@@ -66,6 +66,98 @@ function getOAuthResponseLogMetadata(
 	return { responseType: typeof rawResponse };
 }
 
+const OAUTH_SENSITIVE_BODY_KEYS = [
+	"refresh_token",
+	"refreshToken",
+	"access_token",
+	"accessToken",
+	"id_token",
+	"idToken",
+	"codeVerifier",
+	"token",
+	"code",
+	"code_verifier",
+] as const;
+
+function scrubTokenLikeSubstrings(value: string): string {
+	let scrubbed = value.replace(
+		/(\b(?:refresh|access|id)[_-]?token\s*[:=]\s*)([^\s,;"'}]{8,})/gi,
+		(_match, prefix) => `${prefix}***REDACTED***`,
+	);
+	scrubbed = scrubbed.replace(
+		/\b(?:RT|AT)_ch_[A-Za-z0-9_-]{20,}\b/g,
+		"***REDACTED***",
+	);
+	return scrubbed;
+}
+
+function redactSensitiveFields(value: unknown): unknown {
+	if (Array.isArray(value)) {
+		return value.map((item) => redactSensitiveFields(item));
+	}
+	if (value !== null && typeof value === "object") {
+		const out: Record<string, unknown> = {};
+		const sensitiveSet = new Set<string>(OAUTH_SENSITIVE_BODY_KEYS);
+		for (const [k, v] of Object.entries(value as Record<string, unknown>)) {
+			if (sensitiveSet.has(k)) {
+				out[k] = "***REDACTED***";
+			} else {
+				out[k] = redactSensitiveFields(v);
+			}
+		}
+		return out;
+	}
+	if (typeof value === "string") {
+		return scrubTokenLikeSubstrings(value);
+	}
+	return value;
+}
+
+/**
+ * Scrub opaque tokens from a raw OAuth token-endpoint response body before
+ * interpolating it into a log message.
+ *
+ * Error and success responses from `/oauth/token` may contain `refresh_token`,
+ * `access_token`, or `id_token` values. ChatGPT refresh tokens are opaque
+ * high-entropy strings that do NOT match the logger's `TOKEN_PATTERNS`
+ * (JWT, long hex, `sk-*`, `Bearer <x>`), so they would be written verbatim
+ * to disk log files when a status-body string is concatenated into a
+ * `logError` message.
+ *
+ * Strategy:
+ *   1. If the body parses as JSON, walk it and mask sensitive keys.
+ *   2. Otherwise fall back to a targeted regex scrub of `"key":"value"` and
+ *      `key=value` patterns for the known sensitive keys.
+ *
+ * The returned string is safe to interpolate into log messages.
+ */
+export function sanitizeOAuthResponseBodyForLog(rawBody: string): string {
+	if (!rawBody) return rawBody;
+	const trimmed = rawBody.trim();
+	if (trimmed.length === 0) return rawBody;
+
+	if (trimmed.startsWith("{") || trimmed.startsWith("[")) {
+		try {
+			const parsed = JSON.parse(trimmed) as unknown;
+			const redacted = redactSensitiveFields(parsed);
+			return JSON.stringify(redacted);
+		} catch {
+			// Fall through to regex scrub for malformed JSON.
+		}
+	}
+
+	let scrubbed = rawBody;
+	for (const key of OAUTH_SENSITIVE_BODY_KEYS) {
+		// "key":"value" style (JSON-like text)
+		const jsonPattern = new RegExp(`("${key}"\\s*:\\s*)"[^"]*"`, "g");
+		scrubbed = scrubbed.replace(jsonPattern, `$1"***REDACTED***"`);
+		// key=value style (urlencoded / query-string)
+		const urlPattern = new RegExp(`(^|[?&\\s])(${key}=)[^&\\s]+`, "g");
+		scrubbed = scrubbed.replace(urlPattern, `$1$2***REDACTED***`);
+	}
+	return scrubbed;
+}
+
 /**
  * Redacts sensitive OAuth query parameters for safe logging.
  * Returns the original string when parsing fails.
@@ -160,12 +252,13 @@ export async function exchangeAuthorizationCode(
 	});
 	if (!res.ok) {
 		const text = await res.text().catch(() => "");
-		logError(`code->token failed: ${res.status} ${text}`);
+		const safeText = sanitizeOAuthResponseBodyForLog(text);
+		logError(`code->token failed: ${res.status} ${safeText}`);
 		return {
 			type: "failed",
 			reason: "http_error",
 			statusCode: res.status,
-			message: text || undefined,
+			message: safeText || undefined,
 		};
 	}
 	const rawJson = (await res.json()) as unknown;
@@ -252,12 +345,13 @@ export async function refreshAccessToken(
 
 		if (!response.ok) {
 			const text = await response.text().catch(() => "");
-			logError(`Token refresh failed: ${response.status} ${text}`);
+			const safeText = sanitizeOAuthResponseBodyForLog(text);
+			logError(`Token refresh failed: ${response.status} ${safeText}`);
 			return {
 				type: "failed",
 				reason: "http_error",
 				statusCode: response.status,
-				message: text || undefined,
+				message: safeText || undefined,
 			};
 		}
 

lib/auth/server.ts

@@ -66,14 +66,18 @@ export function startLocalOAuthServer({
 				"default-src 'none'; style-src 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:; script-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'",
 			);
 			res.end(successHtml);
-			const trackedServer = server as http.Server & { _lastCode?: string };
+			const trackedServer = server as http.Server & {
+				_lastCode?: string;
+				_lastState?: string;
+			};
 			if (trackedServer._lastCode) {
 				logWarn(
 					"Duplicate OAuth callback received; preserving first authorization code",
 				);
 				return;
 			}
 			trackedServer._lastCode = code;
+			trackedServer._lastState = state;
 		} catch (err) {
 			logError(
 				`Request handler error: ${(err as Error)?.message ?? String(err)}`,
@@ -95,17 +99,28 @@ export function startLocalOAuthServer({
 						pollAborted = true;
 						server.close();
 					},
-					waitForCode: async () => {
+					waitForCode: async (expectedState: string) => {
 						const POLL_INTERVAL_MS = 100;
 						const TIMEOUT_MS = 5 * 60 * 1000;
 						const maxIterations = Math.floor(TIMEOUT_MS / POLL_INTERVAL_MS);
 						const poll = () =>
 							new Promise<void>((r) => setTimeout(r, POLL_INTERVAL_MS));
 						for (let i = 0; i < maxIterations; i++) {
 							if (pollAborted) return null;
-							const lastCode = (server as http.Server & { _lastCode?: string })
-								._lastCode;
-							if (lastCode) return { code: lastCode };
+							const trackedServer = server as http.Server & {
+								_lastCode?: string;
+								_lastState?: string;
+							};
+							const lastCode = trackedServer._lastCode;
+							if (lastCode) {
+								if (trackedServer._lastState !== expectedState) {
+									logWarn(
+										"Discarding OAuth callback due to state mismatch in waitForCode",
+									);
+									return null;
+								}
+								return { code: lastCode };
+							}
 							await poll();
 						}
 						logWarn("OAuth poll timeout after 5 minutes");
@@ -130,7 +145,7 @@ export function startLocalOAuthServer({
 							);
 						}
 					},
-					waitForCode: () => Promise.resolve(null),
+					waitForCode: (_expectedState: string) => Promise.resolve(null),
 				});
 			});
 	});

lib/codex-manager.ts

@@ -1360,8 +1360,8 @@ async function promptManualCallback(
 			return null;
 		}
 		const parsed = parseAuthorizationInput(answer);
-		if (!parsed.code) return null;
-		if (parsed.state && parsed.state !== state) return null;
+		if (!parsed.code || !parsed.state) return null;
+		if (parsed.state !== state) return null;
 		return parsed.code;
 	} catch (error) {
 		if (isAbortError(error) || isReadlineClosedError(error)) {