fix(plugin/codex): allow empty api_key (unauthenticated local OV)#2023
Merged
ZaynJarvis merged 3 commits intoMay 13, 2026
Conversation
Reported: with an ovcli.conf that has no `api_key` (typical local OV without auth), the plugin would not start cleanly. Root cause: .mcp.json ships with `bearer_token_env_var: "OPENVIKING_API_KEY"`, and when that env var resolves to an empty string at codex launch (because ovcli.conf has no key), Codex interprets it as "auth configured but not provided" and falls back to its OAuth dance — which then fails against an OV that doesn't speak OAuth. Hook side is unaffected: scripts/config.mjs already gates the Bearer header on `if (cfg.apiKey)`, so empty api_key → no Authorization header sent → OV accepts in unauth mode. Verified end-to-end with auto-recall against `http://127.0.0.1:1933` and an empty-key ovcli.conf. Fix: at install time, detect whether ANY api_key is configured (env or ovcli.conf) and conditionally render `.mcp.json` *with or without* `bearer_token_env_var`: - api_key present → keep `bearer_token_env_var: "OPENVIKING_API_KEY"` - api_key absent → drop the field entirely (Codex will then just hit OV without Authorization and treat 200 as success) Implementation uses node (already required) to read/edit the cached .mcp.json as proper JSON rather than sed, so we don't have to worry about field-position-dependent regexes. Installer footer now also reports the resolved auth mode so the user sees `MCP auth: Bearer (OPENVIKING_API_KEY)` vs `MCP auth: none (unauthenticated)` at the end of the run. env_http_headers stays in both modes — identity headers (X-OpenViking-Account / User / Agent) are independent of auth and OV accepts empty values (defaults to "default").
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Codex memory plugin installer to support unauthenticated local OpenViking setups by avoiding Codex’s OAuth fallback when an API key is intentionally absent/empty, and documents the behavior in the plugin docs.
Changes:
- Add install-time API key detection and conditionally render cached
.mcp.jsonwithoutbearer_token_env_varin no-auth mode. - Replace the previous
sed-based.mcp.jsonURL substitution with a Node-based JSON rewrite (also used to drop the bearer field). - Document the unauthenticated-local behavior in the plugin README and the EN/ZH Codex integration docs, and print the resolved auth mode in the installer footer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| examples/codex-memory-plugin/setup-helper/install.sh | Detects presence of API key and rewrites cached .mcp.json accordingly; prints resolved MCP auth mode. |
| examples/codex-memory-plugin/README.md | Documents why .mcp.json omits bearer_token_env_var in unauthenticated local mode. |
| docs/en/agent-integrations/04-codex.md | Adds explanation of unauthenticated local OV behavior and why the bearer env var field is omitted. |
| docs/zh/agent-integrations/04-codex.md | Same as EN docs, in Chinese. |
Comments suppressed due to low confidence (1)
examples/codex-memory-plugin/setup-helper/install.sh:351
- The install footer prints
MCP auth: Bearer (OPENVIKING_API_KEY)wheneverHAS_API_KEY=1, butHAS_API_KEYcan be true due toOPENVIKING_BEARER_TOKEN. In that case the message is misleading and, with the current.mcp.jsonrendering, MCP auth likely won’t work. Track which env var/source is actually used for MCP auth (or derive it during rendering) and print that instead.
Marketplace: $MARKETPLACE_ROOT
Plugin cache: $CACHE_DIR
MCP endpoint: $MCP_URL
MCP auth: $([ "$HAS_API_KEY" = "1" ] && echo "Bearer (OPENVIKING_API_KEY)" || echo "none (unauthenticated)")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+203
to
+235
| detect_api_key() { | ||
| if [ -n "${OPENVIKING_API_KEY:-}" ] || [ -n "${OPENVIKING_BEARER_TOKEN:-}" ]; then | ||
| echo "1" | ||
| return | ||
| fi | ||
| if [ -f "$OVCLI_CONF" ]; then | ||
| node -e ' | ||
| try { | ||
| const c = JSON.parse(require("node:fs").readFileSync(process.argv[1], "utf8")); | ||
| process.stdout.write(c.api_key ? "1" : "0"); | ||
| } catch { process.stdout.write("0"); } | ||
| ' "$OVCLI_CONF" 2>/dev/null || echo "0" | ||
| return | ||
| fi | ||
| echo "0" | ||
| } | ||
| HAS_API_KEY="$(detect_api_key)" | ||
|
|
||
| # Render the OpenViking /mcp URL into the cached .mcp.json (and drop the | ||
| # bearer_token_env_var line in no-auth mode). The repo's checked-in | ||
| # .mcp.json keeps the placeholder + always-present bearer field; the cache | ||
| # copy is what Codex actually loads. | ||
| MCP_JSON="$CACHE_DIR/.mcp.json" | ||
| if [ -f "$MCP_JSON" ]; then | ||
| MCP_URL_ESC="$(printf '%s' "$MCP_URL" | sed -e 's/[\\/&]/\\&/g')" | ||
| sed -i.bak -e "s|__OPENVIKING_MCP_URL__|$MCP_URL_ESC|g" "$MCP_JSON" | ||
| rm -f "${MCP_JSON}.bak" | ||
| node - "$MCP_JSON" "$MCP_URL" "$HAS_API_KEY" <<'NODE' | ||
| const fs = require("node:fs"); | ||
| const [, , file, url, hasKey] = process.argv; | ||
| const j = JSON.parse(fs.readFileSync(file, "utf8")); | ||
| const s = j.mcpServers["openviking-memory"]; | ||
| s.url = url; | ||
| if (hasKey !== "1") { | ||
| delete s.bearer_token_env_var; | ||
| } |
|
|
||
| Auth is sent as `Authorization: Bearer <api_key>` to both the REST API (used by hooks) and the `/mcp` endpoint (used by the model). | ||
|
|
||
| For **unauthenticated local OV** (`ovcli.conf` without `api_key`, or no ovcli.conf at all), the installer renders `.mcp.json` *without* `bearer_token_env_var`. This is intentional — if `bearer_token_env_var` were present but `OPENVIKING_API_KEY` were unset/empty, Codex would interpret that as "auth required but not configured" and fall back to its OAuth dance, breaking unauth mode. The shell-function wrapper still exports identity headers (`OPENVIKING_ACCOUNT` / `_USER` / `_AGENT_ID`) via `env_http_headers` so multi-tenant identity still works. |
Reported: setting OPENVIKING_CLI_CONFIG_FILE=ovcli-local.conf (a config
without api_key, for benchmark-memory isolation) and running codex fails
with:
Environment variable OPENVIKING_API_KEY for MCP server 'openviking-memory'
is empty
Two issues stacked on top of each other:
1. Codex 0.130 hard-fails MCP startup when bearer_token_env_var resolves
to an EMPTY env var (confirmed empirically — not OAuth fallback, just
a startup error).
2. The previous codex() wrapper exported `OPENVIKING_API_KEY=""` via the
inline-prefix syntax `OPENVIKING_API_KEY="${...:-${...:-}}" codex`,
which sets the variable to an empty string when no key is resolvable.
So even my prior fix (don't render bearer_token_env_var when no key
at install time) didn't help users who install with one conf and run
with another via OPENVIKING_CLI_CONFIG_FILE.
Fix is two parts:
a) Build the env prefix dynamically into a bash array, skipping any
OPENVIKING_* whose resolved value is empty. So an empty api_key
produces no OPENVIKING_API_KEY at all in codex's env — neither
set-to-empty nor set-to-something.
b) Have the wrapper re-render the cached .mcp.json's bearer_token_env_var
on every codex launch based on the currently-active ovcli.conf. The
idempotent fast-path skips writing when the desired state already
matches. This makes swapping configs at runtime (typical benchmark
isolation workflow) work without re-running the installer.
The wrapper now uses `env "${_env_args[@]}" codex "$@"` instead of the
inline-prefix form for the same reason — proper handling of conditional
env-var presence.
Manual setup snippets in README + docs (en/zh) updated to the same
empty-aware pattern; the cache-rendering bit is left to the installer-
emitted wrapper since it's noisy and only needed when actually swapping
configs.
Validated with synthetic test:
ovcli-local.conf (no api_key)
→ env passed to codex: URL=..., ACCOUNT=..., USER=..., AGENT_ID=codex
(no OPENVIKING_API_KEY at all)
→ cache .mcp.json rewritten to drop bearer_token_env_var
ovcli.conf (with api_key)
→ env passed to codex: URL=..., API_KEY=..., ACCOUNT=..., USER=..., AGENT_ID=codex
→ cache .mcp.json rewritten to re-add bearer_token_env_var
Idempotent: re-render with same hasKey state does not bump file mtime.
Previously the codex() wrapper only re-rendered bearer_token_env_var
based on the active ovcli.conf, but the cached .mcp.json URL stayed
whatever was baked at install time. Result: swapping
OPENVIKING_CLI_CONFIG_FILE to a config that points at a different OV
server (e.g. localhost) would still hit the install-time URL —
typically the remote production OV — and fail auth.
Reported in testing:
OPENVIKING_CLI_CONFIG_FILE=ovcli-local.conf codex
# ovcli-local.conf: { "url": "http://127.0.0.1:1933" }
# cache .mcp.json still says url=https://ov-dev.tosaki.top/mcp
# Codex hits remote ov-dev with no bearer → 401 → "Not logged in" OAuth dance
Fix: the rewrite block now also patches s.url from the conf-resolved
URL (`${_ov_url%/}/mcp`, or `$OPENVIKING_MCP_URL` if explicitly set).
Same idempotent fast-path — only writes when something actually changed.
Tested both directions:
ovcli-local.conf (no key, localhost)
→ cache .mcp.json: url=http://127.0.0.1:1933/mcp, no bearer field
→ env passed to codex: no OPENVIKING_API_KEY
→ /mcp: Auth: None, tools list populated
ovcli.conf (with key, remote)
→ cache .mcp.json: url=https://ov-dev.tosaki.top/mcp, bearer present
→ /mcp: Auth: Bearer token, tools list populated
ZaynJarvis
approved these changes
May 13, 2026
ZaynJarvis
approved these changes
May 13, 2026
ZaynJarvis
pushed a commit
that referenced
this pull request
May 13, 2026
…ecked-in file (#2026) * refactor(plugin/codex): move shell wrapper to standalone rc file The installer-emitted codex() wrapper had grown to ~60 lines of shell function body inlined as a marker-delimited block inside the user's ~/.zshrc / ~/.bashrc. Every upgrade required the awk-strip-and-append dance, which had a known edge case (rc with begin-marker but no end-marker) we'd already had to harden against, and the inline noise was hostile to anyone reading their own rc. This commit switches to the standard pyenv / nvm / fnm pattern: - Wrapper body lives in its own file at ~/.openviking/codex-plugin.rc.sh (path overridable via OPENVIKING_CODEX_WRAPPER_RC). Full overwrite on every install — no marker logic inside the wrapper file itself. - The user's shell rc gets a single one-line source hook, still wrapped in marker comments for cleanup-on-uninstall: # >>> openviking-codex-plugin >>> [ -f "$HOME/.openviking/codex-plugin.rc.sh" ] && . "..." # <<< openviking-codex-plugin <<< Since the content of this block never changes across installs, the marker-replacement logic only triggers the legacy-cleanup path once when upgrading from a pre-rc-split install that inlined the full wrapper. User-visible improvements: - ~/.zshrc OV-plugin block: ~70 lines → 3 lines. - `cat ~/.openviking/codex-plugin.rc.sh` shows the wrapper directly. - Uninstall is just `rm ~/.openviking/codex-plugin.rc.sh` + delete the 3-line block — no awk required. - Upgrades touch the rc file at most once (to install the source hook); subsequent installs only rewrite the wrapper file. Verified end-to-end: stale install with the old inline wrapper got the 3-line source hook substituted in place; `source ~/.zshrc && type codex` showed the new wrapper loaded from the standalone file. * refactor(plugin/codex): source wrapper from repo path, drop heredoc dance Follow-up to the rc-split commit. Instead of embedding the wrapper body as a heredoc inside install.sh and writing it to a copy under ~/.openviking/, the wrapper now lives as its own checked-in file at examples/codex-memory-plugin/setup-helper/wrapper.sh. The user's shell rc sources that file directly from the cloned plugin checkout (the path the installer already manages via git fetch + reset --hard). What this buys: - Wrapper diffs are real diffs — code review sees `+ codex() { ... }` rather than `+ heredoc lines inside install.sh that produce ~/.openviking/codex-plugin.rc.sh`. - No copy step in the installer means no installer code path for "did the user accidentally edit ~/.openviking/codex-plugin.rc.sh?" or "is the copied file in sync with what the installer would produce now?" - Updates ride for free on `git pull` / the installer's existing fetch+reset. No "re-run installer to refresh the wrapper" step. - Uninstall is just `rm ~/.openviking/openviking-repo` (or just leave it — the source hook will silently no-op when the file is gone, since it's gated with `[ -f ... ] && .`). Installer shrinks from ~420 lines (with the inline heredoc) to ~340. The wrapper is unchanged content-wise; this commit only moves where it lives. * refactor(plugin/claude-code): move shell wrapper to standalone rc file The installer-emitted claude() wrapper had been inlined as a marker-delimited block in the user's ~/.zshrc / ~/.bashrc. Every upgrade required the awk-strip-and-append dance, the inline noise was hostile to anyone reading their own rc, and there was a known footgun: if the END marker got hand-deleted from the rc, the next install's awk-strip would drop everything from the BEGIN marker to EOF. Switch to the standard pyenv / nvm / fnm pattern, mirroring what the codex-memory-plugin installer now does (see #2023): - Wrapper body lives in its own file at ~/.openviking/claude-plugin.rc.sh (path overridable via OPENVIKING_CLAUDE_WRAPPER_RC). Full overwrite on every install — no marker logic inside the wrapper file itself. - The user's shell rc gets a single one-line source hook, still marker-wrapped for clean uninstall: # >>> openviking claude-code memory plugin >>> [ -f "$HOME/.openviking/claude-plugin.rc.sh" ] && . "..." # <<< openviking claude-code memory plugin <<< Content is constant across installs, so the marker-replacement logic only triggers the legacy-cleanup path once (when upgrading from a pre-rc-split install that inlined the full claude() function body). User-visible improvements: - ~/.zshrc OV-plugin block: ~16 lines of wrapper body → 3 lines. - Wrapper body is a real file you can `cat` / `diff` / restore from source control; no need to re-run the installer to inspect it. - Uninstall: `rm ~/.openviking/claude-plugin.rc.sh` + delete the 3-line marker block. - The END-marker corruption footgun is gone, since the marker block content is bytestring-stable and the awk-strip only runs when both markers are present anyway. No behavior change to the wrapper itself (still pulls url/api_key from ovcli.conf via jq). * refactor(plugin/claude-code): extract wrapper to checked-in setup-helper/wrapper.sh Squash-style follow-up to the previous rc-split commit on this branch: now that the wrapper lives in its own file conceptually, just check it in at examples/claude-code-memory-plugin/setup-helper/wrapper.sh and have the user's shell rc source it directly from the cloned plugin checkout. No copy step, no heredoc dance in install.sh. Why this is better than the previous approach (wrapper body embedded as a heredoc in install.sh, written to a copy in ~/.openviking): - Wrapper is a real reviewable file. Diffs show `+ claude() { ... }`, not "+ heredoc lines that produce the wrapper". - Updates ride on the installer's existing `git fetch + reset --hard` step — no separate "re-run installer to refresh the copy" path. - One less source of truth (no $HOME copy that can drift from the installer's intent). - Uninstall: `rm ~/.openviking/openviking-repo`; the source hook in the rc silently no-ops via `[ -f ... ] && .`. The previous commit on this branch already shrunk the rc block from ~16 inline lines to 3 (marker + source hook + marker). This commit just moves the wrapper body from "embedded in installer" to "checked into the repo at a stable path", with no behavior change to the wrapper itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2022. Reported: with an
ovcli.confthat has noapi_key(typical for local-only OV without auth), the plugin would not start cleanly — Codex would drop into its OAuth fallback against a server that doesn't speak OAuth.Root cause
.mcp.jsonships withbearer_token_env_var: "OPENVIKING_API_KEY". When that env var resolves to an empty string at codex launch (because ovcli.conf has no key), Codex interprets it as "auth configured but not provided" and falls back to its OAuth dance — which then fails against an unauthenticated OV.Hook side is unaffected:
scripts/config.mjsalready gates the Bearer header onif (cfg.apiKey), so empty api_key → noAuthorizationheader → OV accepts in unauth mode. Verified end-to-end withauto-recall.mjsagainsthttp://127.0.0.1:1933and an empty-key ovcli.conf.Fix
At install time, detect whether ANY api_key is configured (env or ovcli.conf) and conditionally render
.mcp.json:bearer_token_env_var: "OPENVIKING_API_KEY"Authorizationand treats 200 as successJSON manipulation uses
node(already required) rather thansed, so the edit is structurally safe regardless of field ordering.env_http_headersstays in both modes — identity headers are independent of auth and OV accepts empty values (defaults todefault).Installer footer now reports the resolved auth mode:
vs
Test plan
api_keyovcli.conf + local OV:auto-recall.mjssucceeds, no Bearer header sent (validated via captured request)api_keyovcli.conf + node JSON manipulation: produces.mcp.jsonwithoutbearer_token_env_varfieldapi_keyovcli.conf:bearer_token_env_varis preservedOPENVIKING_API_KEYenv override (no ovcli.conf): still detected,bearer_token_env_varis kept