Compare commits

..
Author SHA1 Message Date
taodengandClaude Opus 5 4759639d9b docs: reference only helpers that exist on main (AGENTS.md)
Review caught a real forward-reference: the new AGENTS.md section named
`ltDiag` and told readers to wait on `closed`, and NEITHER exists on main —
both are introduced by the still-unmerged #204. `git grep ltDiag` matched
exactly one line: the documentation itself.

That is worse than a broken link. AGENTS.md is, by its own "Handoff
expectations", the FIRST file a fresh session reads; it would have sent that
session looking for two identifiers it cannot find, in the file whose whole job
is to orient them.

It also meant this PR had an undeclared merge-order dependency on #204, which
I had not stated anywhere.

Fixed so each PR is self-consistent regardless of merge order:
  - names `ltPostStatus` (which does exist) instead of `ltDiag`
  - states the #203 rule as a PRINCIPLE — a terminated child's pipes may still
    hold unread data, so wait for stdio to close rather than for the exit —
    instead of naming the `closed` flag that #204 adds

Verified: every helper the section now names resolves to a `function <name>` in
test-features.mjs on this branch, and neither `ltDiag` nor `closed` is
referenced any more.

Tests: 457 passed, 0 failed (comments/docs only, unchanged).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
2026-07-27 09:06:58 +10:00
taodengandClaude Opus 5 868a1953f4 docs: guard the asymmetric cache keys (#200); document the ltBoot fault lever (#197)
Two knowledge-preservation fixes. No behavior change: server.mjs gains comments
only, verified by `git diff --stat` (comment lines) and an unchanged suite.

#200 — structuredHash and dedupKey compute the same value and must NOT be
collapsed. They take identical arguments and read as obvious duplicate work,
which is how someone "optimises" them into a bug. Their GUARDS differ, verified
line by line rather than asserted:

  structuredHash: if (CACHE_TTL > 0 && !conversationId && !hasCacheControl(...))
  dedupKey:       (!conversationId && !hasCacheControl(...))          <- no CACHE_TTL

CLAUDE_CACHE_TTL defaults to 0, so in the DEFAULT configuration structuredHash
is null while dedupKey must still be computed — it drives #153's single-flight
stampede protection, which is deliberately independent of response caching.
`dedupKey = structuredHash` would silently disable stampede protection by
default, in exactly the concurrent-AI-Task case it exists to bound. The comment
records that, and says what a safe deduplication would look like if anyone
still wants one (compute under the weaker guard, derive under the stronger,
add a stampede test first).

This was first raised in the #194 review as a cosmetic nit and then RETRACTED
by that reviewer after reading the guards — the retraction is the valuable
part, so it goes in the code rather than staying in a PR thread.

#197 — new "Testing: reaching faults inside server.mjs" section in AGENTS.md.
The premise the issue was originally filed on was wrong twice over, and both
errors were mine: first that no live-server harness existed (ltBoot has been
there all along, and I had used it myself in #191), then that a synchronous
fault deep inside spawnClaudeProcess needed a production test hook (it needs
--stack-size, which ltBoot already forwards).

The section documents the fixture, the --stack-size fault lever with the
concrete numbers (~124k element spread threshold at the default stack vs ~24k
at --stack-size=200, which is what fits Linux's 131072-byte MAX_ARG_STRLEN so
the test RUNS in CI instead of skipping), and the three rules that made it hold:
discover the threshold in a child under the same flags, assert the fault
actually fired, and wait for the thing you are about to assert rather than a
proxy for it. Each rule cites the issue that taught it (#193/#199/#203).

Written to be read BEFORE someone concludes a bug class is untestable, since
that conclusion has now been reached wrongly twice in this repo.

cli.js citation: NOT APPLICABLE — comments only, no wire behavior, no code
path, no argv change. Authorized by ADR 0006 as a behaviour-preserving edit;
the field set and every request/response contract are byte-identical.

Tests: 457 passed, 0 failed (unchanged, as expected for a comments-only diff).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
2026-07-27 08:53:30 +10:00
3 changed files with 77 additions and 98 deletions
+16
View File
@@ -52,6 +52,22 @@ Runtime: Node.js (ESM, `.mjs` throughout). No build step. No bundler. `server.mj
--- ---
## Testing: reaching faults inside `server.mjs`
`test-features.mjs` cannot `import` `server.mjs` (it calls `server.listen()` at top level), and that has twice led to the wrong conclusion that a class of bug is untestable. It isn't. Read this before writing "no regression test is possible here".
**There is a real live-server fixture.** `ltBoot(env, dir, nodeArgs)` (around `test-features.mjs:990`) spawns the actual `server.mjs` as a child with a **fake `claude` binary**, so integration tests cost no quota. `ltPost` / `ltPostStatus` / `ltWait` / `ltFreePort` round it out. It already covers boot gates, cache-epoch invalidation across two boots sharing one SQLite store, and system-prompt capture.
**`--stack-size` is a fault lever.** `ltBoot`'s third argument passes V8 flags to the child, which puts recursion- and argument-count-limited failures in reach at a much smaller input. `#193` needed a *synchronous throw* deep inside `spawnClaudeProcess`; `buildCliArgs` does `args.push("--allowedTools", ...ALLOWED_TOOLS)`, and under `--stack-size=200` that spread throws at ~24k elements instead of ~124k — which is what brings the trigger under Linux's `MAX_ARG_STRLEN` (131072 bytes for a single env string) so the test runs in CI rather than skipping. **No production fault hook was needed.**
Three rules that made it hold up, all learned the hard way:
- **Discover the threshold in a child under the same flags**, never in the test process — the parent's stack is not the one that matters.
- **Assert that the fault actually fired**, not just that the outcome looks right. `#193` asserts HTTP 500 *and* that the body carries `call stack size exceeded`; a control mutation (trigger neutered, bug still present) proves the test fails rather than passing vacuously.
- **Wait for the thing you are about to assert**, not a proxy for it. Waiting on `listening on` and then asserting a different line is a race (`#199`); waiting for the process to *exit* and then reading its `stderr` is another, because a terminated child's pipes may still hold unread data (`#203` — wait for the stdio to close, not for the exit).
Allocate ports with `ltFreePort()`. Fixed ports have caused at least one flake here.
## Release protocol ## Release protocol
OCP follows the machine-readable `release_kit:` overlay in `CLAUDE.md` (Iron Rule 5.5). Before any version bump or tag push, re-read that YAML block and walk every item in `new_feature_doc_expectations` and `bootstrap_quirk_policy`. Tag push triggers `.github/workflows/release.yml`, which creates the GitHub Release automatically — do not create the release manually. OCP follows the machine-readable `release_kit:` overlay in `CLAUDE.md` (Iron Rule 5.5). Before any version bump or tag push, re-read that YAML block and walk every item in `new_feature_doc_expectations` and `bootstrap_quirk_policy`. Tag push triggers `.github/workflows/release.yml`, which creates the GitHub Release automatically — do not create the release manually.
+12
View File
@@ -2923,6 +2923,16 @@ async function handleChatCompletions(req, res) {
const t0s = Date.now(); const t0s = Date.now();
const promptCharsS = messages.reduce((a, m) => a + contentToText(m.content).length, 0); const promptCharsS = messages.reduce((a, m) => a + contentToText(m.content).length, 0);
let structuredHash = null; let structuredHash = null;
// DO NOT collapse this with `dedupKey` below (#200). The two cacheHash calls take IDENTICAL
// arguments and look like obvious duplicate work — they are not interchangeable, because
// their GUARDS differ: this one additionally requires CACHE_TTL > 0. CLAUDE_CACHE_TTL
// DEFAULTS TO 0, so in the default configuration structuredHash is null while dedupKey must
// still be computed — it drives #153's single-flight stampede protection, which is
// deliberately independent of whether response caching is on. `dedupKey = structuredHash`
// would therefore silently disable stampede protection by default, in exactly the
// concurrent-AI-Task case it exists to bound. The duplicate call is the honest price of the
// asymmetry. If you do deduplicate it, compute once under the WEAKER guard and derive the
// cache lookup under the stronger one — and add a stampede test before you do.
if (CACHE_TTL > 0 && !conversationId && !hasCacheControl(messages)) { if (CACHE_TTL > 0 && !conversationId && !hasCacheControl(messages)) {
structuredHash = cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH }); structuredHash = cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH });
try { try {
@@ -2942,6 +2952,8 @@ async function handleChatCompletions(req, res) {
// firing several AI Tasks at once) must NOT each pay N× — they share one flight. We dedup every // firing several AI Tasks at once) must NOT each pay N× — they share one flight. We dedup every
// one-off structured request (not stateful sessions / client-side prompt caching), independent of // one-off structured request (not stateful sessions / client-side prompt caching), independent of
// whether OCP response caching is enabled; when caching IS on, the same key gates cache read/write. // whether OCP response caching is enabled; when caching IS on, the same key gates cache read/write.
// Note the guard here is deliberately WEAKER than structuredHash's — no CACHE_TTL check. See the
// do-not-collapse comment above (#200).
const dedupKey = (!conversationId && !hasCacheControl(messages)) const dedupKey = (!conversationId && !hasCacheControl(messages))
? cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH }) ? cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH })
: null; : null;
+49 -98
View File
@@ -991,48 +991,17 @@ function ltBoot(env, dir, nodeArgs = []) {
CLAUDE_BIND: "127.0.0.1", CLAUDE_AUTH_MODE: "none", CLAUDE_CACHE_TTL: "0", CLAUDE_TIMEOUT: "4000", ...env }, CLAUDE_BIND: "127.0.0.1", CLAUDE_AUTH_MODE: "none", CLAUDE_CACHE_TTL: "0", CLAUDE_TIMEOUT: "4000", ...env },
stdio: ["ignore", "pipe", "pipe"], stdio: ["ignore", "pipe", "pipe"],
}); });
const buf = { out: "", err: "", exit: undefined, signal: undefined, closed: false, spawnErr: null }; const buf = { out: "", err: "", exit: undefined };
child.stdout.on("data", d => { buf.out += d; }); child.stdout.on("data", d => { buf.out += d; });
child.stderr.on("data", d => { buf.err += d; }); child.stderr.on("data", d => { buf.err += d; });
// 'exit' fires when the process terminates, but its stdio pipes may still hold unread data — child.on("exit", code => { buf.exit = code; });
// 'close' is the one that guarantees both are drained. A test that terminates the child and
// then asserts on buf.err/buf.out must wait for `closed`, not `exit != null`, or it can read
// an empty buffer. See ltAssertBoot below.
child.on("exit", (code, signal) => { buf.exit = code; buf.signal = signal; });
child.on("close", () => { buf.closed = true; });
// Without a listener, a spawn 'error' is re-thrown as an uncaught exception and takes down the
// whole runner instead of failing one test.
child.on("error", e => { buf.spawnErr = e; });
return { child, buf }; return { child, buf };
} }
// child.kill("SIGKILL") kills server.mjs but NOT the fake `claude` grandchildren it spawned, and
// those can still be writing sp.txt / spawns.txt into `dir` while rmSync walks it — which surfaced
// as an intermittent ENOTEMPTY (4/200 in review). Node's own retry loop handles the window.
function _ltRmRetry(dir) {
try { _ltRm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }
catch { /* a leaked temp dir must never fail a test — the OS reaps it */ }
}
// Every ltBoot assertion failure should be self-diagnosing. The historical failure text was
// `expected a local-tools FATAL, got: ` — an empty string, which says nothing about whether the
// child never wrote, wrote to the other stream, died on a signal, or was never spawned.
function ltDiag(buf) {
return `exit=${buf.exit} signal=${buf.signal} closed=${buf.closed}` +
(buf.spawnErr ? ` spawnErr=${buf.spawnErr.code || buf.spawnErr.message}` : "") +
` | stderr(${buf.err.length}B)=${JSON.stringify(buf.err.slice(0, 200))}` +
` | stdout(${buf.out.length}B)=${JSON.stringify(buf.out.slice(-200))}`;
}
async function ltWait(cond, ms = 9000) { async function ltWait(cond, ms = 9000) {
const start = Date.now(); const start = Date.now();
while (Date.now() - start < ms) { if (cond()) return true; await new Promise(r => setTimeout(r, 40)); } while (Date.now() - start < ms) { if (cond()) return true; await new Promise(r => setTimeout(r, 40)); }
return false; return false;
} }
async function ltFreePort() {
const srv = _ltNetServer();
await new Promise(r => srv.listen(0, "127.0.0.1", r));
const p = srv.address().port;
await new Promise(r => srv.close(r));
return p;
}
async function ltPost(port, body) { async function ltPost(port, body) {
try { try {
await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, { await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, {
@@ -1046,31 +1015,29 @@ console.log("\nOCP_LOCAL_TOOLS integration (boot server.mjs):");
test("integration: OCP_LOCAL_TOOLS=1 → the -p spawn receives the POSITIVE wrapper (kills the no-op mutation)", async () => { test("integration: OCP_LOCAL_TOOLS=1 → the -p spawn receives the POSITIVE wrapper (kills the no-op mutation)", async () => {
if (!LT_POSIX) return; // sh fake — skip on Windows CI if (!LT_POSIX) return; // sh fake — skip on Windows CI
const dir = ltMkdir(); const cap = join(dir, "sp.txt"); const fake = ltFake(dir); const dir = ltMkdir(); const cap = join(dir, "sp.txt"); const fake = ltFake(dir);
const port = await ltFreePort(); const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39321", SP_CAPTURE: cap }, dir);
const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port), SP_CAPTURE: cap }, dir);
try { try {
assert.ok(await ltWait(() => buf.out.includes("listening on") || buf.exit != null), `server did not start: ${buf.err.slice(0,200)}`); assert.ok(await ltWait(() => buf.out.includes("listening on") || buf.exit != null), `server did not start: ${buf.err.slice(0,200)}`);
await ltPost(port, { model: "sonnet", messages: [{ role: "user", content: "hi" }] }); await ltPost(39321, { model: "sonnet", messages: [{ role: "user", content: "hi" }] });
assert.ok(await ltWait(() => _ltExists(cap)), "fake claude was spawned and captured --system-prompt"); assert.ok(await ltWait(() => _ltExists(cap)), "fake claude was spawned and captured --system-prompt");
const sp = _ltRead(cap, "utf8"); const sp = _ltRead(cap, "utf8");
assert.ok(sp.includes(LT_POS_MARK), `expected POSITIVE wrapper in --system-prompt, got: ${sp.slice(0,90)}`); assert.ok(sp.includes(LT_POS_MARK), `expected POSITIVE wrapper in --system-prompt, got: ${sp.slice(0,90)}`);
assert.ok(!sp.includes(LT_NEG_MARK), "positive wrapper must REPLACE the negative one, not append"); assert.ok(!sp.includes(LT_NEG_MARK), "positive wrapper must REPLACE the negative one, not append");
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); } } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
}); });
test("integration: flag OFF → the -p spawn receives the EXACT negative wrapper (default path byte-for-byte)", async () => { test("integration: flag OFF → the -p spawn receives the EXACT negative wrapper (default path byte-for-byte)", async () => {
if (!LT_POSIX) return; if (!LT_POSIX) return;
const dir = ltMkdir(); const cap = join(dir, "sp.txt"); const fake = ltFake(dir); const dir = ltMkdir(); const cap = join(dir, "sp.txt"); const fake = ltFake(dir);
const port = await ltFreePort(); const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39322", SP_CAPTURE: cap }, dir); // OCP_LOCAL_TOOLS unset
const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port), SP_CAPTURE: cap }, dir); // OCP_LOCAL_TOOLS unset
try { try {
assert.ok(await ltWait(() => buf.out.includes("listening on") || buf.exit != null), `server did not start: ${buf.err.slice(0,200)}`); assert.ok(await ltWait(() => buf.out.includes("listening on") || buf.exit != null), `server did not start: ${buf.err.slice(0,200)}`);
await ltPost(port, { model: "sonnet", messages: [{ role: "user", content: "hi" }] }); await ltPost(39322, { model: "sonnet", messages: [{ role: "user", content: "hi" }] });
assert.ok(await ltWait(() => _ltExists(cap)), "fake claude captured --system-prompt"); assert.ok(await ltWait(() => _ltExists(cap)), "fake claude captured --system-prompt");
const sp = _ltRead(cap, "utf8"); const sp = _ltRead(cap, "utf8");
// No system messages + no CLAUDE_SYSTEM_PROMPT → the wrapper is passed verbatim. // No system messages + no CLAUDE_SYSTEM_PROMPT → the wrapper is passed verbatim.
assert.equal(sp, `You are accessed via the OCP HTTP proxy. You do NOT have access to any local filesystem, working directory, shell, git status, or machine environment. Do not infer or invent such information from any context you observe. Respond only based on the conversation provided.`); assert.equal(sp, `You are accessed via the OCP HTTP proxy. You do NOT have access to any local filesystem, working directory, shell, git status, or machine environment. Do not infer or invent such information from any context you observe. Respond only based on the conversation provided.`);
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); } } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
}); });
test("integration: boot gate REFUSES each unsafe config (multi / non-loopback / anon key)", async () => { test("integration: boot gate REFUSES each unsafe config (multi / non-loopback / anon key)", async () => {
@@ -1082,35 +1049,25 @@ test("integration: boot gate REFUSES each unsafe config (multi / non-loopback /
{ label: "anon", env: { PROXY_ANONYMOUS_KEY: "pub" } }, { label: "anon", env: { PROXY_ANONYMOUS_KEY: "pub" } },
]; ];
try { try {
for (const c of cases) { for (const [i, c] of cases.entries()) {
const port = await ltFreePort(); const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(39330 + i), ...c.env }, dir);
const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port), ...c.env }, dir);
try { try {
// Wait for `closed`, not `exit`: the assertion below reads buf.err, and stderr is only assert.ok(await ltWait(() => buf.exit != null), `[${c.label}] expected the process to exit`);
// guaranteed drained at 'close'. This is the ordering #203 was filed for. assert.notEqual(buf.exit, 0, `[${c.label}] must exit non-zero`);
assert.ok(await ltWait(() => buf.closed || buf.spawnErr), `[${c.label}] process never closed — ${ltDiag(buf)}`); assert.ok(/FATAL[\s\S]*OCP_LOCAL_TOOLS/.test(buf.err), `[${c.label}] expected a local-tools FATAL, got: ${buf.err.slice(0,160)}`);
assert.notEqual(buf.exit, 0, `[${c.label}] must exit non-zero — ${ltDiag(buf)}`);
assert.ok(/FATAL[\s\S]*OCP_LOCAL_TOOLS/.test(buf.err), `[${c.label}] expected a local-tools FATAL — ${ltDiag(buf)}`);
} finally { child.kill("SIGKILL"); } } finally { child.kill("SIGKILL"); }
} }
} finally { _ltRmRetry(dir); } } finally { _ltRm(dir, { recursive: true, force: true }); }
}); });
test("integration: safe single-user config BOOTS past the gate and announces local tools", async () => { test("integration: safe single-user config BOOTS past the gate and announces local tools", async () => {
if (!LT_POSIX) return; if (!LT_POSIX) return;
const dir = ltMkdir(); const fake = ltFake(dir); const dir = ltMkdir(); const fake = ltFake(dir);
const port = await ltFreePort(); const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39340" }, dir); // loopback + none
const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port) }, dir); // loopback + none
try { try {
// Same race as #199, one line over: "Local tools: ON" is written 14 console.log calls AFTER assert.ok(await ltWait(() => buf.out.includes("listening on")), `safe config must boot, got: ${buf.err.slice(0,200)}`);
// "listening on", so gating on the boot marker and then asserting the announcement can read a assert.ok(buf.out.includes("Local tools: ON"), "startup must announce local tools when active");
// buffer holding only the first chunk. Wait for the line actually under assertion. Measured by } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
// review at 9/200 before this change and 0/200 after — it was the suite's top flake.
assert.ok(await ltWait(() => buf.out.includes("Local tools: ON") || buf.closed || buf.spawnErr),
`startup must announce local tools when active — ${ltDiag(buf)}`);
assert.ok(buf.out.includes("Local tools: ON"),
`startup must announce local tools when active — ${ltDiag(buf)}`);
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); }
}); });
test("integration: TUI mode → flag is announced INERT (not 'ON'), boot not refused", async () => { test("integration: TUI mode → flag is announced INERT (not 'ON'), boot not refused", async () => {
@@ -1118,22 +1075,12 @@ test("integration: TUI mode → flag is announced INERT (not 'ON'), boot not ref
const dir = ltMkdir(); const fake = ltFake(dir); const dir = ltMkdir(); const fake = ltFake(dir);
// Non-loopback would normally trip the local-tools gate; under TUI the flag is inert so the // Non-loopback would normally trip the local-tools gate; under TUI the flag is inert so the
// gate must NOT fire on its behalf. Use loopback here to isolate TUI's own guards from ours. // gate must NOT fire on its behalf. Use loopback here to isolate TUI's own guards from ours.
const port = await ltFreePort(); const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_TUI_MODE: "true", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39341" }, dir);
const { child, buf } = ltBoot({ OCP_LOCAL_TOOLS: "1", CLAUDE_TUI_MODE: "true", CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port) }, dir);
try { try {
// Wait for the line actually under assertion, not for a proxy signal. "listening on" and the assert.ok(await ltWait(() => buf.out.includes("listening on") || buf.exit != null), `did not start: ${buf.err.slice(0,200)}`);
// inert-flag warning are written independently, so gating on the former and then asserting assert.ok(!buf.out.includes("Local tools: ON"), "must NOT claim local tools are ON in TUI mode (the wrapper is unused there)");
// the latter is a race — the flake #199 was filed for. Still bounded by the same timeout, and assert.ok(/ignored in TUI mode/.test(buf.out + buf.err), "must warn that OCP_LOCAL_TOOLS is inert under TUI");
// the boot markers are kept in the predicate so a failed boot ends the wait immediately } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
// rather than burning it.
const ready = await ltWait(() => /ignored in TUI mode/.test(buf.out + buf.err)
|| buf.closed || buf.spawnErr);
assert.ok(ready, `no inert-flag warning appeared — ${ltDiag(buf)}`);
assert.ok(/ignored in TUI mode/.test(buf.out + buf.err),
`must warn that OCP_LOCAL_TOOLS is inert under TUI — ${ltDiag(buf)}`);
assert.ok(!buf.out.includes("Local tools: ON"),
`must NOT claim local tools are ON in TUI mode (the wrapper is unused there) — ${ltDiag(buf)}`);
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); }
}); });
test("integration: toggling OCP_LOCAL_TOOLS invalidates the standard response cache (epoch fold)", async () => { test("integration: toggling OCP_LOCAL_TOOLS invalidates the standard response cache (epoch fold)", async () => {
@@ -1151,11 +1098,11 @@ test("integration: toggling OCP_LOCAL_TOOLS invalidates the standard response ca
} finally { child.kill("SIGKILL"); } } finally { child.kill("SIGKILL"); }
}; };
try { try {
const off = await bootOnce({}, await ltFreePort()); // caches "OK" under epoch(negative) const off = await bootOnce({}, 39350); // caches "OK" under epoch(negative)
const on = await bootOnce({ OCP_LOCAL_TOOLS: "1" }, await ltFreePort()); // same DB, epoch(positive) → must MISS → re-spawn const on = await bootOnce({ OCP_LOCAL_TOOLS: "1" }, 39351); // same DB, epoch(positive) → must MISS → re-spawn
assert.equal(off, 1, "first request (cache empty) must spawn claude"); assert.equal(off, 1, "first request (cache empty) must spawn claude");
assert.equal(on, 1, "after toggling the flag the identical request must NOT be served from the old cache (epoch differs → re-spawn)"); assert.equal(on, 1, "after toggling the flag the identical request must NOT be served from the old cache (epoch differs → re-spawn)");
} finally { _ltRmRetry(dir); } } finally { _ltRm(dir, { recursive: true, force: true }); }
}); });
// ── active-request counter is paired to the process lifecycle (#180 / #193) ── // ── active-request counter is paired to the process lifecycle (#180 / #193) ──
@@ -1191,6 +1138,13 @@ function ltSpreadThrowCount(stackKb) {
return Number(String(_ltExecFile(process.execPath, [`--stack-size=${stackKb}`, "-e", src], { encoding: "utf8" })).trim()) || 0; return Number(String(_ltExecFile(process.execPath, [`--stack-size=${stackKb}`, "-e", src], { encoding: "utf8" })).trim()) || 0;
} catch { return 0; } } catch { return 0; }
} }
async function ltFreePort() {
const srv = _ltNetServer();
await new Promise(r => srv.listen(0, "127.0.0.1", r));
const p = srv.address().port;
await new Promise(r => srv.close(r));
return p;
}
async function ltPostStatus(port, body) { async function ltPostStatus(port, body) {
try { try {
const r = await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, { const r = await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, {
@@ -1237,7 +1191,7 @@ test("integration: a synchronous pre-spawn throw must not leak stats.activeReque
const active = (await r.json()).requests.active; const active = (await r.json()).requests.active;
assert.equal(active, 0, assert.equal(active, 0,
`3 requests threw before their spawn; the counter must be back to 0, got ${active} (this is the #180 leak)`); `3 requests threw before their spawn; the counter must be back to 0, got ${active} (this is the #180 leak)`);
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); } } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
}); });
// ── Cache keys hash the RESOLVED model, not the alias string (#194) ────────── // ── Cache keys hash the RESOLVED model, not the alias string (#194) ──────────
@@ -1265,38 +1219,36 @@ console.log("\nCache key resolves the model alias (#194):");
test("integration: an alias and its canonical target share ONE cache slot (normal path)", async () => { test("integration: an alias and its canonical target share ONE cache slot (normal path)", async () => {
if (!LT_POSIX) return; if (!LT_POSIX) return;
const dir = ltMkdir(); const fake = ltFake(dir); const counter = join(dir, "spawns.txt"); const dir = ltMkdir(); const fake = ltFake(dir); const counter = join(dir, "spawns.txt");
const port = await ltFreePort(); const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39360", CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir);
const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port), CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir);
try { try {
assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`); assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`);
_ltWrite(counter, "0"); _ltWrite(counter, "0");
const msgs = [{ role: "user", content: "alias-resolution-probe" }]; const msgs = [{ role: "user", content: "alias-resolution-probe" }];
await ltPost(port, { model: "sonnet", messages: msgs }); // miss → spawn await ltPost(39360, { model: "sonnet", messages: msgs }); // miss → spawn
await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 3000); await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 3000);
await ltPost(port, { model: "claude-sonnet-5", messages: msgs }); // same resolved model → HIT await ltPost(39360, { model: "claude-sonnet-5", messages: msgs }); // same resolved model → HIT
await new Promise(r => setTimeout(r, 600)); await new Promise(r => setTimeout(r, 600));
assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1, assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1,
"the canonical id must hit the slot the alias populated — a 2nd spawn means the key still hashes the raw alias"); "the canonical id must hit the slot the alias populated — a 2nd spawn means the key still hashes the raw alias");
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); } } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
}); });
test("integration: an alias and its canonical target share ONE cache slot (STRUCTURED path)", async () => { test("integration: an alias and its canonical target share ONE cache slot (STRUCTURED path)", async () => {
if (!LT_POSIX) return; if (!LT_POSIX) return;
const dir = ltMkdir(); const fake = ltFakeJson(dir); const counter = join(dir, "spawns.txt"); const dir = ltMkdir(); const fake = ltFakeJson(dir); const counter = join(dir, "spawns.txt");
const port = await ltFreePort(); const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39361", CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir);
const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port), CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir);
try { try {
assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`); assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`);
_ltWrite(counter, "0"); _ltWrite(counter, "0");
const rf = { type: "json_schema", json_schema: { name: "probe", schema: LT_SCHEMA } }; const rf = { type: "json_schema", json_schema: { name: "probe", schema: LT_SCHEMA } };
const msgs = [{ role: "user", content: "structured-alias-probe" }]; const msgs = [{ role: "user", content: "structured-alias-probe" }];
await ltPost(port, { model: "sonnet", messages: msgs, response_format: rf }); await ltPost(39361, { model: "sonnet", messages: msgs, response_format: rf });
await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 4000); await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 4000);
await ltPost(port, { model: "claude-sonnet-5", messages: msgs, response_format: rf }); await ltPost(39361, { model: "claude-sonnet-5", messages: msgs, response_format: rf });
await new Promise(r => setTimeout(r, 600)); await new Promise(r => setTimeout(r, 600));
assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1, assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1,
"structured cache key must resolve the alias too — this is the path the epoch-only fix missed"); "structured cache key must resolve the alias too — this is the path the epoch-only fix missed");
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); } } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
}); });
// MODEL_MAP is models[] + aliases + legacyAliases, so resolving covers legacyAliases for free. // MODEL_MAP is models[] + aliases + legacyAliases, so resolving covers legacyAliases for free.
@@ -1305,19 +1257,18 @@ test("integration: an alias and its canonical target share ONE cache slot (STRUC
test("integration: a legacyAlias shares ONE cache slot with its canonical target", async () => { test("integration: a legacyAlias shares ONE cache slot with its canonical target", async () => {
if (!LT_POSIX) return; if (!LT_POSIX) return;
const dir = ltMkdir(); const fake = ltFake(dir); const counter = join(dir, "spawns.txt"); const dir = ltMkdir(); const fake = ltFake(dir); const counter = join(dir, "spawns.txt");
const port = await ltFreePort(); const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39364", CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir);
const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port), CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir);
try { try {
assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`); assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`);
_ltWrite(counter, "0"); _ltWrite(counter, "0");
const msgs = [{ role: "user", content: "legacy-alias-probe" }]; const msgs = [{ role: "user", content: "legacy-alias-probe" }];
await ltPost(port, { model: "claude-haiku-4-5", messages: msgs }); // legacyAlias await ltPost(39364, { model: "claude-haiku-4-5", messages: msgs }); // legacyAlias
await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 3000); await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 3000);
await ltPost(port, { model: "claude-haiku-4-5-20251001", messages: msgs }); // canonical await ltPost(39364, { model: "claude-haiku-4-5-20251001", messages: msgs }); // canonical
await new Promise(r => setTimeout(r, 600)); await new Promise(r => setTimeout(r, 600));
assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1, assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1,
"legacyAliases live in MODEL_MAP too — resolving must collapse them onto the canonical slot"); "legacyAliases live in MODEL_MAP too — resolving must collapse them onto the canonical slot");
} finally { child.kill("SIGKILL"); _ltRmRetry(dir); } } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); }
}); });
test("integration: a config change invalidates the STRUCTURED cache too (closes the #177 gap)", async () => { test("integration: a config change invalidates the STRUCTURED cache too (closes the #177 gap)", async () => {
@@ -1336,11 +1287,11 @@ test("integration: a config change invalidates the STRUCTURED cache too (closes
} finally { child.kill("SIGKILL"); } } finally { child.kill("SIGKILL"); }
}; };
try { try {
const off = await bootOnce({}, await ltFreePort()); // caches under epoch(negative wrapper) const off = await bootOnce({}, 39362); // caches under epoch(negative wrapper)
const on = await bootOnce({ OCP_LOCAL_TOOLS: "1" }, await ltFreePort()); // same DB, epoch differs → must re-spawn const on = await bootOnce({ OCP_LOCAL_TOOLS: "1" }, 39363); // same DB, epoch differs → must re-spawn
assert.equal(off, 1, "first structured request (cache empty) must spawn claude"); assert.equal(off, 1, "first structured request (cache empty) must spawn claude");
assert.equal(on, 1, "structured cache must honor CONFIG_EPOCH — before #194 it omitted the epoch entirely and served the stale answer"); assert.equal(on, 1, "structured cache must honor CONFIG_EPOCH — before #194 it omitted the epoch entirely and served the stale answer");
} finally { _ltRmRetry(dir); } } finally { _ltRm(dir, { recursive: true, force: true }); }
}); });
// ── Upgrade Tests ── // ── Upgrade Tests ──