mirror of
https://github.com/volcengine/OpenViking.git
synced 2026-09-28 11:43:00 +08:00
* fix(codex): read the MCP proxy's connection from the hooks' loadConfig
The proxy resolved url, api_key, account and user through the bare
credential chain while taking everything else from loadConfig(). Since
the loader became buildPluginConfig() it adds layers that chain never
sees: ovcli.conf's plugin.codex apiKey/accountId/userId, and ov.conf's
codex.apiKey when ovcli.conf names only the server. With either, hooks
authenticated and every MCP tool call went out without a key.
Codex also hands a stdio MCP server only the env vars .mcp.json lists,
and OPENVIKING_AUTH_MODE was not one of them, so an env-set auth mode
decided the identity headers for hooks but not for MCP calls.
* fix(dsh): forward the resolved auth mode and timeout to the MCP proxy
The proxy runs as a child whose env DSH scrubs, so the parent forwards
what it resolved. It forwarded the endpoint, key, account, user and
peer but not the auth mode or request timeout, so a Cordis patch that
set either configured the in-process runtime and not the MCP calls.
The proxy now also takes its credential source and watched paths from
the resolved config instead of a second credential-chain call, and a
shared test keeps every proxy that ships beside hooks off that chain.
* refactor(shared): one connection resolver for hooks and the MCP proxy
The credential chain lived in two layers. `resolveOpenVikingCredentials()`
could not read ovcli.conf's `plugin.<harness>` keys, the ov.conf harness
fallback or the root-key tail; `buildPluginConfig()` patched those in, and
any caller that used the lower layer alone resolved a different key and
identity than the hooks did.
`resolveConnection(harness, { env, files, hostInput, rootKeyFallback })`
now answers server, key, identity and auth mode in one place, reading only
host input, the environment and the two ~/.openviking files. The hook
loader and `buildProxyConnection()` both consume it, and the old
two-layer entry points (`resolveOpenVikingCredentials`, `resolveAuthMode`,
the credentials.mjs CLI) are gone so a half-resolved chain cannot be
written again.
Behaviour is unchanged for every hook harness (checked field by field
against the previous implementation over thousands of generated file/env
combinations). Two deliberate additions: the portable agent-plugins proxy
now honours ovcli.conf's `plugin` connection keys and ov.conf's harness
section like every other harness, and dsh hands the host's `authMode`
(or `auth_mode`) over as host input, ranking it with the host's endpoint,
key and identity.
* fix(shared): a forced env credential source reads only the environment
`OPENVIKING_CREDENTIAL_SOURCE=env` is documented as "env vars only", but
only the url honoured it: the key, account, user, ovcli.conf's actor peer
and the auth mode still fell through to ovcli.conf, its plugin keys,
ov.conf and the root key when the variable was unset. A process that
exported an empty key to mean "no key" was silently handed whatever the
files held.
Forced to `env`, the connection now reads no file and an unset variable
stays empty; the url defaults to http://127.0.0.1:1933. The `peerId`
setting keeps its own layers. The doctor labels that mode instead of
pointing at files the chain skipped.
* refactor(shared): one proxy-config mapper and one forwarded-env list
Each proxy entrypoint copied a dozen fields out of its loader by hand,
under two sets of names, and the copies had drifted. What the proxy
process must be handed was a second hand-kept list, in Codex's
`.mcp.json` and in its test.
`toMcpProxyConfig(cfg, options)` maps a resolved loader or proxy
connection to the proxy config once. `MCP_PROXY_ENV_VARS` names every
variable that changes what a proxy sends; Codex's `env_vars` is now
checked against it, which adds the missing `OPENVIKING_STATE_DIR`.
* refactor(plugins): every loader takes an env, every proxy exports readProxyConfig(env)
The six MCP proxy entrypoints now reduce to one line: resolve through the
harness's own loader (or `buildProxyConnection` for the hook-less package)
and hand the result to `toMcpProxyConfig`. Every one exports
`readProxyConfig(env)`, and the codex, claude-code, opencode and agent-hook
loaders accept an injected env, so a test can drive a hook and its proxy
from the same inputs without touching process.env.
Mapping through one function fixes what the hand copies had lost: the
Claude Code and DSH proxies never passed `mcpUrl`, so
`OPENVIKING_MCP_URL` moved the hooks and left the tools behind.
The source guard now requires the shared mapper and the exported reader
in every proxy, and `buildProxyConnection` reports its two config paths
instead of a watch list of its own.
* fix(dsh): forward the resolved connection to the MCP proxy
DSH starts its MCP subprocess with the parent's environment minus
credential-shaped names (`/KEY|PASSWORD|SECRET|TOKEN/i`), so the bundle
forwards what it resolved. It forwarded the values but not the mode, and
only the non-empty ones:
- A child that receives `OPENVIKING_URL` runs the chain unpinned. Where
the parent's chain was pinned to an ovcli.conf that names only a url,
the parent sent no key while the proxy fell through to ov.conf's
`server.root_api_key`, so the tools reached the server as root while
the hooks were anonymous.
- `OPENVIKING_ACCOUNT`, `OPENVIKING_USER` and `OPENVIKING_PEER_ID` survive
DSH's scrub, so a value the parent's chain ignored filled the gap in
the child and went out as an identity header.
- With no peer to forward, the proxy derived one from its own launch
directory and sent an actor peer the runtime did not.
`forwardConnectionEnv(connection)` now writes every credential variable,
the empty ones too, with the forced `env` source, so the child reads no
file and resolves exactly the parent's url, MCP url, key, identity, auth
mode and peer. The proxy takes its peer from that environment only.
`buildMcpConfig` moves to `mcp-env.mjs`, which carries no host
dependency, so shared tests can build the child environment without the
DSH bridge.
* test(shared): prove the proxy and the hooks resolve one connection
The existing guards checked shape — that a proxy called the shared
builder — never that it reached the server as the same caller its hooks
did, which is how two harnesses shipped proxies that disagreed with them.
`mcp-hook-parity.test.mjs` runs every harness that ships a proxy beside
hooks through a dozen configurations: ovcli.conf's own fields, its
`plugin.<harness>` and shared plugin keys, ov.conf-only installs, the
pinned fallbacks to a harness key and to the root key, credential and
auth-mode variables, a forced source over stale variables, an explicit
MCP URL, a host's own input, and a workspace file that tries to move the
connection. The hook loader sees the full environment; the proxy sees
only what its host lets through — Codex's `env_vars`, DSH's scrubbed
inheritance plus the forwarded connection, everyone else's full
environment — and the url, key, identity and identity-header switch they
put on the wire must match. Scenarios with a known answer pin it too, and
a coverage check fails when a new proxy or hook client has no row.
The two codex-only proxy tests the matrix now covers are removed.
* docs(plugins): one connection for hooks and MCP, and version bumps
The capability reference, plugin development guide, Agent Plugins and
Codex pages (en/zh), both doctor references and the plugin READMEs now
describe the chain `resolveConnection()` runs: host input first, the
pinned ovcli.conf branch and what still falls through it, the auth mode
reading `OPENVIKING_AUTH_MODE` and the `plugin` keys in every mode, a
forced `env` source reading no file, and the two ways a connection crosses
into an MCP process (Codex's forwarded-variable list, dsh's forwarded
connection). The parity test is registered with the credential tests.
Versions move past both this branch's base and main: claude-code 0.5.2,
codex 0.9.2, agent-hook 0.3.2, opencode 0.3.2, dsh 0.4.3, pi 0.3.2,
agent-plugins 0.1.2.
197 lines
6.9 KiB
JavaScript
197 lines
6.9 KiB
JavaScript
/**
|
|
* Helpers shared by the plugin test files.
|
|
*
|
|
* They live outside `lib/` on purpose: `sync.mjs` copies that directory into
|
|
* every plugin, and nothing here belongs in a shipped plugin. Importing a
|
|
* helper from a `*.test.mjs` file would also register that file's own tests a
|
|
* second time in whichever runner picked it up.
|
|
*/
|
|
|
|
import assert from "node:assert/strict";
|
|
import { spawn } from "node:child_process";
|
|
import { mkdtemp, rm, writeFile } from "node:fs/promises";
|
|
import http from "node:http";
|
|
import { tmpdir } from "node:os";
|
|
import { join } from "node:path";
|
|
|
|
import { buildPluginConfig } from "../lib/plugin-config.mjs";
|
|
|
|
/**
|
|
* A built config resolved against nothing at all, for callers that only need
|
|
* the shape of the object a loader receives.
|
|
*/
|
|
export function buildConfigForTest(harness) {
|
|
const dir = join(tmpdir(), "ov-plugin-config-absent");
|
|
return buildPluginConfig(harness, {
|
|
cwd: dir,
|
|
env: {
|
|
OPENVIKING_CLI_CONFIG_FILE: join(dir, "ovcli.conf"),
|
|
OPENVIKING_CONFIG_FILE: join(dir, "ov.conf"),
|
|
OPENVIKING_HOME: dir,
|
|
},
|
|
});
|
|
}
|
|
|
|
/**
|
|
* An ovcli.conf / ov.conf pair in a fresh directory, and the env that points
|
|
* the credential chain at it. A file given as `null` is not written, so the
|
|
* chain sees it as absent rather than as a real ~/.openviking.
|
|
*/
|
|
export async function writeCredentialFiles(prefix, { ovcli = null, ov = null } = {}) {
|
|
const dir = await mkdtemp(join(tmpdir(), prefix));
|
|
const cliPath = join(dir, "ovcli.conf");
|
|
const ovPath = join(dir, "ov.conf");
|
|
if (ovcli) await writeFile(cliPath, JSON.stringify(ovcli, null, 2) + "\n");
|
|
if (ov) await writeFile(ovPath, JSON.stringify(ov, null, 2) + "\n");
|
|
return {
|
|
dir,
|
|
cliPath,
|
|
ovPath,
|
|
env: { OPENVIKING_CLI_CONFIG_FILE: cliPath, OPENVIKING_CONFIG_FILE: ovPath, OPENVIKING_HOME: join(dir, "home") },
|
|
};
|
|
}
|
|
|
|
const isOpenVikingVar = (name) => name.startsWith("OPENVIKING_") || name === "OV_DEBUG_LOG";
|
|
|
|
/**
|
|
* Clear the developer shell's `OPENVIKING_*` out of process.env, and return
|
|
* the function that puts them back. A test that injects its env everywhere
|
|
* still runs under this, so a read that slips past the injection fails the
|
|
* same way on every machine instead of passing on the one with a config.
|
|
*/
|
|
export function scrubOpenVikingEnv() {
|
|
const saved = Object.entries(process.env).filter(([name]) => isOpenVikingVar(name));
|
|
for (const [name] of saved) delete process.env[name];
|
|
return () => {
|
|
for (const name of Object.keys(process.env)) {
|
|
if (isOpenVikingVar(name)) delete process.env[name];
|
|
}
|
|
for (const [name, value] of saved) process.env[name] = value;
|
|
};
|
|
}
|
|
|
|
const PENDING_ENV_KEYS = [
|
|
"OPENVIKING_PENDING_DIR",
|
|
"OPENVIKING_PENDING_MAX_RETRIES",
|
|
"OPENVIKING_PENDING_REPLAY_LIMIT",
|
|
"OPENVIKING_PENDING_TTL_DAYS",
|
|
];
|
|
|
|
/**
|
|
* Run `fn` against an empty pending queue in a throwaway directory.
|
|
*
|
|
* The queue reads its directory and its limits from the environment, so the
|
|
* knobs are cleared for the duration and every one of them is restored after,
|
|
* whether or not the test set it.
|
|
*/
|
|
export async function withPendingDir(fn) {
|
|
const saved = PENDING_ENV_KEYS.map((key) => [key, process.env[key]]);
|
|
const dir = await mkdtemp(join(tmpdir(), "openviking-pending-test-"));
|
|
for (const key of PENDING_ENV_KEYS) delete process.env[key];
|
|
process.env.OPENVIKING_PENDING_DIR = dir;
|
|
try {
|
|
return await fn(dir);
|
|
} finally {
|
|
for (const [key, value] of saved) {
|
|
if (value === undefined) delete process.env[key];
|
|
else process.env[key] = value;
|
|
}
|
|
await rm(dir, { recursive: true, force: true });
|
|
}
|
|
}
|
|
|
|
/** The JSON body of a request, or `null` when the request carried no body. */
|
|
export function readRequestBody(req) {
|
|
return new Promise((resolve, reject) => {
|
|
const chunks = [];
|
|
req.on("data", (chunk) => chunks.push(chunk));
|
|
req.on("end", () => {
|
|
const raw = Buffer.concat(chunks).toString("utf-8");
|
|
try {
|
|
resolve(raw ? JSON.parse(raw) : null);
|
|
} catch (error) {
|
|
reject(error);
|
|
}
|
|
});
|
|
req.on("error", reject);
|
|
});
|
|
}
|
|
|
|
/** Answer with a JSON body, 200 unless the caller names another status. */
|
|
export function writeJson(res, value, statusCode = 200) {
|
|
res.writeHead(statusCode, { "Content-Type": "application/json" });
|
|
res.end(JSON.stringify(value));
|
|
}
|
|
|
|
/**
|
|
* Run `fn` against a mock OpenViking listening on loopback.
|
|
*
|
|
* `handler` may be sync or async; whatever it throws becomes a 500 rather than
|
|
* an unhandled rejection that outlives the test. `fn` receives the base URL and
|
|
* a live log of every request the mock saw, so a test that only cares about
|
|
* which endpoints were hit does not have to record them inside its handler.
|
|
*/
|
|
export async function withMockOpenViking(handler, fn) {
|
|
const requests = [];
|
|
const server = http.createServer((req, res) => {
|
|
requests.push({
|
|
method: req.method,
|
|
path: new URL(req.url, "http://127.0.0.1").pathname,
|
|
url: req.url,
|
|
headers: req.headers,
|
|
});
|
|
Promise.resolve(handler(req, res)).catch((error) => {
|
|
writeJson(res, { status: "error", error: String(error?.stack || error) }, 500);
|
|
});
|
|
});
|
|
await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve));
|
|
try {
|
|
const { port } = server.address();
|
|
return await fn(`http://127.0.0.1:${port}`, requests);
|
|
} finally {
|
|
await new Promise((resolve) => server.close(resolve));
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Run a hook script as its host runs it, and never reject.
|
|
*
|
|
* A hook that exits non-zero is a result a test may well be asserting on, so
|
|
* the exit code comes back like any other output; call `expectExit` where a
|
|
* clean exit is part of the expectation.
|
|
*/
|
|
export function runHookScript(scriptPath, { argv = [], input, env, cwd } = {}) {
|
|
return new Promise((resolve) => {
|
|
const child = spawn(process.execPath, [scriptPath, ...argv], {
|
|
cwd,
|
|
env: { ...process.env, ...env },
|
|
stdio: ["pipe", "pipe", "pipe"],
|
|
});
|
|
let stdout = "";
|
|
let stderr = "";
|
|
let spawnError = null;
|
|
let settled = false;
|
|
const settle = (code, signal) => {
|
|
if (settled) return;
|
|
settled = true;
|
|
resolve({ code, signal, stdout, stderr, error: spawnError });
|
|
};
|
|
child.stdout.on("data", (chunk) => { stdout += chunk; });
|
|
child.stderr.on("data", (chunk) => { stderr += chunk; });
|
|
child.on("error", (error) => { spawnError = error; settle(null, null); });
|
|
child.on("close", settle);
|
|
// A hook that answers without draining stdin makes this write fail EPIPE.
|
|
child.stdin.on("error", () => {});
|
|
child.stdin.end(
|
|
input === undefined || typeof input === "string" ? (input ?? "") : JSON.stringify(input),
|
|
);
|
|
});
|
|
}
|
|
|
|
/** Assert a `runHookScript` result exited with `code`, and return the result. */
|
|
export function expectExit(result, code = 0) {
|
|
const detail = result.stderr.trim() || String(result.error || "");
|
|
assert.equal(result.code, code, detail || `expected exit ${code}, got ${result.code}`);
|
|
return result;
|
|
}
|