mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-22 21:45:08 +00:00
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>
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