mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-19 09:44:07 +00:00
fix: -p spawn-home isolation (③ latency 3x) + concurrency queue/429 (⑥) + ocp restart env + /ocp plugin compat (#144)
* fix(server): isolate default -p spawn in credential-free scratch HOME (latency ③) The default (-p/stream-json) spawn inherited the operator's real HOME (global ~/.claude plugins/skills/hooks) and ran with cwd=~/ocp (project CLAUDE.md/skills), loading heavy host context on EVERY request. Measured: pure API floor for haiku "hi" ≈ 1–2s; same CLI in the operator's real HOME/cwd ≈ 10–28s; a clean minimal HOME + CLAUDE_CODE_OAUTH_TOKEN ≈ 3–7s with auth intact. When an OAuth token is resolvable (and OCP_SPAWN_REAL_HOME!=1), spawnClaudeProcess now runs claude under a credential-free minimal scratch HOME (<HOME>/.ocp/spawn-home, no .credentials.json / settings.json / plugins) with cwd = that neutral dir and the resolved token in CLAUDE_CODE_OAUTH_TOKEN (env token is authoritative for -p). Mirrors the TUI path's resolveTuiHome() env-token mode. Falls back to real HOME + inherited cwd when no token resolves (zero regression); OCP_SPAWN_REAL_HOME=1 is a kill-switch. The token is resolved ONCE (memoized via getSpawnHomeMode, reusing getOAuthCredentials, the same resolver the /usage probe uses) and never logged. Adds a startup log line and an additive /health `spawn` block so the operator can confirm isolation is on. ALIGNMENT.md Rule 2 justification: this is an INFRA / PROCESS-MANAGEMENT change (HOME/cwd/env isolation of the spawned process). It does NOT mirror a cli.js wire operation, introduces no new endpoint/header, and adds no API token to the wire path — so there is no cli.js function to cite. CLAUDE_CODE_OAUTH_TOKEN / CLAUDE_CODE_DISABLE_* are existing claude-CLI env contracts already used by the TUI path and getOAuthCredentials. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(server): bounded wait-queue + HTTP 429 for -p concurrency overflow (⑥) spawnClaudeProcess used `if (activeRequests >= MAX_CONCURRENT) throw` → the client got an opaque 500 AND the rejection was uncounted (a 15-concurrent stress run returned 7×500 while /health stats.errors stayed 0). The TUI path already had a bounded-queue semaphore (TuiSemaphore); the -p path did not. Now the -p path reuses TuiSemaphore as `claudeSemaphore = new TuiSemaphore(MAX_CONCURRENT, { maxQueue: CLAUDE_MAX_QUEUE })`. Requests beyond MAX_CONCURRENT WAIT (up to CLAUDE_MAX_QUEUE, default 16) instead of being rejected; only when the queue is ALSO full does the request get HTTP 429 + Retry-After (rate_limit_error, NOT 500), a distinct `concurrency_queue_full` log, and a stats.queueRejections counter surfaced on /health. callClaude / callClaudeStreaming now acquire a slot (acquireClaudeSlot) before spawning; the release fn is wired into the existing idempotent cleanup() so the slot is freed on EVERY exit path (close/error/timeout/abort) — the #37/#40 slot-leak guard. MAX_CONCURRENT semantics (max concurrent claude procs) are unchanged; only overflow handling changed from throw-500 to queue-then-429. claudeSemaphore.limit is kept in sync with runtime /settings maxConcurrent changes. Live-verified (fake claude, MAX_CONCURRENT=1 MAX_QUEUE=1): 3 concurrent → exactly one 429 (Retry-After: 7, rate_limit_error body) + two 200s, on BOTH the non-streaming and streaming paths; /health stats.queueRejections=1 while stats.errors=0; after drain activeRequests=0 / inflight=0 / queued=0 and a follow-up request returns 200 (no slot leak). 2 new unit tests (247 passed, 0 failed). ALIGNMENT.md Rule 2 justification: this is an INFRA / PROCESS-MANAGEMENT change (a concurrency queue + backpressure status code in front of the existing spawn). It does NOT mirror a cli.js wire operation, adds no new endpoint or wire header, and introduces no API token — so there is no cli.js function to cite. (Retry-After is a standard HTTP response header on OCP's own 429, not a claude wire header.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ocp): macOS restart uses bootout+bootstrap so plist env is re-read The macOS restart path used `launchctl kickstart -k gui/$uid/dev.ocp.proxy`, which only re-execs the process and reuses launchd's CACHED environment — so a plist EnvironmentVariables edit (CLAUDE_BIND, CLAUDE_CODE_OAUTH_TOKEN, etc.) was silently ignored until a full unload/reload. This is the documented pit-index footgun. `ocp restart` (macOS) now does a full `launchctl bootout` + `bootstrap` of the agent via a new `_launchd_reload` helper, which re-reads the plist EnvironmentVariables so env changes take effect. Success is keyed on the bootstrap (the env-reloading load), not the bootout (which may legitimately fail if the agent is not currently loaded). A missing plist returns failure so the `elif` chain falls through to the legacy label and then to the Linux `systemctl --user restart` path unchanged (systemctl already re-reads its EnvironmentFile). Updates `ocp restart` help text and adds a README Troubleshooting subsection ("Env var change doesn't take effect after restart") with the manual bootout+bootstrap commands and a ps-based verification one-liner. Verified `_launchd_reload` with a stubbed launchctl: missing plist → rc=1 (falls through, no launchctl call); bootout-fail + bootstrap-ok → rc=0; bootstrap-fail → rc=1; call order is bootout then bootstrap. `bash -n ocp` clean; npm test 247/0 (server.mjs untouched). ALIGNMENT.md Rule 2 justification: this changes ONLY the `ocp` CLI wrapper's local service-restart mechanism (launchctl invocation). It does not touch server.mjs, the wire path, any endpoint/header, or any API token — so there is no cli.js function to cite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(readme): document new env vars CLAUDE_MAX_QUEUE / CLAUDE_QUEUE_RETRY_AFTER / OCP_SPAWN_REAL_HOME Release_kit contract (CLAUDE.md Iron Rule 5.5: "new env var → README § Environment Variables table") requires the three env vars added by the perf/concurrency fixes to be in the README table. Adds rows for CLAUDE_MAX_QUEUE (16) + CLAUDE_QUEUE_RETRY_AFTER (5) (FIX ⑥ -p wait-queue + 429) and OCP_SPAWN_REAL_HOME (FIX ③ spawn-home isolation kill-switch), each cross-referencing the additive /health.concurrency and /health.spawn fields. Addresses the independent reviewer's MEDIUM finding. Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ocp-plugin): add openclaw.extensions for OpenClaw 2026.5.27 compat + sync version (②/ocp) ocp-plugin/package.json's openclaw object lacked the 'extensions' field that OpenClaw 2026.5.27 requires to install/load a plugin (matches the sibling olp plugin). Without it the daemon refused to load the local path plugin, breaking /ocp Telegram commands. Version synced 3.12.0 -> 3.16.2 to match the manifest. Plugin-layer change; no server.mjs / ALIGNMENT cli.js surface touched. --------- Co-authored-by: dtzp555 <dtzp555@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2046,6 +2046,45 @@ await asyncTest("wait queue is bounded — run() rejects with tui_queue_full whe
|
||||
assert.equal(sem.inflight, 0);
|
||||
});
|
||||
|
||||
console.log("\n-p concurrency wait-queue (FIX ⑥ — same TuiSemaphore reused for the -p path):");
|
||||
|
||||
// server.mjs reuses TuiSemaphore as `claudeSemaphore = new TuiSemaphore(MAX_CONCURRENT,
|
||||
// { maxQueue: CLAUDE_MAX_QUEUE })` and wraps acquire()/release() in acquireClaudeSlot(). These
|
||||
// tests assert the contract that the 429-mapping depends on: requests beyond the limit QUEUE
|
||||
// (not reject), only an overflow past the queue rejects (→ HTTP 429 in server.mjs), and a
|
||||
// released slot is reusable (the #37/#40 slot-leak guard — no leak on normal completion).
|
||||
await asyncTest("FIX ⑥: requests beyond MAX_CONCURRENT queue, not reject (limit=1, queue=1)", async () => {
|
||||
const sem = new TuiSemaphore(1, { maxQueue: 1 }); // mirrors CLAUDE_MAX_CONCURRENT=1, CLAUDE_MAX_QUEUE=1
|
||||
const g1 = deferred();
|
||||
const inflightP = sem.run(async () => { await g1.p; }); // request 1 — holds the only slot
|
||||
await new Promise((r) => setImmediate(r));
|
||||
assert.equal(sem.inflight, 1, "req1 inflight");
|
||||
const queuedP = sem.run(async () => {}); // request 2 — WAITS (queued), does NOT reject
|
||||
await new Promise((r) => setImmediate(r));
|
||||
assert.equal(sem.queued, 1, "req2 queued (waits), not rejected → would be served, not 429");
|
||||
// request 3 — queue full → reject (server.mjs maps this single case to 429 + Retry-After)
|
||||
await assert.rejects(sem.run(async () => {}), /tui_queue_full|queue/, "req3 overflows → reject (→429)");
|
||||
g1.resolve();
|
||||
await inflightP; await queuedP;
|
||||
assert.equal(sem.inflight, 0, "all slots released after drain (no leak)");
|
||||
assert.equal(sem.queued, 0, "queue fully drained");
|
||||
});
|
||||
|
||||
await asyncTest("FIX ⑥: slot released on normal completion is immediately reusable (no #37/#40 leak)", async () => {
|
||||
const sem = new TuiSemaphore(1, { maxQueue: 16 }); // mirrors default CLAUDE_MAX_QUEUE=16
|
||||
for (let i = 0; i < 5; i++) {
|
||||
await sem.run(async () => { /* a normal, completing turn */ });
|
||||
assert.equal(sem.inflight, 0, `slot released after turn ${i}`);
|
||||
}
|
||||
// Prove the limit still binds after many acquire/release cycles.
|
||||
const g = deferred();
|
||||
const held = sem.run(async () => { await g.p; });
|
||||
await new Promise((r) => setImmediate(r));
|
||||
assert.equal(sem.inflight, 1, "limit still enforced after reuse cycles");
|
||||
g.resolve(); await held;
|
||||
assert.equal(sem.inflight, 0);
|
||||
});
|
||||
|
||||
console.log("\nTUI drift observability (C-5):");
|
||||
|
||||
test("recordTuiEntrypoint: observed 'cli' is NOT a mismatch and sets lastEntrypoint", () => {
|
||||
|
||||
Reference in New Issue
Block a user