fix(brains): Codex owns the MCP credential via its own OAuth login - #16
Conversation
The Codex plugin declared `bearer_token_env_var`, which Codex resolves from its own process environment when it starts the MCP server. Nothing in a Codex plugin install can set that variable — the plugin manifest has no field for a secret and `plugin add` never prompts — so a plugin-only install could not authenticate, and `codex mcp list` still reported `Bearer token` because it reports on the presence of the config key rather than the value. Its presence also switches Codex's OAuth path off entirely, so the two could not coexist. Drop the declaration and let Codex authenticate itself: `codex mcp login brains` runs a browser consent flow, registers dynamically, and stores the token in the OS keychain. `scopes` is baked rather than discovered — with no `scopes` key Codex requests every scope the server advertises, which would mint an admin-carrying token for every user. Conversation capture and the inbox keep their own bearer and become opt-in: the MCP server no longer depends on it, so a user who never sets a token still gets a working plugin. Both hooks already no-op silently in that state; the contract test now pins it. Marketplace auth policy becomes ON_USE, which is what actually happens. The contract test gains an allow-list on the server declaration's keys and a resolved-config check that asks a real Codex what it made of the file. Codex ignores keys it does not recognise instead of rejecting them, so a misspelled `scopes` would otherwise pass review and silently widen every user's token.
The README still documented `export BRAINS_API_TOKEN` as the way into Codex, with `launchctl setenv` for the desktop app. That install cannot work: Codex reads the variable from its own process environment when it starts the MCP server, and nothing in a plugin install can set it. Anyone who finds this repo directly — rather than the guided page — was following instructions that end in "not set" at startup. Rewrite it around `codex mcp login brains`, with the version check first and the consent-screen expectation spelled out. The token keeps a section of its own, under an explicit "optional" heading, as the credential for conversation capture and the inbox; the desktop `launchctl` path moves there with it, since a desktop app inherits no shell export and would otherwise silently get no capture. The version floor is 0.131 and the probe is `codex plugin --help`: `codex mcp login` shipped long before the `codex plugin` commands, so probing the login passes on versions that then fail at `marketplace add`. The contract test now pins the README the way it pins the declaration — the sign-in command, the floor, the probe, the desktop path, and that no token is presented above the optional heading. It also guards the shape of the MCP file rather than just the keys inside our entry: exactly one top-level key, exactly one server. A sibling entry — a stdio `command` server especially, which Codex would launch on the user's machine — previously passed every assertion, and the resolved-config check selected our server out of the list instead of rejecting the extra one.
…cated The README checks sliced between two headings without confirming either was found. A missing end heading gives indexOf -1, and slice(start, -1) widens the region to almost the whole file — the assertions would then read the wrong section and still pass, which is the failure mode a contract test exists to prevent. Resolve both indices first, assert each was found and that they are in order, then slice.
nir-ssvlabs
left a comment
nir-ssvlabs
left a comment
There was a problem hiding this comment.
Approving the code — it's clean, and the contract test is the strongest part.
Verified:
.mcp.jsonis now{type, url, scopes:["read","write"]}withbearer_token_env_vargone — consistent with the web-side default I reviewed inssvlabs/brains#1066, and matching the mint-side scope vocabulary (read/write, withadmin/perf_insightsas the login escape hatches). Thescopeskey is what Codex forwards to DCR (your fresh-CODEX_HOMErun against the stub AS confirmsscope=read write), so this is the field that actually drives the request, not decoration.- The contract test pins the right invariants and can't silently rot: bearer-absence (with the "presence disables OAuth login" reason recorded), exact
scopes, strict key allow-lists on both the file and the server object (a stdio/http sibling can't ride along), the resolved-config check against a real Codex, and no-token hooks exiting 0 silently — load-bearing now that capture/inbox are opt-in. - Versions bump uniformly
2.3.3 → 2.4.0(correct increment from the #13 that just merged), andmarketplace.json→ON_USEmatches the sign-in-on-use model. Hooks are untouched and still key offBRAINS_API_TOKENindependently.
ssvlabs/brains#1066 is still open on stage and is not in the 2026-07-28 release batch, so the updated /install/codex-cli guide is not live on prod. Since merging this force-reinstalls HEAD-tracking Codex installs at their next launch, it must land only after #1066 reaches main — exactly the sequence you documented. My approval is on code readiness; please hold the merge until the guide is live.
Nothing critical in the diff itself.
RaekwonIII
left a comment
RaekwonIII
left a comment
There was a problem hiding this comment.
Reviewed the pair together; the server-side and cross-repo claims check out. Not approving yet — one thing I'd like addressed first.
Main concern — the strongest test only runs locally / at release
The resolved-config check (asking a real Codex what it made of the declaration) is the one assertion that catches an upstream Codex change that stops honouring the plugin MCP shape — the static key/scope asserts can all stay green while resolution silently breaks or falls back onto the bearer path. But CI is bun-only with no codex on PATH, so it SKIPs there and only runs locally and at release.
That's disclosed, and I'm not necessarily asking CI to install codex — but I'd like the gap closed or bounded before this merges, since this merge is the go-live switch for every HEAD-tracking install. Options:
- Run it in a release-gate job against a pinned/known Codex version (doesn't have to be per-PR).
- Make the skip loud at release — fail if the check didn't actually execute, so "skipped" can't masquerade as "passed" on the one merge that migrates every install.
- Or document explicitly who runs it, and against which codex version, at go-live.
I'd want the resolved-config check to have definitely run against a real codex before this lands — not just be present in the suite.
What checks out (verified against ssvlabs/brains)
- Scopes are real server-side:
read | write | admin | perf_insightsare the exact recognized set (packages/mcp-auth), and DCR + loopback (127.0.0.1) redirect are supported (apps/mcp/src/oauth.ts). - The scope pin is load-bearing, not cosmetic: the server maps no/unknown scopes to an unscoped = full-access token, so dropping
scopesreally would request every advertised scope and mint an admin-carrying token for every user. Baking["read","write"]+ asserting it exactly is a genuine control. ON_USEand the bearer-absence invariant are reasoned correctly.
Nice work on the contract-test hardening — the file-shape guards (only mcpServers / only brains, no stdio sibling) and the no-token silent-exit assertions are the right instincts.
|
@RaekwonIII Went with your third option — documentation, no new machinery. The "Do not merge" section in the description now names the step: before this merges, the merger runs the contract suite locally with codex ≥ 0.131 on PATH and confirms Deliberately no CI change: PRs here are rare, so PR-triggered CI can't catch upstream drift in time anyway, and an unpinned codex install would make every unrelated PR inherit OpenAI's release quality. If upstream ever breaks resolution once, a scheduled job is the first follow-up. |
stefan-ssv-labs
left a comment
stefan-ssv-labs
left a comment
There was a problem hiding this comment.
✅ review-pr: clean against BRNS-DIST-012 — the plugin resolves through Codex OAuth without a bearer env var; merge only after ssvlabs/brains#1066 is live on production.
RaekwonIII
left a comment
RaekwonIII
left a comment
There was a problem hiding this comment.
Thanks — this addresses it. The go-live checklist step makes the skip load-bearing: requiring resolved-config check OK (never a silent SKIP) at merge time is exactly the guarantee I was after, and putting it at the go-live gate rather than per-PR is the right call. Agreed on not adding CI — rare PRs mean PR-triggered runs wouldn't catch upstream drift anyway, and an unpinned codex would tie every unrelated PR to OpenAI's release quality; a scheduled job is the sensible escalation if resolution ever breaks once.
Approving, with the standing caveat already in the description: #1066 to prod first, then the local real-codex contract run (resolved-config check OK) + prod login smoke, then merge this.
4 participants
Summary
plugins/brains/.mcp.jsondropsbearer_token_env_varand declaresscopes: ["read", "write"]— Codex signs itself in viacodex mcp login brains(browser consent, dynamic client registration, credential kept by Codex). The env-var declaration made Codex fail hard at startup when the variable was missing — nothing in a plugin install can set it — and its presence also disabled the OAuth login path entirely.codex plugin --helpprobe, marketplace add, plugin add,codex mcp login brains, the consent-screen expectation, and the scope escape hatches. The token moves under "Optional: conversation capture and the inbox" with both the shell-export and desktoplaunchctlpaths..agents/plugins/marketplace.jsonauthentication →ON_USE; the contract-test comment records the desktop app-server nuance so it isn't flipped back on a shallow re-derivation.mcpServers, onlybrains— a stdio sibling can't ride along), a resolved-config check against a real Codex (skips visibly when codex isn't on PATH), no-token hooks must exit 0 silently (load-bearing now that capture/inbox are opt-in), and README pins with hardened section delimiters.Hooks are unchanged: capture + the inbox still read
BRAINS_API_TOKEN, now as an opt-in rather than the price of admission.Test
scopetypo, bearer returning, stray ignored key,ON_INSTALLrestored, http sibling, stdiocommandsibling, stray top-level key, README export-as-prerequisite, README droppinglaunchctl, README floor regression, README missing Claude heading, README section reorder.CODEX_HOMEend to end against a stub AS: DCR sendsclient_name: "Codex"+scope=read write, login succeeds,codex mcp listreads OAuth, and MCP startup presents the stored Bearer with no env var.Known gap: CI is bun-only, so the resolved-config check skips there (it runs locally and at release).
Do not merge before the guide is live on prod
Codex auto-refreshes git marketplaces and force-reinstalls plugins unconditionally, so this merge migrates HEAD-tracking installs at their next launch — it is the go-live switch, not a staging step. Sequence: ssvlabs/brains#1066 → stage → released to main (guide live) → merge this. The small set of existing token-based installs is notified directly when this merges.
Before merging: run
bun run tests/plugin-contract/run.tslocally with codex ≥ 0.131 on PATH and confirm it printsresolved-config check OK— the suite prints either that or a visible SKIP, so the evidence is unambiguous. Last executed against codex-cli 0.145.0.BRNS-DIST-012.