Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
16 changes: 15 additions & 1 deletion plugins/codex/scripts/lib/app-server.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const PLUGIN_MANIFEST_URL = new URL("../../.claude-plugin/plugin.json", import.m
const PLUGIN_MANIFEST = JSON.parse(fs.readFileSync(PLUGIN_MANIFEST_URL, "utf8"));

export const BROKER_ENDPOINT_ENV = "CODEX_COMPANION_APP_SERVER_ENDPOINT";
export const BROKER_DISABLE_ENV = "CODEX_COMPANION_APP_SERVER_DISABLE_BROKER";
export const BROKER_BUSY_RPC_CODE = -32001;

/** @type {ClientInfo} */
Expand Down Expand Up @@ -332,10 +333,23 @@ class BrokerCodexAppServerClient extends AppServerClientBase {
}
}

export function resolveDisableBroker(options) {
if (options.disableBroker !== undefined) {
return Boolean(options.disableBroker);
}
const raw = options.env?.[BROKER_DISABLE_ENV] ?? process.env[BROKER_DISABLE_ENV] ?? null;
if (raw === null || raw === undefined) {
return false;
}
const normalized = String(raw).trim().toLowerCase();
return normalized === "true" || normalized === "1" || normalized === "yes";
}

export class CodexAppServerClient {
static async connect(cwd, options = {}) {
const disableBroker = resolveDisableBroker(options);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Honor the disable flag in runtime status

When this environment variable is enabled while either CODEX_COMPANION_APP_SERVER_ENDPOINT or a saved broker session exists, connect() correctly chooses the direct transport, but getSessionRuntimeStatus() in plugins/codex/scripts/lib/codex.mjs:906-914 still reports a “shared session” because it only checks for an endpoint. Consequently, /codex:status and setup output claim jobs are reusing the broker even though every connection bypasses it; apply the same disable-value resolution when determining the displayed runtime mode.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Applied: getSessionRuntimeStatus() now calls resolveDisableBroker() so /codex:status reports direct mode whenever CODEX_COMPANION_APP_SERVER_DISABLE_BROKER is set, even if a broker endpoint or saved session exists. Added tests/codex.test.mjs covering disable-wins-over-endpoint and the default cases.

let brokerEndpoint = null;
if (!options.disableBroker) {
if (!disableBroker) {
brokerEndpoint = options.brokerEndpoint ?? options.env?.[BROKER_ENDPOINT_ENV] ?? process.env[BROKER_ENDPOINT_ENV] ?? null;
if (!brokerEndpoint && options.reuseExistingBroker) {
brokerEndpoint = loadBrokerSession(cwd)?.endpoint ?? null;
Expand Down
11 changes: 10 additions & 1 deletion plugins/codex/scripts/lib/codex.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import os from "node:os";
import path from "node:path";

import { readJsonFile } from "./fs.mjs";
import { BROKER_BUSY_RPC_CODE, BROKER_ENDPOINT_ENV, CodexAppServerClient } from "./app-server.mjs";
import { BROKER_BUSY_RPC_CODE, BROKER_ENDPOINT_ENV, CodexAppServerClient, resolveDisableBroker } from "./app-server.mjs";
import { loadBrokerSession } from "./broker-lifecycle.mjs";
import { binaryAvailable } from "./process.mjs";

Expand Down Expand Up @@ -904,6 +904,15 @@ export function getCodexAvailability(cwd) {
}

export function getSessionRuntimeStatus(env = process.env, cwd = process.cwd()) {
if (resolveDisableBroker({ env })) {
return {
mode: "direct",
label: "direct startup",
detail: "Broker discovery is disabled by CODEX_COMPANION_APP_SERVER_DISABLE_BROKER; each command starts its own Codex runtime.",
endpoint: null
};
}

const endpoint = env?.[BROKER_ENDPOINT_ENV] ?? loadBrokerSession(cwd)?.endpoint ?? null;
if (endpoint) {
return {
Expand Down
44 changes: 44 additions & 0 deletions tests/app-server.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import test from "node:test";
import assert from "node:assert/strict";

import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs";
import { makeTempDir } from "./helpers.mjs";
import { BROKER_DISABLE_ENV, CodexAppServerClient } from "../plugins/codex/scripts/lib/app-server.mjs";

test("CodexAppServerClient.connect spawns directly when CODEX_COMPANION_APP_SERVER_DISABLE_BROKER is true", async () => {
const binDir = makeTempDir();
installFakeCodex(binDir);
const cwd = makeTempDir();
const env = { ...buildEnv(binDir), [BROKER_DISABLE_ENV]: "true" };

const client = await CodexAppServerClient.connect(cwd, { env });

assert.equal(client.transport, "direct");
await client.close();
});

test("CodexAppServerClient.connect accepts 1 as a truthy disableBroker env value", async () => {
const binDir = makeTempDir();
installFakeCodex(binDir);
const cwd = makeTempDir();
const env = { ...buildEnv(binDir), [BROKER_DISABLE_ENV]: "1" };

const client = await CodexAppServerClient.connect(cwd, { env });

assert.equal(client.transport, "direct");
await client.close();
});

test("CodexAppServerClient.connect treats an explicit disableBroker option as an override", async () => {
const binDir = makeTempDir();
installFakeCodex(binDir);
const cwd = makeTempDir();
// Even though the env var is set, an explicit option wins.
const client = await CodexAppServerClient.connect(cwd, {
env: buildEnv(binDir),
disableBroker: true
});

assert.equal(client.transport, "direct");
await client.close();
});
50 changes: 50 additions & 0 deletions tests/codex.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import test from "node:test";
import assert from "node:assert/strict";

import { getSessionRuntimeStatus } from "../plugins/codex/scripts/lib/codex.mjs";
import { BROKER_DISABLE_ENV } from "../plugins/codex/scripts/lib/app-server.mjs";
import { makeTempDir } from "./helpers.mjs";

test("getSessionRuntimeStatus reports direct mode when disable-broker env is set", () => {
const cwd = makeTempDir();
const env = { [BROKER_DISABLE_ENV]: "true" };

const status = getSessionRuntimeStatus(env, cwd);

assert.equal(status.mode, "direct");
assert.equal(status.endpoint, null);
assert.ok(status.detail.includes("disabled by CODEX_COMPANION_APP_SERVER_DISABLE_BROKER"));
});

test("getSessionRuntimeStatus honors disable-broker env even when endpoint is set", () => {
const cwd = makeTempDir();
const env = {
[BROKER_DISABLE_ENV]: "1",
CODEX_COMPANION_APP_SERVER_ENDPOINT: "tcp://127.0.0.1:12345"
};

const status = getSessionRuntimeStatus(env, cwd);

assert.equal(status.mode, "direct");
assert.equal(status.endpoint, null);
});

test("getSessionRuntimeStatus reports shared mode when endpoint is set and disable is unset", () => {
const cwd = makeTempDir();
const env = {
CODEX_COMPANION_APP_SERVER_ENDPOINT: "tcp://127.0.0.1:12345"
};

const status = getSessionRuntimeStatus(env, cwd);

assert.equal(status.mode, "shared");
assert.equal(status.endpoint, "tcp://127.0.0.1:12345");
});

test("getSessionRuntimeStatus reports direct mode when no endpoint or disable flag is set", () => {
const cwd = makeTempDir();
const status = getSessionRuntimeStatus({}, cwd);

assert.equal(status.mode, "direct");
assert.equal(status.endpoint, null);
});