fix(codex-local): detect server-visible Codex credentials in ACP environment test (#10703)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The codex-local adapter runs Codex agents through the ACP lane and offers a "Test" button on the agent configuration page to validate the environment > - The environment test used its own ad-hoc credential probe, while real dispatch uses the shared `evaluateCodexCredentialReadiness` predicate in `codex-home.ts` > - The two paths disagreed: a user with valid Codex subscription auth in the shared, server-visible Codex home still saw "No Codex ACP credentials were detected" > - The old warning also suggested `codex login` without explaining that a `/login` in a separate Codex or chat session does not authenticate the Paperclip server process > - This pull request makes `testCodexAcpEnvironment` use the same shared readiness predicate as real dispatch and rewords the warning to name the server credential boundary > - The benefit is that the Test button now agrees with what dispatch will actually do, and the warning tells the user exactly which process needs the credentials ## Linked Issues or Issue Description No public GitHub issue exists for this bug. Related credential-handling work, not duplicates: Refs #10160 (classifies OpenAI invalid-key 401s at dispatch time) and Refs #9598 (classifies Codex refresh auth failures). Both cover dispatch-time failures; this PR fixes the pre-dispatch environment test. Description follows the bug report template: **What happened?** A user configured a Codex ACP agent and had already authenticated Codex (subscription auth present in the shared Codex home visible to the Paperclip server). Clicking "Test" on the agent configuration page still reported: `warn: No Codex ACP credentials were detected. Hint: Set OPENAI_API_KEY or run codex login before starting a Codex ACP agent.` **Expected behavior** The environment test should detect the same credentials that real agent dispatch would use. When shared managed Codex auth is available, the test should pass with an informational check instead of warning. When credentials really are missing, the warning should explain that the Paperclip server process is the one that needs them. **Steps to reproduce** 1. Run the Paperclip server as an OS user whose shared Codex home contains valid subscription `auth.json` (no `OPENAI_API_KEY` in the adapter env or server env). 2. Configure an agent with the codex-local adapter using the ACP engine. 3. Click "Test" on the agent configuration page. 4. Observe the `codex_acp_credentials_missing` warning even though dispatch would succeed. **Agent adapter(s) involved** Codex **Additional context** The confusion was amplified by the hint: users had run `/login` in a Codex chat session and assumed the server was authenticated. That login lives in a different process and home directory, so the server never saw it. ## What Changed - `testCodexAcpEnvironment` (packages/adapters/codex-local/src/server/acp.ts) now calls the shared `evaluateCodexCredentialReadiness` predicate from `codex-home.ts` instead of a local ad-hoc `hasCodexNativeCredentials` probe, so the Test button and real dispatch agree. - An explicit empty `OPENAI_API_KEY` in the adapter config env no longer falls through to the server environment key. - An externally managed `CODEX_HOME` override is now reported as its own informational check (`codex_acp_external_home_configured`). - The `codex_acp_credentials_missing` warning now says the credentials must be visible to the Paperclip server, and the hint explains that a `/login` in a separate Codex or chat session does not authenticate the server. - Removed the now-unused `hasCodexNativeCredentials` helper. - Added two regression tests: shared managed Codex auth is detected (no false warning), and the missing-credentials warning carries the new server-boundary wording. ## Verification - `pnpm --filter @paperclip/adapter-codex-local test -- src/server/acp.test.ts` — the two new tests cover the shared-home detection branch and the new warning wording; the existing ACP lane tests cover the API-key and remote-target branches. - Manual: with subscription auth in the server-visible shared Codex home and no `OPENAI_API_KEY`, the agent configuration Test now reports `codex_acp_native_auth_detected` (info) instead of `codex_acp_credentials_missing` (warn). ## Risks - Low risk. The change only affects the environment test path, not dispatch. The readiness predicate is the same one dispatch already uses, so drift between the two paths is now structurally prevented. - Behavioral shift: an explicit empty adapter `OPENAI_API_KEY` no longer silently falls back to the server env key in the test result. This matches dispatch behavior and is intentional. ## Model Used - Implementation authored by OpenAI Codex (gpt-5.5) running through the Codex ACP lane with tool use. - PR preparation, rebase onto master, and review fix-up by Anthropic Claude (Claude Code CLI agent, extended thinking, tool use). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Cody <noreply@paperclip.ing>
This commit is contained in:
Devin Foley
GitHub
parent
772fa98393
commit
bd86dbe41b
|
|
@ -68,6 +68,7 @@ const originalNodeVersion = process.version;
|
|||
const originalPaperclipHome = process.env.PAPERCLIP_HOME;
|
||||
const originalPaperclipInstanceId = process.env.PAPERCLIP_INSTANCE_ID;
|
||||
const originalCodexHome = process.env.CODEX_HOME;
|
||||
const originalOpenAiApiKey = process.env.OPENAI_API_KEY;
|
||||
|
||||
// Older/newer ISO timestamps for the copy-back monotonic (strictly-newer)
|
||||
// decision predicate, plus a subscription-shaped auth.json fixture matching the
|
||||
|
|
@ -118,6 +119,8 @@ afterEach(async () => {
|
|||
else process.env.PAPERCLIP_INSTANCE_ID = originalPaperclipInstanceId;
|
||||
if (originalCodexHome === undefined) delete process.env.CODEX_HOME;
|
||||
else process.env.CODEX_HOME = originalCodexHome;
|
||||
if (originalOpenAiApiKey === undefined) delete process.env.OPENAI_API_KEY;
|
||||
else process.env.OPENAI_API_KEY = originalOpenAiApiKey;
|
||||
await Promise.all(tempRoots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
|
|
@ -521,6 +524,99 @@ describe("codex_local ACP lane", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("detects shared managed Codex auth in ACP environment tests", async () => {
|
||||
const root = await makeTempRoot("paperclip-codex-acp-managed-auth-");
|
||||
const commandPath = path.join(root, "bin", "codex-acp");
|
||||
const sharedCodexHome = path.join(root, "shared-codex-home");
|
||||
const managedAgentHome = path.join(
|
||||
root,
|
||||
"paperclip-home",
|
||||
"instances",
|
||||
"test",
|
||||
"companies",
|
||||
"company-1",
|
||||
"agents",
|
||||
"agent-1",
|
||||
"codex-home",
|
||||
);
|
||||
await fs.mkdir(path.dirname(commandPath), { recursive: true });
|
||||
await fs.writeFile(commandPath, "#!/usr/bin/env sh\n", "utf8");
|
||||
await fs.mkdir(sharedCodexHome, { recursive: true });
|
||||
await fs.writeFile(path.join(sharedCodexHome, "auth.json"), '{"OPENAI_API_KEY":"sk-shared"}', "utf8");
|
||||
setNodeVersion("v22.13.0");
|
||||
process.env.CODEX_HOME = sharedCodexHome;
|
||||
delete process.env.OPENAI_API_KEY;
|
||||
|
||||
const result = await testCodexAcpEnvironment({
|
||||
adapterType: "codex_local",
|
||||
companyId: "company-1",
|
||||
config: {
|
||||
engine: "acp",
|
||||
cwd: root,
|
||||
agentCommand: commandPath,
|
||||
env: { CODEX_HOME: managedAgentHome, OPENAI_API_KEY: "" },
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.status).toBe("pass");
|
||||
expect(result.checks).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "codex_acp_native_auth_detected",
|
||||
level: "info",
|
||||
detail: expect.stringContaining(sharedCodexHome),
|
||||
}),
|
||||
);
|
||||
expect(result.checks).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "codex_acp_credentials_missing",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("explains the Paperclip server credential boundary when ACP auth is missing", async () => {
|
||||
const root = await makeTempRoot("paperclip-codex-acp-missing-auth-");
|
||||
const commandPath = path.join(root, "bin", "codex-acp");
|
||||
const sharedCodexHome = path.join(root, "shared-codex-home");
|
||||
const managedAgentHome = path.join(
|
||||
root,
|
||||
"paperclip-home",
|
||||
"instances",
|
||||
"test",
|
||||
"companies",
|
||||
"company-1",
|
||||
"agents",
|
||||
"agent-1",
|
||||
"codex-home",
|
||||
);
|
||||
await fs.mkdir(path.dirname(commandPath), { recursive: true });
|
||||
await fs.writeFile(commandPath, "#!/usr/bin/env sh\n", "utf8");
|
||||
await fs.mkdir(sharedCodexHome, { recursive: true });
|
||||
setNodeVersion("v22.13.0");
|
||||
process.env.CODEX_HOME = sharedCodexHome;
|
||||
delete process.env.OPENAI_API_KEY;
|
||||
|
||||
const result = await testCodexAcpEnvironment({
|
||||
adapterType: "codex_local",
|
||||
companyId: "company-1",
|
||||
config: {
|
||||
engine: "acp",
|
||||
cwd: root,
|
||||
agentCommand: commandPath,
|
||||
env: { CODEX_HOME: managedAgentHome, OPENAI_API_KEY: "" },
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.status).toBe("warn");
|
||||
expect(result.checks).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "codex_acp_credentials_missing",
|
||||
level: "warn",
|
||||
message: expect.stringContaining("Paperclip server"),
|
||||
hint: expect.stringContaining("separate Codex/chat session"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("executes through ACPX with Codex session config and ephemeral skills", async () => {
|
||||
const root = await makeTempRoot("paperclip-codex-acp-exec-");
|
||||
const skill = await createRuntimeSkill(root);
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ import { classifyCodexAuthRefreshFailure } from "./parse.js";
|
|||
import { copyBackCodexAuth } from "./codex-auth-copyback.js";
|
||||
import { buildCodexAuthInboundProvision } from "./codex-auth-merge-scripts.js";
|
||||
import {
|
||||
evaluateCodexCredentialReadiness,
|
||||
resolveSharedCodexHomeDir,
|
||||
stageCodexHomeForSync,
|
||||
} from "./codex-home.js";
|
||||
|
|
@ -491,19 +492,6 @@ function isNonEmpty(value: unknown): value is string {
|
|||
return typeof value === "string" && value.trim().length > 0;
|
||||
}
|
||||
|
||||
async function hasCodexNativeCredentials(codexHome: string): Promise<boolean> {
|
||||
const raw = await fs.readFile(path.join(codexHome, "auth.json"), "utf8").catch(() => null);
|
||||
if (!raw) return false;
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as unknown;
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return false;
|
||||
const record = parsed as Record<string, unknown>;
|
||||
return isNonEmpty(record.OPENAI_API_KEY) || isNonEmpty(record.refresh_token);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export async function testCodexAcpEnvironment(
|
||||
ctx: AdapterEnvironmentTestContext,
|
||||
): Promise<AdapterEnvironmentTestResult> {
|
||||
|
|
@ -573,34 +561,50 @@ export async function testCodexAcpEnvironment(
|
|||
});
|
||||
|
||||
const envConfig = parseObject(config.env);
|
||||
const considerHostEnv = !targetIsRemote;
|
||||
const configApiKey = envConfig.OPENAI_API_KEY;
|
||||
const hostApiKey = considerHostEnv ? process.env.OPENAI_API_KEY : undefined;
|
||||
if (isNonEmpty(configApiKey) || isNonEmpty(hostApiKey)) {
|
||||
const source = isNonEmpty(configApiKey) ? "adapter config env" : "server environment";
|
||||
checks.push({
|
||||
code: "codex_acp_openai_api_key_detected",
|
||||
level: "info",
|
||||
message: "OPENAI_API_KEY is set for Codex ACP authentication.",
|
||||
detail: `Detected in ${source}.`,
|
||||
if (!targetIsRemote) {
|
||||
const configApiKey = isNonEmpty(envConfig.OPENAI_API_KEY) ? envConfig.OPENAI_API_KEY : null;
|
||||
const hostApiKey =
|
||||
Object.prototype.hasOwnProperty.call(envConfig, "OPENAI_API_KEY")
|
||||
? null
|
||||
: isNonEmpty(process.env.OPENAI_API_KEY)
|
||||
? process.env.OPENAI_API_KEY
|
||||
: null;
|
||||
const configuredApiKey = configApiKey ?? hostApiKey;
|
||||
const configuredCodexHome = isNonEmpty(envConfig.CODEX_HOME) ? envConfig.CODEX_HOME : null;
|
||||
const credentialReadiness = await evaluateCodexCredentialReadiness({
|
||||
env: process.env,
|
||||
companyId: ctx.companyId,
|
||||
configuredCodexHome,
|
||||
configuredApiKey,
|
||||
});
|
||||
} else if (!targetIsRemote) {
|
||||
const codexHome = isNonEmpty(envConfig.CODEX_HOME)
|
||||
? envConfig.CODEX_HOME
|
||||
: path.join(process.env.HOME ?? "", ".codex");
|
||||
if (codexHome && await hasCodexNativeCredentials(codexHome)) {
|
||||
|
||||
if (credentialReadiness.ready && credentialReadiness.authMode === "api") {
|
||||
checks.push({
|
||||
code: "codex_acp_openai_api_key_detected",
|
||||
level: "info",
|
||||
message: "OPENAI_API_KEY is set for Codex ACP authentication.",
|
||||
detail: `Detected in ${configApiKey ? "adapter config env" : "server environment"}.`,
|
||||
});
|
||||
} else if (credentialReadiness.ready && !credentialReadiness.managed) {
|
||||
checks.push({
|
||||
code: "codex_acp_external_home_configured",
|
||||
level: "info",
|
||||
message: "Codex ACP will use an externally managed CODEX_HOME.",
|
||||
detail: credentialReadiness.effectiveHome,
|
||||
});
|
||||
} else if (credentialReadiness.ready) {
|
||||
checks.push({
|
||||
code: "codex_acp_native_auth_detected",
|
||||
level: "info",
|
||||
message: "Codex ACP can use Codex native authentication.",
|
||||
detail: `Credentials found in ${path.join(codexHome, "auth.json")}.`,
|
||||
detail: `Credentials are available through ${credentialReadiness.effectiveHome} or shared source ${credentialReadiness.sharedSourceHome}.`,
|
||||
});
|
||||
} else {
|
||||
checks.push({
|
||||
code: "codex_acp_credentials_missing",
|
||||
level: "warn",
|
||||
message: "No Codex ACP credentials were detected.",
|
||||
hint: "Set OPENAI_API_KEY or run `codex login` before starting a Codex ACP agent.",
|
||||
message: "No Codex ACP credentials visible to the Paperclip server were detected.",
|
||||
hint: "Set OPENAI_API_KEY in the agent adapter env, set it in the Paperclip server environment, or run `codex login` for the same OS user that runs the Paperclip server before starting a Codex ACP agent. A `/login` in a separate Codex/chat session does not authenticate the server.",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue