[cliffhall]

Closes #2834
Closes #2835
Closes #2836
Closes #2837

All four issues live in the shared --load engine (core/cli/handlers/load-test.ts), so one PR covers them. The CLI and mcpdo get the same fixes.

#2834: Ctrl+C / SIGTERM keeps the partial summary

• The CLI now passes a signal into runToolLoad (clients/cli/src/handlers/load-interrupt.ts). On the first SIGINT/SIGTERM, no new calls start and in-flight calls are allowed to finish. The summary then prints with "stoppedEarly": { "reason": "interrupted", "detail": "received SIGINT" }, and the CLI exits 130 with envelope code interrupted (new EXIT_CODES.INTERRUPTED). Thresholds are not checked on an interrupted run. A second signal exits 130 immediately.
• CLI --load flag refusals (including --load-connections above --load-concurrency, and above 1 over stdio) now use the same load_flag_conflict code as mcpdo. The exit code stays 1. The code is a shared constant, LOAD_FLAG_CONFLICT.

#2835: circuit breaker for a dead server

• The engine stops starting calls once 50 calls in a row have thrown, or once calls have done nothing but throw for 1 s (at least 5 of them). Only thrown calls count. A tool error or a success resets the streak. The summary reports stoppedEarly: { reason: "consecutive_errors", detail }, and a warning goes to stderr in both CLIs. The exit code does not change.
• mcpdo's existing abort on a terminal connection status now reports reason: "connection_lost".

#2836: success and failure latency

• latencyMs now covers ok calls only. The new errorLatencyMs times failures, and the new okThroughput is the success-only throughput.
• --load-max-p99 therefore judges successful calls only, and it fails when none succeeded ("no call succeeded…").
• Each isError result's first text block, trimmed and capped at 200 characters, is tallied into the new toolErrorMessages.

#2837: dropped rate slots

• These fields appear only with --load-rate: plannedStarts, droppedSlots and achievedRate. They are computed over the window the run spent starting calls: the full duration, up to the last start for a request-bound run, or up to the stop.
• When achievedRate is below 90% of the target rate, a warning goes to stderr. --quiet drops it in the CLI.
• A run that reaches its --load-duration measures elapsedMs to at least the end of that window, so a --load-rate 1 run over 5 s now reports 1/s.
• --load-max-dropped is not added.

Behavior change to note

latencyMs and --load-max-p99 used to include failed calls. They no longer do. This is the point of #2836, and it is documented in clients/cli/README.md, clients/mcpdo/README.md and skills/mcpdo/SKILL.md.

Tests

• New clients/cli/__tests__/load-summary.test.ts covers:
  • engine tests for each fix, using a fake clock now shared via helpers/load-clock.ts
  • the interrupt helper, including a second signal and listener cleanup
  • in-process CLI runs: a Ctrl+C run that exits 130 with a partial summary; load_flag_conflict codes; isError text tally; p99 failing with no successes; the rate-shortfall warning with and without --quiet; an HTTP server stopped mid-run that trips the breaker
• mcpdo: human-format rendering of the new fields, the connection_lost abort reason, and an end-to-end rate-shortfall warning.
• npm run local:gate passes.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved load-test defects miscount dropped slots, mishandle interrupts after breaker trips, distort accounting during in-flight draining, and produce false fractional-rate shortfall warnings.

4 open issues

• Allow Ctrl+C to override an existing breaker stop · New
• Avoid breaker stops after start bounds are exhausted · New
• Count all grid slots despite request start cap · New
• Use unrounded achieved rate for shortfall warnings · New

What changed in this PR

This PR addresses #2834–#2837 in the shared load-testing engine used by the CLI and mcpdo, improving interrupted-run reporting and load metrics.

Changes:

• Preserves partial summaries on CLI interrupts and adds a circuit breaker for consecutive thrown errors.
• Separates success and failure latency, adds success-only throughput, and tallies tool-error text.
• Reports planned starts, dropped rate slots and achieved rate, with warnings, documentation and regression coverage.

