fix(codex): avoid stale actor peer in MCP proxy (#4400)

Signed-off-by: Axiomoth <alearner@splrad.com>
2026-09-29 16:58:31 +08:00 · 2026-08-27 18:33:20 +08:00
parent 206054cf15
commit cf5cc308ca
11 changed files with 196 additions and 5 deletions
@@ -41,6 +41,30 @@ export function defaultCredentialPaths(env = process.env) {
  ].filter(Boolean);
 }

+/**
+ * Resolve the actor peer header for a long-lived MCP proxy process.
+ *
+ * MCP servers may start in the plugin directory rather than the active
+ * workspace, so their process cwd is not a reliable peer identity. Broad
+ * recall intentionally spans the authenticated user's peer workspaces and
+ * therefore leaves the actor header unset. Actor-scoped recall requires an
+ * explicit peer so isolation never depends on the proxy launch directory.
+ */
+export function resolveMcpActorPeerId({
+  peerId = "",
+  recallPeerScope = "all",
+} = {}) {
+  if (recallPeerScope !== "actor") return "";
+
+  const explicitPeerId = String(peerId || "").trim();
+  if (explicitPeerId) return explicitPeerId;
+
+  throw new Error(
+    "OpenViking MCP actor-scoped recall requires an explicit peer ID. "
+    + "Set actor_peer_id in ovcli.conf or OPENVIKING_PEER_ID in the MCP environment.",
+  );
+}
+
 function uniq(values) {
  return [...new Set(values.filter(Boolean))];
 }
@@ -41,6 +41,30 @@ export function defaultCredentialPaths(env = process.env) {
  ].filter(Boolean);
 }

+/**
+ * Resolve the actor peer header for a long-lived MCP proxy process.
+ *
+ * MCP servers may start in the plugin directory rather than the active
+ * workspace, so their process cwd is not a reliable peer identity. Broad
+ * recall intentionally spans the authenticated user's peer workspaces and
+ * therefore leaves the actor header unset. Actor-scoped recall requires an
+ * explicit peer so isolation never depends on the proxy launch directory.
+ */
+export function resolveMcpActorPeerId({
+  peerId = "",
+  recallPeerScope = "all",
+} = {}) {
+  if (recallPeerScope !== "actor") return "";
+
+  const explicitPeerId = String(peerId || "").trim();
+  if (explicitPeerId) return explicitPeerId;
+
+  throw new Error(
+    "OpenViking MCP actor-scoped recall requires an explicit peer ID. "
+    + "Set actor_peer_id in ovcli.conf or OPENVIKING_PEER_ID in the MCP environment.",
+  );
+}
+
 function uniq(values) {
  return [...new Set(values.filter(Boolean))];
 }
@@ -115,11 +115,13 @@ Hooks and the MCP proxy call the same resolver directly, so the model tools and

 Auth is sent as `Authorization: Bearer <api_key>` to both the REST API (used by hooks) and the `/mcp` endpoint (used by the model); the hooks also send the same key as `X-API-Key` for compatibility with older servers.

-By default the plugin derives a peer from the current workspace path using Claude's project-directory naming rule: every non-letter-or-digit character becomes `-`, with no path normalization. For example, `/Users/x/Dev/OpenViking` becomes `-Users-x-Dev-OpenViking`. Hooks pass the effective peer as `peer_id` for captured session messages and as `X-OpenViking-Actor-Peer` for retrieval/filesystem calls; MCP gets the same header mapping.
+By default the hooks derive a peer from the current workspace path using Claude's project-directory naming rule: every non-letter-or-digit character becomes `-`, with no path normalization. For example, `/Users/x/Dev/OpenViking` becomes `-Users-x-Dev-OpenViking`. Hooks pass the effective peer as `peer_id` for captured session messages and as `X-OpenViking-Actor-Peer` for retrieval and filesystem calls.

 Set `actor_peer_id` in `ovcli.conf` (or `OPENVIKING_PEER_ID` with `OPENVIKING_CREDENTIAL_SOURCE=env`) to override the workspace-derived peer. The legacy `codex.peerId` / `codex.peer_id` fields in `ov.conf` still resolve as a fallback. Set `OPENVIKING_WORKSPACE_PEER=0` or `codex.workspacePeer=false` to turn off workspace-derived peers.

