Skip to content

fix(memory-plugin): setup wizard first-run path, proxy hint, config source reporting - #4387

Merged
t0saki merged 1 commit into
volcengine:mainfrom
t0saki:fix/memory-plugin-setup-wizard-proxy-hints
Aug 27, 2026
Merged

fix(memory-plugin): setup wizard first-run path, proxy hint, config source reporting#4387
t0saki merged 1 commit into
volcengine:mainfrom
t0saki:fix/memory-plugin-setup-wizard-proxy-hints

Conversation

@t0saki

@t0saki t0saki commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes three defects in the shared memory-plugin code: the setup wizard cannot create a config on a fresh machine, the MCP proxy's connection-failure hint names a subcommand that does not exist, and the Claude Code plugin reports the wrong file as the source of the API key.

Defect 1 — the setup wizard cannot create a first-run config

Root cause. runSetupWizard in examples/memory-plugin-shared/lib/setup-wizard.mjs destructured cliPath from loadCredentialFiles(env). In examples/memory-plugin-shared/lib/credentials.mjs, cliPath is deliberately "" when the file does not exist (let cliPath = cliFile ? cliPathCandidate : ""), because callers use it as an "ovcli.conf is in play" signal; the path the user should write to is returned separately as cliPathCandidate. The wizard used the signal as if it were the path, so on a machine with no ~/.openviking/ovcli.conf it read, printed, and wrote the empty string.