File	Description
skills/​mcpdo/​SKILL.md	Explains load metrics and early-stop reasons.
core/​cli/​handlers/​load-thresholds.ts	Updates p99 failure messaging for success-only latency.
core/​cli/​handlers/​load-test.ts	Adds stop handling, circuit breaking, split metrics and rate accounting.
core/​cli/​error-handler.ts	Defines interrupted exit code 130.
clients/​mcpdo/​src/​daemon/​server.ts	Names connection-loss aborts.
clients/​mcpdo/​src/​connection/​mcp.ts	Updates p99 option help.
clients/​mcpdo/​src/​connection/​load-flags.ts	Uses the shared load flag-conflict code.
clients/​mcpdo/​src/​connection/​format-human.ts	Renders new metrics and stop details.
clients/​mcpdo/​src/​connection/​dispatch.ts	Prints load-summary warnings.
clients/​mcpdo/​README.md	Documents metrics, warnings and circuit breaking.
clients/​mcpdo/​__tests__/​load.test.ts	Covers updated formatting and rate warnings.
clients/​mcpdo/​__tests__/​daemon-load.test.ts	Checks the connection-loss abort reason.
clients/​cli/​src/​handlers/​load-interrupt.ts	Handles graceful first interrupts and immediate second interrupts.
clients/​cli/​src/​cli.ts	Integrates interrupts, warnings and consistent flag errors.
clients/​cli/​README.md	Documents changed metrics and interruption behavior.
clients/​cli/​__tests__/​load-thresholds.test.ts	Updates success-only p99 expectations.
clients/​cli/​__tests__/​load-test.test.ts	Reuses the clock helper and adapts abort coverage.
clients/​cli/​__tests__/​load-summary.test.ts	Adds engine and CLI regression coverage.
clients/​cli/​__tests__/​helpers/​load-clock.ts	Extracts the shared deterministic load-test clock.

_{Review effort: Balanced}

[cliffhall]

Copilot round 1: all 4 findings are fixed in 562c232, with a reply in each thread.

• An interrupt now outranks a breaker stop that was recorded while calls drained, so Ctrl+C still exits 130.
• The breaker trips only while another call could still start.
• plannedStarts is no longer capped at --load-requests.
• The shortfall warning compares exact counts, so a fractional rate that was held does not warn.

local:gate passes.

[Copilot]

🟡 Changes recommended

Rate accounting can falsely report dropped requests and shortfall warnings when timers wake slightly late or an interrupt occurs after the request or duration boundary.

1 open issue

• Derive rate window from next slot boundary to avoid false drops · New

1 previously missed issue

In code that hasn't changed since last review

End rate window at boundary or stop time during interruption

core/​cli/​handlers/​load-test.ts:671

An interrupt while in-flight calls finish extends the rate-accounting window past the bound that already ended starts. With requests: 1 and rate: 1, interrupting at 5 s while the only call is still running reports five planned starts and four dropped slots; interruption after the duration deadline similarly inflates the counts. Preserve the interrupted reason and exit 130, but end the rate window at the earliest applicable request boundary, duration boundary, or stop time.

4 resolved since last review

• Use unrounded achieved rate for shortfall warnings
• Count all grid slots despite request start cap
• Avoid breaker stops after start bounds are exhausted
• Allow Ctrl+C to override an existing breaker stop

_{Review effort: Balanced}

[cliffhall]

Copilot round 2: both findings are fixed in 2cfa44e.

• Inline: a request-bound window now ends at the scheduler's nextStart, so a slightly late final timer no longer counts as a dropped slot. I replied in the thread.
• Previously missed (no thread to reply into): an interrupt that lands while calls drain past a bound no longer stretches the rate window. The window now ends at the earliest of three points: the duration deadline, the request bound's next slot, or the stop time. The run is still reported as interrupted and still exits 130. New tests cover both the request-bound case and the deadline case.

local:gate passes.

[Copilot]

🔵 Needs a closer look

Older daemon payloads can crash summary rendering, while aborted workers can count uninvoked calls, overstating achieved rates and undercounting dropped slots.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Handle incompatible daemon load summary fields safely

clients/​mcpdo/​src/​connection/​format-human.ts:1024

After a CLI upgrade, ensureDaemon() can reuse an older running daemon whose load summary lacks errorLatencyMs and toolErrorMessages. This branch then throws on e.min, losing the human-readable summary; toolErrorMessages.length below has the same problem. Handle the older payload consistently in rendering and threshold checks, or detect an incompatible daemon and report an actionable restart requirement before running the load test.

Correct rate metrics when claimed slots are not invoked

core/​cli/​handlers/​load-test.ts:617

claimSlot() has already incremented started, so a worker that returns here after another worker aborts the run still counts as a started call without invoking call. The new rate figures therefore overstate achievedRate and undercount dropped slots in interrupted or breaker-stopped summaries. Undo the claimed start before returning, or track actual invocations separately.

1 resolved since last review

• Derive rate window from next slot boundary to avoid false drops

_{Review effort: Balanced}

[cliffhall]

Copilot round 3 had no inline comments. It raised two findings in its "previously missed" block, which have no threads, so I'm answering them here.

• A claimed slot that was never used still counted as started. Fixed in 794b072. A worker that claims a slot after the run has stopped now hands it back, so achievedRate and droppedSlots count only the calls that actually ran. The new test ("does not count a slot claimed after the run stopped as started") fails without the fix.
• An older running daemon sends a summary without the new fields. Declined as out of scope. mcpdo has no version handshake between its front end and the daemon, so every field added to any daemon payload has always had this skew. connections from mcpdo: bring --load-connections and the --load-max-* thresholds to tools/call --load #2800 is one example. Detecting a stale daemon or forcing a restart is a separate capability, not part of CLI --load: Ctrl+C / SIGTERM discards the whole run — no partial summary, exit 130 #2834–--load-rate: dropped slots and the achieved rate are not reported; low-rate runs overstate throughput #2837. Running mcpdo daemon stop after an upgrade clears it.

local:gate passes.

[Copilot]

🔵 Needs a closer look

Human assessment is needed because teardown-time interrupts can produce incorrect exit statuses, while older daemon summaries can crash formatting.

No open issues 🎉

_{Review effort: Balanced}

[cliffhall]

Copilot round 4 had no inline comments and no suppressed block, so I'm closing the review loop.

Its headline sentence repeats two points:

• Older daemon summaries. Already declined in round 3 as pre-existing version skew between the mcpdo front end and its daemon.
• Teardown-time interrupts. A signal that arrives after the run has ended, while the summary is being written or the client is disconnecting, falls to Node's default SIGINT/SIGTERM handling. That is deliberate (see withLoadInterrupt's header): the process dies by the signal, which the shell reports as 130 / 143. This PR does not change that path.

Neither needs a change for #2834–#2837.

[cliffhall]

#2834 follow-up, fixed in f5c2de5. The rollup smoke found that one real SIGINT or SIGTERM to the built CLI exited 130 with an empty stdout.

Cause. when-exit@2.1.5 arrives via atomically and listens for these signals too. After running its cleanups it unconditionally re-raises the signal with process.kill(process.pid, signal). Our listener received that copy as a "second Ctrl+C" and exited before the drained summary was written. The in-process tests call the listener directly, so they could never see the re-raise.

Fix. A repeat of the same signal within REPEAT_GRACE_MS (500 ms) of the first is now treated as a re-raise and ignored. A press after that, or a different signal at any time, still exits 130 at once.

Why a time window. A listener cannot tell where a signal came from: the re-raise is a real kernel signal. Removing or reordering a dependency's listeners would mean reaching into their internals, and when-exit re-raises whatever we do. The re-raise lands within milliseconds. A person pressing again is reacting to the "Press Ctrl+C again" line, so a press inside the window costs at most one extra press.

Test. New clients/cli/__tests__/load-interrupt-e2e.test.ts spawns build/index.js and delivers real signals:

• one SIGINT, and one SIGTERM: each prints the partial summary and exits 130
• a second SIGINT 700 ms later while calls hang: exits 130 at once with no summary

With the window disabled, the two single-signal cases fail, so the test reproduces the bug. The unit tests now drive the window with an injected clock. local:gate passes.

[Copilot]

🔵 Needs a closer look

Human assessment is needed because reused daemons can return legacy load summaries that cause the new formatter to throw before printing results.

No open issues 🎉

_{Review effort: Balanced}

[cliffhall]

Copilot round 5, on f5c2de5: no inline comments, no suppressed block, and no open issues. The headline only repeats the legacy-daemon payload point, which I declined in round 3 as pre-existing front-end/daemon version skew. Closing the review loop.