-Recall defaults to the broad mode: global memory, the current workspace, and other workspace memories can all be recalled, with other workspaces penalized and rendered later. Set `OPENVIKING_RECALL_PEER_SCOPE=actor` or `codex.recallPeerScope="actor"` for the isolation mode, which only sees global memory plus the current workspace. In deployments where one bot serves multiple real people, such as zouk, vikingbot, or AstrBot, use the isolation mode with an explicit actor peer so one person's memories are not recalled into another person's session.
+Recall defaults to broad mode: global memory, the current workspace, and other workspace memories can all be recalled, with other workspaces ranked lower and rendered later. In this mode, the MCP proxy omits `X-OpenViking-Actor-Peer` so it can read any URI returned by broad recall for the authenticated user.
+
+Set `OPENVIKING_RECALL_PEER_SCOPE=actor` or `codex.recallPeerScope="actor"` for isolation mode, which only sees global memory plus the configured peer. The MCP proxy requires `actor_peer_id` or `OPENVIKING_PEER_ID` in this mode and exits with a configuration error if neither is set. In deployments where one bot serves multiple people, such as zouk, vikingbot, or AstrBot, use isolation mode with an explicit actor peer so sessions cannot read another person's memories.

 The checked-in `.mcp.json` contains only a stdio command. It never stores server URLs, bearer-token env mappings, or identity headers, so switching `ovcli.conf` changes the MCP target on the next Codex launch without cache rendering.

@@ -180,6 +180,8 @@ test("Codex MCP entrypoint forwards only native OpenViking tools", () => {
  const entrypoint = readFileSync(join(pluginDir, "servers", "mcp-proxy.mjs"), "utf-8");
  assert.doesNotMatch(entrypoint, /createExperienceToolProvider/);
  assert.doesNotMatch(entrypoint, /localToolProvider/);
+  assert.match(entrypoint, /resolveMcpActorPeerId\(cfg\)/);
+  assert.doesNotMatch(entrypoint, /resolveEffectivePeerId|process\.cwd\(\)/);
 });

 test("canonical MCP tool list matches server registrations", () => {
@@ -41,6 +41,30 @@ export function defaultCredentialPaths(env = process.env) {
  ].filter(Boolean);
 }

+/**
+ * Resolve the actor peer header for a long-lived MCP proxy process.
+ *
+ * MCP servers may start in the plugin directory rather than the active
+ * workspace, so their process cwd is not a reliable peer identity. Broad
+ * recall intentionally spans the authenticated user's peer workspaces and
+ * therefore leaves the actor header unset. Actor-scoped recall requires an
+ * explicit peer so isolation never depends on the proxy launch directory.
+ */
+export function resolveMcpActorPeerId({
+  peerId = "",
+  recallPeerScope = "all",
+} = {}) {
+  if (recallPeerScope !== "actor") return "";
+
+  const explicitPeerId = String(peerId || "").trim();
+  if (explicitPeerId) return explicitPeerId;
+
+  throw new Error(
+    "OpenViking MCP actor-scoped recall requires an explicit peer ID. "
+    + "Set actor_peer_id in ovcli.conf or OPENVIKING_PEER_ID in the MCP environment.",
+  );
+}
+
 function uniq(values) {
  return [...new Set(values.filter(Boolean))];
 }
@@ -13,9 +13,11 @@ import { fileURLToPath } from "node:url";
 import { loadConfig } from "../scripts/config.mjs";
 import { createLogger } from "../scripts/debug-log.mjs";
 import { resolveOpenVikingCredentials } from "../scripts/ov-credentials.mjs";
-import { buildMcpProxyConfig } from "../scripts/shared/mcp-proxy-config.mjs";
+import {
+  buildMcpProxyConfig,
+  resolveMcpActorPeerId,
+} from "../scripts/shared/mcp-proxy-config.mjs";
 import { createOpenVikingMcpProxy } from "../scripts/shared/mcp-proxy-core.mjs";
-import { resolveEffectivePeerId } from "../scripts/shared/workspace-peer.mjs";

 export { createOpenVikingMcpProxy } from "../scripts/shared/mcp-proxy-core.mjs";

@@ -28,7 +30,7 @@ function readProxyConfig() {
    apiKey: creds.apiKey,
    account: creds.account,
    user: creds.user,
-    peerId: resolveEffectivePeerId({ cfg, cwd: process.cwd() }).peerId,
+    peerId: resolveMcpActorPeerId(cfg),
    userAgent: cfg.userAgent,
    timeoutMs: cfg.timeoutMs,
    debug: cfg.debug,
@@ -41,6 +41,30 @@ export function defaultCredentialPaths(env = process.env) {
  ].filter(Boolean);
 }

+/**
+ * Resolve the actor peer header for a long-lived MCP proxy process.
+ *
+ * MCP servers may start in the plugin directory rather than the active
+ * workspace, so their process cwd is not a reliable peer identity. Broad
+ * recall intentionally spans the authenticated user's peer workspaces and
+ * therefore leaves the actor header unset. Actor-scoped recall requires an
+ * explicit peer so isolation never depends on the proxy launch directory.
+ */
+export function resolveMcpActorPeerId({
+  peerId = "",
+  recallPeerScope = "all",
+} = {}) {
+  if (recallPeerScope !== "actor") return "";
+
+  const explicitPeerId = String(peerId || "").trim();
+  if (explicitPeerId) return explicitPeerId;
+
+  throw new Error(
+    "OpenViking MCP actor-scoped recall requires an explicit peer ID. "
+    + "Set actor_peer_id in ovcli.conf or OPENVIKING_PEER_ID in the MCP environment.",
+  );
+}
+
 function uniq(values) {
  return [...new Set(values.filter(Boolean))];
 }
@@ -40,6 +40,30 @@ export function defaultCredentialPaths(env = process.env) {
  ].filter(Boolean);
 }