Introduced in. f90556253feat(plugins): stdio MCP proxy, remote marketplace install, and type-quota recall for memory plugins (#3039), which added both setup-wizard.mjs and the cliPath / cliPathCandidate split in credentials.mjs in the same commit. The wizard has never worked on a first run.

Symptom. node examples/claude-code-memory-plugin/scripts/setup.mjs on a fresh machine prints Config file: with a blank path and then either exits silently without writing anything or fails with ENOENT: no such file or directory, open ''. This is the only supported configuration path for a pure-marketplace install with no installer script, so those users could not configure the plugin at all. Every harness that exposes the wizard is affected (Claude Code, Codex, opencode, pi).

Fix. Fall back to cliPathCandidate when cliPath is empty, which keeps honoring an explicit OPENVIKING_CLI_CONFIG_FILE and otherwise resolves to ~/.openviking/ovcli.conf. The existing mkdirSync(dirname(...), { recursive: true }) before the write now runs against a real path, so a missing ~/.openviking directory is created.

Verified. New examples/memory-plugin-shared/setup-wizard.test.mjs drives the wizard with scripted stdin; the first-run case fails with ENOENT ... open '' against the pre-fix module and passes after. Also reproduced end to end: printf 'y\n\n\n\ny\n' | OPENVIKING_CLI_CONFIG_FILE=<tmp>/.openviking/ovcli.conf node examples/claude-code-memory-plugin/scripts/setup.mjs now prints the real target path instead of a blank one.

Defect 2 — the proxy error hint names a command that does not exist

Root cause. The JSON-RPC -32001 transport-failure path in examples/memory-plugin-shared/lib/mcp-proxy-core.mjs told the user to check that 'ov serve' is running. There is no ov serve subcommand — ov is the Rust CLI, and the server is started with openviking-server (see the entry points in pyproject.toml).

Introduced in. f90556253feat(plugins): stdio MCP proxy, remote marketplace install, and type-quota recall for memory plugins (#3039), the commit that added mcp-proxy-core.mjs.

Symptom. Whenever the server is down or the URL is wrong, every harness's MCP tool call surfaces an error telling the user to run a command that does not exist; ov serve exits with an unknown-subcommand error, sending the user down a dead end during exactly the failure they are trying to diagnose.

Fix. The hint now reads ... and that the OpenViking server (\openviking-server`) is reachable.A repo-wide grep forov servenow returns nothing; all other occurrences were the generated copies of this same string, regenerated bysync.mjs`.

Defect 3 — loadConfig().configPath reports the wrong file

Root cause. examples/claude-code-memory-plugin/scripts/config.mjs set configPath: ovConf?.configPath || cliConf?.configPath, i.e. whichever config file merely parsed, with ov.conf unconditionally preferred. The actual credential chain is the opposite order and is per-field: env → ovcli.conf api_keyov.conf claude_code.apiKeyov.conf server.root_api_key. servers/mcp-proxy.mjs then derived credentialSource from configPath.endsWith("ovcli.conf") and passed credentialPath: cfg.configPath, so on any machine that has both files the proxy reported ov.conf even when the key came from ovcli.conf. scripts/ov-status.mjs re-implemented the real chain independently, so the two disagreed.

Introduced in. The configPath line dates to 8c01e97eefeat(cc-memory-plugin): persistent session and recall redesign (#1615), the commit that added ovcli.conf as a credential source ahead of ov.conf in the api_key chain while leaving configPath preferring ov.conf. It became user-visible in f90556253 (#3039), which introduced servers/mcp-proxy.mjs and derived credentialSource / credentialPath from that field.

Symptom. The proxy's debug start log line and the data.credentialPath in its 401 error point at ~/.openviking/ov.conf while the key actually comes from ~/.openviking/ovcli.conf — so an operator debugging a 401 edits the wrong file. On this machine, before the fix, loadConfig().configPath printed the ov.conf path while ov-status.mjs printed api_key from ~/.openviking/ovcli.conf. The Codex proxy had the same class of bug through credentialPath: creds.cliPath || creds.ovPath, which names ovcli.conf whenever that file exists even if the key came from ov.conf.

Fix. loadConfig() now also returns credentialSource (env / ovcli / ov / none) and credentialPath, computed by walking the same chain that produces apiKey — including the case where apiKey arrives through ovcli.conf's plugin.claude_code section, which is reported as ovcli. configPath keeps its old meaning for backward compat. servers/mcp-proxy.mjs and scripts/ov-status.mjs both read the new fields, so they can no longer disagree. On the shared side, resolveOpenVikingCredentials gained the same credentialPath and the Codex proxy now uses it.

Verified. New examples/claude-code-memory-plugin/scripts/config.test.mjs covers all five branches of the chain against throwaway config pairs; examples/codex-memory-plugin/scripts/ov-credentials.test.mjs gained a case for the shared resolver. Confirmed live on a machine with both files: loadConfig() now reports credentialSource: ovcli / credentialPath: ~/.openviking/ovcli.conf, matching ov-status.mjs's api_key from ~/.openviking/ovcli.conf.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • examples/memory-plugin-shared/lib/setup-wizard.mjs: write to cliPath || cliPathCandidate so a first run creates ovcli.conf (and its parent directory) instead of writing to "".
  • examples/memory-plugin-shared/lib/mcp-proxy-core.mjs: replace the nonexistent 'ov serve' in the transport-failure hint with openviking-server.
  • examples/memory-plugin-shared/lib/credentials.mjs: resolveOpenVikingCredentials now returns credentialPath, the file that actually supplied the api_key (empty for env-sourced keys).
  • examples/claude-code-memory-plugin/scripts/config.mjs: loadConfig() returns credentialSource and credentialPath following the real api_key chain; configPath is unchanged for backward compat.
  • examples/claude-code-memory-plugin/servers/mcp-proxy.mjs and scripts/ov-status.mjs: consume the new fields so the proxy log, the proxy's 401 payload and /ov all name the same file.
  • examples/codex-memory-plugin/servers/mcp-proxy.mjs: use creds.credentialPath instead of creds.cliPath || creds.ovPath.
  • New tests examples/memory-plugin-shared/setup-wizard.test.mjs and examples/claude-code-memory-plugin/scripts/config.test.mjs, plus a new case in examples/codex-memory-plugin/scripts/ov-credentials.test.mjs; both new files added to the node --test list in .github/workflows/pr.yml.
  • Regenerated the sync.mjs copies of the three changed shared modules.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

node examples/memory-plugin-shared/sync.mjs followed by the workflow's node --test list: 221 tests, 221 passing.

The wizard test was run against the pre-fix module to confirm it reproduces the defect (Error: ENOENT: no such file or directory, open '') before passing against the fix.

Two files in the workflow list, install-agent-hooks.test.mjs and install-opencode-jsonc.test.mjs, fail when run from a linked git worktree rather than a normal checkout: install.sh's resolve_self_checkout tests [ -d "$dir/../../.git" ], and in a worktree .git is a file, so CHECKOUT_DIR stays empty and the marketplace directory resolves to /examples. This is unrelated to these changes and does not affect CI, which runs on a normal checkout.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Additional Notes

agent-plugins/servers/config.mjs and agent-plugins/servers/mcp-proxy.mjs carry the same configPath / credentialSource pattern as defect 3. It is left alone here to keep this change scoped to the plugins named above.

Copilot AI lite review requested due to automatic review settings August 27, 2026 05:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@t0saki
t0saki merged commit 3b1db20 into volcengine:main Aug 27, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants