fix(auth): stop the API key placeholder from shadowing Oh My Pi /login credentials (#78)

Oh My Pi kept the unresolved $COMMAND_CODE_API_KEY placeholder as a literal config API key that shadowed its /login credential store and was sent as the Bearer token (401). The placeholder is now registered only on pi, where it keeps the API-key auth method and --api-key working next to OAuth; on OMP the provider omits apiKey unless a real key is configured. Host-supplied placeholders are resolved or stripped on every stream path, and the legacy generate transport uses the same rule.

Stored /login OAuth and API-key credentials, --api-key, and env keys are now covered end to end on both pi and Oh My Pi, and CI runs the pi suite against a real binary.

Co-authored-by: ebreen <ebreen@users.noreply.github.com>

tests/test-omp-compat.mjs

@@ -9,7 +9,7 @@
  */
 
 import assert from "node:assert/strict"
-import { spawn } from "node:child_process"
+import { spawn, spawnSync } from "node:child_process"
 import { accessSync, constants, mkdtempSync, rmSync } from "node:fs"
 import { createServer } from "node:http"
 import { tmpdir } from "node:os"
@@ -176,19 +176,93 @@ const address = server.address()
 const port = typeof address === "object" && address ? address.port : 0
 const apiBase = `http://127.0.0.1:${port}`
 
-function runOmp(args, timeoutMs = 30_000) {
+const agentDir = join(tempHome, ".omp", "agent")
+
+function ompEnv(overrides = {}) {
+  const env = {
+    ...process.env,
+    HOME: tempHome,
+    USERPROFILE: tempHome,
+    PI_CODING_AGENT_DIR: agentDir,
+    COMMAND_CODE_API_KEY: "mock-key",
+    COMMANDCODE_API_BASE: `${apiBase}/provider/v1`,
+    COMMANDCODE_MODELS_URL: `${apiBase}/provider/v1/models`,
+  }
+  for (const [key, value] of Object.entries(overrides)) {
+    if (value === undefined) delete env[key]
+    else env[key] = value
+  }
+  return env
+}
+
+// Same DDL OMP 18 runs for its credential store; OMP's own migration is
+// `CREATE TABLE IF NOT EXISTS`, so creating it first is safe.
+const OMP_AUTH_CREDENTIALS_DDL = `CREATE TABLE IF NOT EXISTS auth_credentials (
+  id INTEGER PRIMARY KEY AUTOINCREMENT,
+  provider TEXT NOT NULL,
+  credential_type TEXT NOT NULL,
+  data TEXT NOT NULL,
+  disabled_cause TEXT DEFAULT NULL,
+  identity_key TEXT DEFAULT NULL,
+  created_at INTEGER NOT NULL DEFAULT (CAST(strftime('%s','now') AS INTEGER)),
+  updated_at INTEGER NOT NULL DEFAULT (CAST(strftime('%s','now') AS INTEGER))
+)`
+
+/**
+ * Store a credential the way OMP's `/login` does: in the `auth_credentials`
+ * table of `agent.db`. OMP is a Bun binary, so `bun:sqlite` is always
+ * available next to it and matches the SQLite build OMP itself uses.
+ * Pass `undefined` to remove any stored Command Code credential.
+ */
+function seedOmpCredential(credential) {
+  const rows =
+    credential === undefined
+      ? []
+      : [
+          [
+            credential.type,
+            JSON.stringify(
+              credential.type === "oauth"
+                ? {
+                    access: credential.access,
+                    refresh: credential.refresh,
+                    expires: credential.expires,
+                  }
+                : { key: credential.key, source: "login" },
+            ),
+          ],
+        ]
+  const script = `
+    import { Database } from "bun:sqlite"
+    // \`bun -e\` has no script slot: argv is [bun, ...args].
+    const [dbPath, ddl, rowsJson] = process.argv.slice(1)
+    const db = new Database(dbPath)
+    db.run(ddl)
+    db.run("DELETE FROM auth_credentials WHERE provider = ?", ["commandcode"])
+    for (const [type, data] of JSON.parse(rowsJson)) {
+      db.run(
+        "INSERT INTO auth_credentials (provider, credential_type, data) VALUES (?, ?, ?)",
+        ["commandcode", type, data],
+      )
+    }
+    db.close()
+  `
+  const result = spawnSync(
+    "bun",
+    ["-e", script, join(agentDir, "agent.db"), OMP_AUTH_CREDENTIALS_DDL, JSON.stringify(rows)],
+    { env: ompEnv(), encoding: "utf-8" },
+  )
+  assert.equal(result.status, 0, result.stderr)
+}
+
+function runOmp(args, timeoutOrOptions = 30_000) {
+  const options =
+    typeof timeoutOrOptions === "number" ? { timeoutMs: timeoutOrOptions } : timeoutOrOptions
+  const timeoutMs = options.timeoutMs ?? 30_000
   return new Promise((resolve) => {
     const child = spawn(OMP_BIN, args, {
       cwd: PROJECT_DIR,
-      env: {
-        ...process.env,
-        HOME: tempHome,
-        USERPROFILE: tempHome,
-        PI_CODING_AGENT_DIR: join(tempHome, ".omp", "agent"),
-        COMMAND_CODE_API_KEY: "mock-key",
-        COMMANDCODE_API_BASE: `${apiBase}/provider/v1`,
-        COMMANDCODE_MODELS_URL: `${apiBase}/provider/v1/models`,
-      },
+      env: ompEnv(options.env),
       stdio: ["ignore", "pipe", "pipe"],
     })
     let stdout = ""
@@ -269,6 +343,65 @@ try {
   assert.equal(lastRequestBody?.model, TEST_MODEL)
   assert.ok(Array.isArray(lastRequestBody?.messages))
 
+  // OMP stores `/login` credentials in agent.db and consults them only when
+  // the extension does not install a config API key. main registered the
+  // unresolved `$COMMAND_CODE_API_KEY` placeholder, which OMP kept as a
+  // literal config override and sent as the Bearer token, so stored
+  // credentials never reached the request (401).
+  const chatArgs = ["-e", EXT_PATH, "-p", "say mock token", "--model", `commandcode/${TEST_MODEL}`]
+  const noEnvKey = { COMMAND_CODE_API_KEY: undefined, COMMANDCODE_API_KEY: undefined }
+
+  console.log("[omp-compat] stored /login OAuth credential is used when no env key exists")
+  seedOmpCredential({
+    type: "oauth",
+    access: "stored-oauth-token",
+    refresh: "stored-oauth-token",
+    expires: Date.now() + 24 * 60 * 60 * 1000,
+  })
+  requestCount = 0
+  lastRequestHeaders = {}
+  const oauthChat = await runOmp(chatArgs, { env: noEnvKey })
+  assert.equal(oauthChat.code, 0, oauthChat.stderr)
+  assert.match(oauthChat.stdout, /mock-omp-ok/)
+  assert.equal(requestCount, 1)
+  assert.equal(lastRequestHeaders.authorization, "Bearer stored-oauth-token")
+
+  console.log("[omp-compat] stored /login API key credential is used when no env key exists")
+  seedOmpCredential({ type: "api_key", key: "stored-api-key" })
+  requestCount = 0
+  lastRequestHeaders = {}
+  const apiKeyChat = await runOmp(chatArgs, { env: noEnvKey })
+  assert.equal(apiKeyChat.code, 0, apiKeyChat.stderr)
+  assert.match(apiKeyChat.stdout, /mock-omp-ok/)
+  assert.equal(requestCount, 1)
+  assert.equal(lastRequestHeaders.authorization, "Bearer stored-api-key")
+
+  console.log("[omp-compat] --api-key wins over a stored credential")
+  requestCount = 0
+  lastRequestHeaders = {}
+  const cliKeyChat = await runOmp([...chatArgs, "--api-key", "cli-key"], { env: noEnvKey })
+  assert.equal(cliKeyChat.code, 0, cliKeyChat.stderr)
+  assert.match(cliKeyChat.stdout, /mock-omp-ok/)
+  assert.equal(requestCount, 1)
+  assert.equal(lastRequestHeaders.authorization, "Bearer cli-key")
+
+  console.log("[omp-compat] COMMAND_CODE_API_KEY still works alongside a stored credential")
+  requestCount = 0
+  lastRequestHeaders = {}
+  const envKeyChat = await runOmp(chatArgs)
+  assert.equal(envKeyChat.code, 0, envKeyChat.stderr)
+  assert.match(envKeyChat.stdout, /mock-omp-ok/)
+  assert.equal(requestCount, 1)
+  assert.match(lastRequestHeaders.authorization ?? "", /^Bearer (mock-key|stored-api-key)$/)
+
+  console.log("[omp-compat] no credential at all never sends the placeholder")
+  seedOmpCredential(undefined)
+  requestCount = 0
+  lastRequestHeaders = {}
+  const noKeyChat = await runOmp(chatArgs, { env: noEnvKey })
+  assert.equal(requestCount, 0, JSON.stringify(lastRequestHeaders))
+  assert.doesNotMatch(noKeyChat.stdout + noKeyChat.stderr, /\$COMMAND_CODE_API_KEY/)
+
   console.log("[omp-compat] developer advisory reaches the legacy generate request body")
   requestCount = 0
   requestBodies = []

tests/test-pi-local.mjs

@@ -44,12 +44,20 @@ function findPiBinary() {
 
 const PI_BIN = findPiBinary()
 if (!PI_BIN) {
+  if (process.env.PI_LOCAL_REQUIRED === "1") {
+    console.error("[pi-local] FAIL - pi is required but not on PATH and PI_BIN is unset")
+    process.exit(1)
+  }
   console.log("[pi-local] SKIP — pi is not on PATH")
   process.exit(0)
 }
 
 const piCheck = spawnSync(PI_BIN, ["--help"], { stdio: "ignore" })
 if (piCheck.error) {
+  if (process.env.PI_LOCAL_REQUIRED === "1") {
+    console.error(`[pi-local] FAIL - pi failed to start: ${piCheck.error.message}`)
+    process.exit(1)
+  }
   console.log(`[pi-local] SKIP — pi failed to start: ${piCheck.error.message}`)
   process.exit(0)
 }
@@ -226,11 +234,19 @@ const env = {
   COMMANDCODE_MODELS_URL: `${apiBase}/provider/v1/models`,
 }
 
-function runPi(args, timeoutMs = 30_000) {
+function runPi(args, timeoutOrOptions = 30_000) {
+  const options =
+    typeof timeoutOrOptions === "number" ? { timeoutMs: timeoutOrOptions } : timeoutOrOptions
+  const timeoutMs = options.timeoutMs ?? 30_000
+  const childEnv = { ...env }
+  for (const [key, value] of Object.entries(options.env ?? {})) {
+    if (value === undefined) delete childEnv[key]
+    else childEnv[key] = value
+  }
   return new Promise((resolve) => {
     const child = spawn(PI_BIN, args, {
       cwd: PROJECT_DIR,
-      env,
+      env: childEnv,
       stdio: ["ignore", "pipe", "pipe"],
     })
     let stdout = ""
@@ -421,9 +437,9 @@ async function runRpcExtensionCommands(timeoutMs = 30_000) {
     stderr += chunk.toString("utf-8")
   })
 
-  const waitFor = (predicate) =>
+  const waitFor = (predicate, fromIndex = 0) =>
     new Promise((resolve, reject) => {
-      const existing = events.find(predicate)
+      const existing = events.slice(fromIndex).find(predicate)
       if (existing) {
         resolve(existing)
         return
@@ -445,17 +461,27 @@ async function runRpcExtensionCommands(timeoutMs = 30_000) {
     )
     const commandNames = commandsResponse.data?.commands?.map((command) => command.name) ?? []
 
-    send({ id: "status-before", type: "prompt", message: "/commandcode-status" })
-    await waitFor(
-      (event) => event.type === "response" && event.id === "status-before" && event.success,
-    )
-    const statusBefore = await waitFor(
-      (event) =>
-        event.type === "extension_ui_request" &&
-        event.method === "notify" &&
-        typeof event.message === "string" &&
-        event.message.includes("model count: 3"),
-    )
+    // The cached catalog registers immediately and refreshes in the
+    // background. `/commandcode-refresh` coalesces with an in-flight refresh,
+    // so the status must report the startup refresh as finished before the
+    // catalog is changed; otherwise the command reports the old catalog.
+    let statusBefore
+    for (let attempt = 0; attempt < 20; attempt += 1) {
+      const id = `status-before-${attempt}`
+      const fromIndex = events.length
+      send({ id, type: "prompt", message: "/commandcode-status" })
+      await waitFor((event) => event.type === "response" && event.id === id && event.success)
+      statusBefore = await waitFor(
+        (event) =>
+          event.type === "extension_ui_request" &&
+          event.method === "notify" &&
+          typeof event.message === "string" &&
+          event.message.includes("model count: 3"),
+        fromIndex,
+      )
+      if (/source: live[\s\S]*refresh: idle/.test(statusBefore.message)) break
+      await new Promise((resolve) => setTimeout(resolve, 100))
+    }
 
     includeRefreshedModel = true
     send({ id: "refresh", type: "prompt", message: "/commandcode-refresh" })
@@ -713,7 +739,11 @@ try {
   const offlineListOutput = offlineList.stdout || offlineList.stderr
   assert.match(offlineListOutput, /gpt-5\.4/)
   assert.match(offlineListOutput, /cc-second-model/)
-  assert.match(offlineList.stderr, /Using the cached catalog/)
+  // The cached catalog is registered before the background refresh fails, and
+  // `--list-models` exits as soon as the list is printed. Whether the refresh
+  // warning reaches stderr first depends on the host runtime (Bun flushes it,
+  // Node does not), so the warning is asserted on the print run below, which
+  // waits for the response.
 
   console.log("[pi-local] use a cached model while model discovery is offline")
   requestCount = 0
@@ -824,6 +854,75 @@ try {
     "string",
   )
 
+  // pi resolves `/login` credentials, `--api-key`, and env keys through the
+  // provider's registered auth methods. Stored credentials and `--api-key`
+  // only reach the request when the provider keeps an API-key auth method
+  // next to OAuth, so every credential source is checked without an env key.
+  const authArgs = [
+    "--no-extensions",
+    "-e",
+    EXT_PATH,
+    "-p",
+    "say mock token",
+    "--provider",
+    "commandcode",
+    "--model",
+    TEST_MODEL,
+  ]
+  const noEnvKey = { COMMAND_CODE_API_KEY: undefined, COMMANDCODE_API_KEY: undefined }
+  const authPath = join(agentDir, "auth.json")
+
+  console.log("[pi-local] stored /login OAuth credential is used when no env key exists")
+  writeFileSync(
+    authPath,
+    JSON.stringify({
+      commandcode: {
+        type: "oauth",
+        access: "stored-oauth-token",
+        refresh: "stored-oauth-token",
+        expires: Date.now() + 24 * 60 * 60 * 1000,
+      },
+    }),
+  )
+  requestCount = 0
+  lastRequestHeaders = {}
+  const oauthPrint = await runPi(authArgs, { env: noEnvKey })
+  assert.equal(oauthPrint.code, 0, oauthPrint.stderr)
+  assert.match(oauthPrint.stdout, /mock-pi-ok/)
+  assert.equal(requestCount, 1)
+  assert.equal(lastRequestHeaders.authorization, "Bearer stored-oauth-token")
+
+  console.log("[pi-local] stored /login API key credential is used when no env key exists")
+  writeFileSync(
+    authPath,
+    JSON.stringify({ commandcode: { type: "api_key", key: "stored-api-key" } }),
+  )
+  requestCount = 0
+  lastRequestHeaders = {}
+  const apiKeyPrint = await runPi(authArgs, { env: noEnvKey })
+  assert.equal(apiKeyPrint.code, 0, apiKeyPrint.stderr)
+  assert.match(apiKeyPrint.stdout, /mock-pi-ok/)
+  assert.equal(requestCount, 1)
+  assert.equal(lastRequestHeaders.authorization, "Bearer stored-api-key")
+
+  console.log("[pi-local] --api-key is used when no env key or stored credential exists")
+  rmSync(authPath, { force: true })
+  requestCount = 0
+  lastRequestHeaders = {}
+  const cliKeyPrint = await runPi([...authArgs, "--api-key", "cli-key"], { env: noEnvKey })
+  assert.equal(cliKeyPrint.code, 0, cliKeyPrint.stderr)
+  assert.match(cliKeyPrint.stdout, /mock-pi-ok/)
+  assert.equal(requestCount, 1)
+  assert.equal(lastRequestHeaders.authorization, "Bearer cli-key")
+
+  console.log("[pi-local] no credential at all never sends the placeholder")
+  requestCount = 0
+  lastRequestHeaders = {}
+  const noKeyPrint = await runPi(authArgs, { env: noEnvKey })
+  assert.notEqual(noKeyPrint.code, 0)
+  assert.equal(requestCount, 0, JSON.stringify(lastRequestHeaders))
+  assert.doesNotMatch(noKeyPrint.stdout + noKeyPrint.stderr, /\$COMMAND_CODE_API_KEY/)
+
   console.log("[pi-local] Claude request through Anthropic Messages endpoint")
   requestCount = 0
   const claudePrint = await runPi(

tests/test-pure-functions.ts

@@ -17,6 +17,7 @@ import {
   messagesToCC,
   parseStreamEventLine,
   pickCommandCodeApiKey,
+  withResolvedCommandCodeApiKey,
   projectSlugFromPath,
   textContent,
   toJsonSchema,
@@ -148,6 +149,42 @@ describe("pickCommandCodeApiKey()", () => {
   it("trims a real registry key", () => {
     assert.equal(pickCommandCodeApiKey("  real-registry-key  ", "file-key"), "real-registry-key")
   })
+
+  it("never returns a placeholder as the host fallback", () => {
+    assert.equal(pickCommandCodeApiKey(undefined, "$COMMAND_CODE_API_KEY"), undefined)
+    assert.equal(pickCommandCodeApiKey("$COMMAND_CODE_API_KEY", "$COMMANDCODE_API_KEY"), undefined)
+    assert.equal(pickCommandCodeApiKey("COMMAND_CODE_API_KEY", "COMMANDCODE_API_KEY"), undefined)
+  })
+
+  it("omits a placeholder from registerProvider when no real key is configured", () => {
+    assert.equal(pickCommandCodeApiKey(undefined, undefined), undefined)
+    assert.equal(pickCommandCodeApiKey("$COMMAND_CODE_API_KEY", undefined), undefined)
+    assert.equal(pickCommandCodeApiKey("user_real-key", undefined), "user_real-key")
+  })
+})
+
+describe("withResolvedCommandCodeApiKey()", () => {
+  it("replaces a host placeholder with the configured key", () => {
+    assert.deepEqual(
+      withResolvedCommandCodeApiKey({ apiKey: "$COMMAND_CODE_API_KEY", extra: true }, "file-key"),
+      { apiKey: "file-key", extra: true },
+    )
+  })
+
+  it("drops a placeholder when no configured key exists", () => {
+    assert.deepEqual(
+      withResolvedCommandCodeApiKey({ apiKey: "$COMMAND_CODE_API_KEY" }, undefined),
+      {
+        apiKey: undefined,
+      },
+    )
+  })
+
+  it("keeps a real host key and injects a configured key when the host omitted one", () => {
+    const options = { apiKey: "host-key" }
+    assert.equal(withResolvedCommandCodeApiKey(options, "file-key"), options)
+    assert.deepEqual(withResolvedCommandCodeApiKey(undefined, "file-key"), { apiKey: "file-key" })
+  })
 })
 
 describe("projectSlugFromPath()", () => {

tests/test-stream.ts

@@ -108,6 +108,21 @@ describe("streamCommandCode — auth", () => {
 
     assert.equal(server.lastRequestHeaders().authorization, "Bearer option-key")
   })
+
+  it("treats a blank options.apiKey like a missing one", async () => {
+    server.mockResponse({
+      type: "success",
+      events: [JSON.stringify({ type: "finish", finishReason: "stop" })],
+    })
+    const { streamCommandCode } = createTestDeps({
+      apiBase: server.baseUrl(),
+      env: { COMMAND_CODE_API_KEY: "env-key" },
+    })
+
+    await collectEvents(streamCommandCode(makeModel(), makeContext(), { apiKey: "   " }))
+
+    assert.equal(server.lastRequestHeaders().authorization, "Bearer env-key")
+  })
 })
 
 describe("streamCommandCode — successful streams", () => {