+/**
+ * Resolve the actor peer header for a long-lived MCP proxy process.
+ *
+ * MCP servers may start in the plugin directory rather than the active
+ * workspace, so their process cwd is not a reliable peer identity. Broad
+ * recall intentionally spans the authenticated user's peer workspaces and
+ * therefore leaves the actor header unset. Actor-scoped recall requires an
+ * explicit peer so isolation never depends on the proxy launch directory.
+ */
+export function resolveMcpActorPeerId({
+  peerId = "",
+  recallPeerScope = "all",
+} = {}) {
+  if (recallPeerScope !== "actor") return "";
+
+  const explicitPeerId = String(peerId || "").trim();
+  if (explicitPeerId) return explicitPeerId;
+
+  throw new Error(
+    "OpenViking MCP actor-scoped recall requires an explicit peer ID. "
+    + "Set actor_peer_id in ovcli.conf or OPENVIKING_PEER_ID in the MCP environment.",
+  );
+}
+
 function uniq(values) {
  return [...new Set(values.filter(Boolean))];
 }
@@ -7,6 +7,7 @@ import {
  DEFAULT_PROXY_TIMEOUT_MS,
  defaultCredentialPaths,
  normalizeConfigPath,
+  resolveMcpActorPeerId,
  trimSlash,
 } from "./lib/mcp-proxy-config.mjs";

@@ -66,6 +67,22 @@ test("debug stays strictly boolean-true opt-in", () => {
  assert.equal(buildMcpProxyConfig({ debug: true }).debug, true);
 });

+test("broad MCP recall does not send an actor peer header", () => {
+  assert.equal(resolveMcpActorPeerId({ peerId: "workspace-a", recallPeerScope: "all" }), "");
+  assert.equal(resolveMcpActorPeerId({ peerId: "workspace-a" }), "");
+});
+
+test("actor-scoped MCP recall requires and trims an explicit peer", () => {
+  assert.equal(
+    resolveMcpActorPeerId({ peerId: " workspace-a ", recallPeerScope: "actor" }),
+    "workspace-a",
+  );
+  assert.throws(
+    () => resolveMcpActorPeerId({ recallPeerScope: "actor" }),
+    /requires an explicit peer ID/,
+  );
+});
+
 test("path and URL helpers stay exported for entrypoints that need them", () => {
  assert.equal(trimSlash("http://x/"), "http://x");
  assert.equal(normalizeConfigPath(""), "");
@@ -41,6 +41,30 @@ export function defaultCredentialPaths(env = process.env) {
  ].filter(Boolean);
 }

+/**
+ * Resolve the actor peer header for a long-lived MCP proxy process.
+ *
+ * MCP servers may start in the plugin directory rather than the active
+ * workspace, so their process cwd is not a reliable peer identity. Broad
+ * recall intentionally spans the authenticated user's peer workspaces and
+ * therefore leaves the actor header unset. Actor-scoped recall requires an
+ * explicit peer so isolation never depends on the proxy launch directory.
+ */
+export function resolveMcpActorPeerId({
+  peerId = "",
+  recallPeerScope = "all",
+} = {}) {
+  if (recallPeerScope !== "actor") return "";
+
+  const explicitPeerId = String(peerId || "").trim();
+  if (explicitPeerId) return explicitPeerId;
+
+  throw new Error(
+    "OpenViking MCP actor-scoped recall requires an explicit peer ID. "
+    + "Set actor_peer_id in ovcli.conf or OPENVIKING_PEER_ID in the MCP environment.",
+  );
+}
+
 function uniq(values) {
  return [...new Set(values.filter(Boolean))];
 }
@@ -41,6 +41,30 @@ export function defaultCredentialPaths(env = process.env) {
  ].filter(Boolean);
 }

+/**
+ * Resolve the actor peer header for a long-lived MCP proxy process.
+ *
+ * MCP servers may start in the plugin directory rather than the active
+ * workspace, so their process cwd is not a reliable peer identity. Broad
+ * recall intentionally spans the authenticated user's peer workspaces and
+ * therefore leaves the actor header unset. Actor-scoped recall requires an
+ * explicit peer so isolation never depends on the proxy launch directory.
+ */
+export function resolveMcpActorPeerId({
+  peerId = "",
+  recallPeerScope = "all",
+} = {}) {
+  if (recallPeerScope !== "actor") return "";
+
+  const explicitPeerId = String(peerId || "").trim();
+  if (explicitPeerId) return explicitPeerId;
+
+  throw new Error(
+    "OpenViking MCP actor-scoped recall requires an explicit peer ID. "
+    + "Set actor_peer_id in ovcli.conf or OPENVIKING_PEER_ID in the MCP environment.",
+  );
+}
+
 function uniq(values) {
  return [...new Set(values.filter(Boolean))];
 }