Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
21 changes: 21 additions & 0 deletions apps/server/src/provider/Drivers/CodexDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { makeCodexAdapter } from "../Layers/CodexAdapter.ts";
import * as ResetCreditCoordinator from "../Layers/resetCreditCoordinator.ts";
import {
checkCodexProviderStatus,
makeCodexAccountSwitchTracker,
makePendingCodexProvider,
probeCodexSkillsForCwd,
withCodexAppServerClient,
Expand Down Expand Up @@ -188,6 +189,16 @@ export const CodexDriver: ProviderDriver<CodexSettings, CodexDriverEnv> = {
),
);

// Live sessions keep the account they spawned with. When a probe shows a
// different login, stop them so the next send respawns app-server with
// the current credentials (the same path as Stop or the idle reaper).
const trackAccountSwitch = yield* makeCodexAccountSwitchTracker(
Effect.logInfo("Codex account changed; stopping live sessions.", { instanceId }).pipe(
// `adapter` is created below; suspend so the stop resolves it lazily.
Effect.andThen(Effect.suspend(() => adapter.stopAll())),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Drivers/CodexDriver.ts:198

adapter.stopAll() can leave an old-credential app-server running after an account switch, so subsequent turns reuse that stale session. CodexAdapter.stopAll snapshots sessions while startSession adds the runtime only after await runtime.start(), allowing an in-flight start to be missed; synchronize session creation with stopping or make stopAll await and stop in-flight starts.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Drivers/CodexDriver.ts around line 198:

`adapter.stopAll()` can leave an old-credential app-server running after an account switch, so subsequent turns reuse that stale session. `CodexAdapter.stopAll` snapshots `sessions` while `startSession` adds the runtime only after `await runtime.start()`, allowing an in-flight start to be missed; synchronize session creation with stopping or make `stopAll` await and stop in-flight starts.

),
);

// Build a managed snapshot whose settings never change — mutations come
// in as instance rebuilds from the registry rather than in-place
// updates. Pre-provide `ChildProcessSpawner` so the check fits
Expand All @@ -205,6 +216,16 @@ export const CodexDriver: ProviderDriver<CodexSettings, CodexDriverEnv> = {
{ concurrent: true },
),
),
Effect.tap((probed) =>
Comment thread
Gigioxx marked this conversation as resolved.
Resolved
trackAccountSwitch(probed.auth).pipe(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Drivers/CodexDriver.ts:220

The first trackAccountSwitch(probed.auth) call records the observed account without stopping existing sessions, so a session spawned with account A can keep using A after the initial probe observes account B. Because later probes use B as the baseline, that session is never stopped. Initialize the tracker before the adapter is usable, or make the first probe stop live sessions when its baseline is unset.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Drivers/CodexDriver.ts around line 220:

The first `trackAccountSwitch(probed.auth)` call records the observed account without stopping existing sessions, so a session spawned with account A can keep using A after the initial probe observes account B. Because later probes use B as the baseline, that session is never stopped. Initialize the tracker before the adapter is usable, or make the first probe stop live sessions when its baseline is unset.

Effect.catchCause((cause) =>
Effect.logWarning("Failed to stop Codex sessions after account change.", {
instanceId,
cause,
}),
),
),
),
Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
);
const snapshotSettings = makeProviderSnapshotSettingsSource(effectiveConfig, serverSettings);
Expand Down
49 changes: 48 additions & 1 deletion apps/server/src/provider/Layers/CodexProvider.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { assert, it } from "@effect/vitest";
import * as Effect from "effect/Effect";

import { applyPreferredCodexDefaultModel, mapCodexModelCapabilities } from "./CodexProvider.ts";
import {
applyPreferredCodexDefaultModel,
hasCodexAccountChanged,
makeCodexAccountSwitchTracker,
mapCodexModelCapabilities,
} from "./CodexProvider.ts";

it("maps current Codex model capability fields", () => {
const capabilities = mapCodexModelCapabilities({
Expand Down Expand Up @@ -171,3 +177,44 @@ it("ignores custom models that shadow a preferred slug", () => {

assert.deepStrictEqual(models.find((model) => model.isDefault)?.slug, "gpt-5.4");
});

it("detects a switch between two signed-in Codex accounts and nothing else", () => {
const alice = { status: "authenticated", type: "chatgpt", email: "alice@example.com" } as const;
const bob = { status: "authenticated", type: "chatgpt", email: "bob@example.com" } as const;
assert.isTrue(hasCodexAccountChanged(alice, bob));
assert.isFalse(
hasCodexAccountChanged(alice, { ...alice, label: "ChatGPT Pro 20x Subscription" }),
);
assert.isFalse(hasCodexAccountChanged(null, alice));
assert.isFalse(hasCodexAccountChanged(alice, { status: "unauthenticated" }));
assert.isFalse(hasCodexAccountChanged({ status: "unknown" }, alice));
});

it.effect("stops live sessions on an account switch and retries a failed stop", () =>
Effect.gen(function* () {
const alice = { status: "authenticated", type: "chatgpt", email: "alice@example.com" } as const;
const bob = { status: "authenticated", type: "chatgpt", email: "bob@example.com" } as const;
let stops = 0;
let failNextStop = false;
const track = yield* makeCodexAccountSwitchTracker(
Effect.suspend(() => {
stops++;
if (!failNextStop) return Effect.void;
failNextStop = false;
return Effect.fail("stop failed");
}),
);

yield* track(alice);
yield* track(alice);
yield* track({ status: "unauthenticated" });
assert.strictEqual(stops, 0);

failNextStop = true;
assert.strictEqual(yield* Effect.flip(track(bob)), "stop failed");
yield* track(bob);
assert.strictEqual(stops, 2);
yield* track(bob);
assert.strictEqual(stops, 2);
}),
);
41 changes: 41 additions & 0 deletions apps/server/src/provider/Layers/CodexProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as Duration from "effect/Duration";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
import * as Ref from "effect/Ref";
import * as Result from "effect/Result";
import * as Schema from "effect/Schema";
import * as Scope from "effect/Scope";
Expand Down Expand Up @@ -542,6 +543,46 @@ const makePendingCodexProvider = (
});
});

/**
* A live app-server keeps the credentials it loaded at spawn, so after
* `codex login` to another account, threads with a live process keep using
* the old one. Only a change between two signed-in accounts counts; a
* signed-out or failed probe is not a switch, and neither is a plan change
* (`label`) on the same account.
*/
export function hasCodexAccountChanged(
previous: ServerProvider["auth"] | null,
next: ServerProvider["auth"],
): boolean {
if (!previous || previous.status !== "authenticated" || next.status !== "authenticated") {
return false;
}
return previous.type !== next.type || previous.email !== next.email;
Comment thread
Gigioxx marked this conversation as resolved.
Resolved
}

/**
* Runs `stopLiveSessions` when a probe reports a different signed-in account.
* Only signed-in probes move the baseline, and a switch moves it only after the
* stop succeeds, so a logout, failed probe, or failed stop in between does not
* hide the switch from the next probe.
*/
export const makeCodexAccountSwitchTracker = <E>(stopLiveSessions: Effect.Effect<void, E>) =>
Ref.make<ServerProvider["auth"] | null>(null).pipe(
Effect.map(
(lastProbedAuth) =>
(auth: ServerProvider["auth"]): Effect.Effect<void, E> =>
auth.status !== "authenticated"
? Effect.void
: Ref.get(lastProbedAuth).pipe(
Effect.flatMap((previous) =>
hasCodexAccountChanged(previous, auth)
? stopLiveSessions.pipe(Effect.andThen(Ref.set(lastProbedAuth, auth)))
: Ref.set(lastProbedAuth, auth),
),
),
),
);

function accountProbeStatus(account: CodexAppServerProviderSnapshot["account"]): {
readonly status: Exclude<ServerProviderState, "disabled">;
readonly auth: ServerProvider["auth"];
Expand Down
Loading