Acceptance harness and pre-release hardening

[dennisdornon]

What this is

33 commits preparing @mainwp/control for the next release. Three arcs:

Acceptance harness (tests/acceptance/). Packs the tarball into a fresh consumer (or runs from source) and drives the installed mainwpcontrol binary through 20 scenarios against fixture and live Dashboards: read cross-checks against an independent verifier, error contracts, and safety flows proven by mock-server request recording (dry-run previews once and never confirms, --force skips only the prompt, failed preview fails closed). A graded agent layer drives claude -p through four scenarios and grades the ordered CLI argv plus final answer against ground truth computed before the model runs. Live-writes baseline: 17 passed / 0 failed / 3 skipped (destructive scenarios are fixture-only).

Release-audit remediation. Five fix/verify rounds against Codex's pre-release audit, each round independently re-verified with direct probes. The headline items:

• Keychain credentials are stored bound to the profile's canonical Dashboard identity. Repointing a profile at a different host in profiles.json gets an AuthError, not the password.
• Destructive confirms are fail-closed end to end: a dispatch-stage audit entry before every confirm call, and a transport failure after dispatch exits 3 (OUTCOME_UNKNOWN) with an audit record instead of a generic network error.
• Remote schemas never contribute regexes to ajv: pattern/patternProperties are stripped structurally from every tree node, and the whole tree (arrays and literal data included) counts against a 32-level depth budget.
• Provider boundaries: tool results are key-redacted before entering provider-bound history, redirects are refused, hosted providers refuse http:// base URLs.

Post-audit polish pass (final commit before the changelog). fsync-before-rename atomic writes, prototype-pollution-safe sanitizers, bidi control stripping, bounded audit free text, case-sensitive ability-name validation, explicit live-test gating, and subprocess/artifact bounds in the harness.

Breaking change

Existing beta users must run mainwpcontrol login once per profile: unbound credentials stored by earlier versions are refused for authenticated requests. The changelog leads with this.

Not in this PR

Version bump, tag, and npm publish happen after merge.

Verification

• npm run typecheck, npm run lint (0 errors), npm test including the process suite: 938/938 passing
• npm run build and git diff --check clean
• Live acceptance baseline 17/0/3 against the testbed Dashboard
• Codex verified the remediation state at 8e4ab76 with direct probes; the polish pass on top has matching tests for every change

Summary by CodeRabbit

• New Features
  • Added/expanded acceptance-test harness scenarios (agent and write flows) and documentation.
  • Added more CLI --json/envelope contract coverage and deterministic verification tooling.
• Bug Fixes
  • Improved job/batch status parsing (including cancelled) and stricter API response validation.
  • Refined jobs watch/batch timeout and improved SIGINT handling to consistently exit with 130.
• Security
  • Strengthened redaction and sanitization for errors, config/doctor/login output, and destructive execution safety/auditing.
• Documentation
  • Updated terminal guides (Windows/PowerShell JSON input), cron instructions, and required Node.js version (20.18.1+).
• Chores
  • Tightened CI Node.js test matrix and added test-results/ to .gitignore.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

This PR adds acceptance-test infrastructure, strengthens destructive execution and provider protocol handling, introduces bounded and sanitized I/O, updates credential and configuration safety, standardizes JSON/error contracts, and documents the resulting CLI, automation, and runtime requirements.

Changes

Core safety and execution

Layer / File(s)	Summary
Policy enforcement and destructive execution src/core/*, src/commands/abilities/run.ts	Execution now shares policy validation, fails closed when previews fail, records dispatch and unknown outcomes, and includes preview data in destructive result envelopes.
Chat and provider protocol handling src/chat/*	Chat sends are serialized, tool aliases and native IDs are preserved, malformed protocol payloads are rejected, provider history is redacted, and context truncation avoids orphaned tool messages.
Transport and response validation src/core/http-client.ts, src/chat/providers/*, src/core/batch-manager.ts	Redirects, response sizes, JSON shapes, SSE streams, job IDs, statuses, and queued-job responses receive bounded and stricter validation.
Credential, config, and output protection src/config/*, src/utils/*, src/output/*, src/lib/base-command.ts	Credentials are identity-bound and masked, writes use safer primitives, audit logs are bounded and permission-healed, and terminal/error output is sanitized.
CLI behavior and verification src/commands/*, src/__tests__/*	JSON envelopes, exit codes, signal cancellation, keychain rollback, input validation, and command harness behavior are covered by updated process and E2E tests.
Acceptance harness tests/acceptance/*, docs/acceptance-testing.md	Fixture/live and source/packed acceptance modes add independent verification, guarded writes, agent scenarios, artifact recording, redaction audits, and reproducible reports.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AcceptanceRunner
  participant CLIInvoker
  participant MainWPControl
  participant MockServer
  participant IndependentVerifier
  AcceptanceRunner->>CLIInvoker: run scenario command
  CLIInvoker->>MainWPControl: execute CLI with isolated environment
  MainWPControl->>MockServer: request ability or destructive preview
  MockServer-->>MainWPControl: JSON result or preview failure
  MainWPControl-->>CLIInvoker: JSON envelope and exit code
  AcceptanceRunner->>IndependentVerifier: verify catalog and resource state
  IndependentVerifier-->>AcceptanceRunner: independent response
  AcceptanceRunner->>AcceptanceRunner: record artifacts and evaluate scenario

Loading

Possibly related PRs

• mainwp/mainwp-control#1 — Related keychain timeout and credential-handling changes.
• mainwp/mainwp-control#4 — Related Windows PowerShell JSON quoting and --input-file documentation.
• mainwp/mainwp-control#5 — Related debug redaction and SIGINT exit-code behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main themes of the change: acceptance harness work and release hardening.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/acceptance-harness

---

_{Comment @coderabbitai help to get the list of available commands.}

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​oclif/​plugin-help@​6.2.38 ⏵ 6.2.53				-2
	tsx@​4.21.0
	@​oclif/​plugin-autocomplete@​3.2.39 ⏵ 3.2.53				-1
	undici@​7.24.4 ⏵ 7.28.0	-3	+31	+1
	@​oclif/​core@​4.9.0 ⏵ 4.11.14	+1			-1

View full report

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/chat/providers/gemini.ts (1)

227-238: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Streaming tool-call IDs should preserve Gemini-provided IDs and include an index. The streaming path always synthesizes fc_${Date.now()} and drops part.functionCall.id, so multiple functionCall parts in the same chunk can collide and later functionResponse matching loses the original correlation ID. Mirror the convertResponse fallback with part.functionCall.id ?? \fc_${Date.now()}_${index}``.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/chat/providers/gemini.ts` around lines 227 - 238, Update the streaming
functionCall handling to preserve the provider-supplied ID and avoid collisions
by using part.functionCall.id when present, otherwise falling back to the
existing timestamp-based ID with the available part index appended. Keep the
rest of the yielded toolCall structure unchanged.

🧹 Nitpick comments (9)

src/__tests__/process/fixtures/cli-runner.ts (1)

108-137: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Add an error handler to runCLIWithSignal so spawn failures reject/resolve instead of hanging.

Unlike runWithStdin (which handles child.on('error') at Line 207), this helper only resolves on 'close'. If spawn fails (e.g., bad BIN_PATH), no 'close' fires and the promise stays pending until the Vitest timeout, masking the real failure.

♻️ Add error handling

     child.stdout.on('data', (chunk: Buffer) => stdoutChunks.push(chunk));
     child.stderr.on('data', (chunk: Buffer) => stderrChunks.push(chunk));
+    child.on('error', (err) => {
+      clearTimeout(signalTimer);
+      clearTimeout(timeoutTimer);
+      resolve({ stdout: '', stderr: err.message, exitCode: 1, duration: Date.now() - start });
+    });
     child.on('close', (code, closeSignal) => {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/process/fixtures/cli-runner.ts` around lines 108 - 137, Update
runCLIWithSignal to register a child.on('error') handler that settles the
promise when spawn fails, matching the existing runWithStdin behavior. Ensure
the handler clears both timers and rejects or resolves with the underlying spawn
error instead of waiting indefinitely, while preserving the current close-event
result handling.

src/core/batch-manager.ts (1)

260-267: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate terminal-status predicate across batch-manager.ts and watch.ts. Both files independently define an identical "is job status terminal" check, updated in lockstep for cancelled in this PR but with nothing enforcing they stay in sync going forward.

• src/core/batch-manager.ts#L260-L267: export the client-facing terminal predicate (equivalent to the current private isTerminalStatus) as a shared helper alongside JobStatusType.
• src/commands/jobs/watch.ts#L40-L43: drop the locally duplicated isTerminalStatus and import the shared helper from batch-manager.ts instead.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/batch-manager.ts` around lines 260 - 267, The terminal-status logic
is duplicated across the batch manager and watch command. In
src/core/batch-manager.ts lines 260-267, export the client-facing predicate
equivalent to private isTerminalStatus alongside JobStatusType; in
src/commands/jobs/watch.ts lines 40-43, remove the local isTerminalStatus and
import and use the shared helper instead.

src/lib/base-command.ts (1)

314-343: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Bound redactDebugValue recursion like sanitizeErrorValue.

No depth limit or cycle (WeakSet) guard here, unlike the sibling hardening added to sanitizeErrorValue in this same PR. A deeply-nested or self-referential debug-context value would recurse unbounded.

♻️ Proposed bounded refactor

-  private redactDebugContext(context: Record<string, unknown>): Record<string, unknown> {
+  private redactDebugContext(
+    context: Record<string, unknown>,
+    depth = 0,
+    seen: WeakSet<object> = new WeakSet()
+  ): Record<string, unknown> {
     const redacted: Record<string, unknown> = {};

     for (const [key, value] of Object.entries(context)) {
-      redacted[key] = isSensitiveKey(key) ? '[REDACTED]' : this.redactDebugValue(value);
+      redacted[key] = isSensitiveKey(key) ? '[REDACTED]' : this.redactDebugValue(value, depth, seen);
     }

     return redacted;
   }

-  private redactDebugValue(value: unknown): unknown {
+  private redactDebugValue(value: unknown, depth = 0, seen: WeakSet<object> = new WeakSet()): unknown {
     if (typeof value === 'string' && value.length > 300) {
       return `${value.slice(0, 297)}...`;
     }

+    if (depth >= MAX_DEBUG_DEPTH) {
+      return '[TRUNCATED]';
+    }
+
     if (Array.isArray(value)) {
-      return value.map((item) => this.redactDebugValue(item));
+      if (seen.has(value)) return '[TRUNCATED]';
+      seen.add(value);
+      return value.map((item) => this.redactDebugValue(item, depth + 1, seen));
     }

     if (value && typeof value === 'object') {
-      return this.redactDebugContext(value as Record<string, unknown>);
+      if (seen.has(value)) return '[TRUNCATED]';
+      seen.add(value);
+      return this.redactDebugContext(value as Record<string, unknown>, depth + 1, seen);
     }

     return value;
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/base-command.ts` around lines 314 - 343, Bound recursion in
redactDebugValue and redactDebugContext using the same depth limit and WeakSet
cycle guard established by sanitizeErrorValue. Ensure deeply nested values are
safely truncated or redacted and self-referential structures do not recurse
indefinitely, while preserving existing string truncation and sensitive-key
handling.

tests/acceptance/lib/artifacts.ts (1)

134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

No-op .replace('Z', 'Z').

This replaces Z with itself and does nothing. Either drop it or replace with the intended transform (e.g. stripping the trailing Z).

Proposed tidy-up

-  const timestamp = startTime.replace(/[-:.]/g, '').replace('Z', 'Z');
+  const timestamp = startTime.replace(/[-:.]/g, '');

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/acceptance/lib/artifacts.ts` at line 134, Remove the no-op
.replace('Z', 'Z') from the timestamp construction in the artifact timestamp
logic, while preserving the existing removal of hyphens, colons, and periods.

tests/acceptance/scenarios/read.ts (1)

97-112: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a page-count safety cap to the read.ts pagination loops. All four loops terminate only on items.length >= total or an empty page. A Dashboard that reports a wrong total while returning non-empty pages will loop forever and hang the run — exactly the failure mode cliListAllSites in tests/acceptance/scenarios/types.ts (L138-181) already guards against with MAX_SITE_PAGES. Mirror that guard so a paging bug fails loudly instead of hanging.

• tests/acceptance/scenarios/read.ts#L97-L112: add a max-page guard to cliListAll and throw when exceeded.
• tests/acceptance/scenarios/read.ts#L114-L127: add the same guard to verifierListAll.
• tests/acceptance/scenarios/read.ts#L171-L189: add the same guard to verifierListUpdates.
• tests/acceptance/scenarios/read.ts#L191-L210: add the same guard to cliListUpdates.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/acceptance/scenarios/read.ts` around lines 97 - 112, read.ts pagination
can loop indefinitely when the reported total is incorrect; add a shared
MAX_SITE_PAGES-style cap and throw once it is exceeded. Apply the guard to
cliListAll (tests/acceptance/scenarios/read.ts:97-112), verifierListAll
(114-127), verifierListUpdates (171-189), and cliListUpdates (191-210),
preserving normal termination when totals are reached or pages are empty.

src/chat/tool-envelope.ts (1)

64-110: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Single-pass extractFirstJsonObject
extractFirstJsonObject is quadratic on brace-heavy responses like "{".repeat(n) + "}". Provider maxTokens and SSE line-buffer limits bound some inputs, but LLMResponse.content is still not hard-capped here, so a malformed response can make parsing unnecessarily expensive. A one-pass scan or explicit length cap would avoid that.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/chat/tool-envelope.ts` around lines 64 - 110, The nested scanning in
extractFirstJsonObject can rescan brace-heavy malformed input quadratically.
Refactor this function to scan the text once while tracking JSON string, escape,
and brace-depth state, returning the first valid object candidate without
restarting from each opening brace; alternatively enforce an explicit maximum
scan length before parsing.

src/utils/format.test.ts (1)

124-181: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

LGTM! Note: none of these cases cover the malformed/whitespace-normalized-URL edge case raised in the consolidated comment on src/utils/format.ts#L113-L128 — once that's fixed, consider adding a regression test here (e.g. a dashboardUrl with an embedded tab/leading space that still round-trips through new URL() with non-empty userinfo).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/format.test.ts` around lines 124 - 181, Add a regression test in
the maskUrlUserinfo test suite for a dashboardUrl containing leading or embedded
whitespace that new URL() normalizes while retaining non-empty userinfo. Assert
that maskUrlUserinfo masks the normalized credentials correctly, covering the
malformed/whitespace-normalized URL edge case described for maskUrlUserinfo.

src/core/http-client.ts (1)

195-198: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Optional: cancel the redirect response body before recursing.

The 3xx response body is left unconsumed here before handleRedirect issues the next request. For undici-backed fetch, an unread body can delay socket release/reuse. Redirect bodies are typically empty so impact is small, but an explicit void response.body?.cancel().catch(() => {}) keeps connection handling tidy.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/http-client.ts` around lines 195 - 198, In the redirect branch of
the HTTP client’s request flow, cancel the unread response body before invoking
handleRedirect, safely ignoring any cancellation rejection, then preserve the
existing recursive redirect behavior.

tests/acceptance/lib/commands.ts (1)

36-42: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Optional: avoid O(n²) recomputation in appendBounded.

reduce over all retained chunks runs on every data event, so buffering a large stream in many chunks is quadratic. A running total kept across calls (e.g., module/closure state or a small wrapper object) trims in amortized O(1).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/acceptance/lib/commands.ts` around lines 36 - 42, Optimize
appendBounded by avoiding a full chunks.reduce recalculation on every call.
Maintain the retained byte total across invocations, decrementing it as chunks
are removed and trimming until it is within MAX_STREAM_BYTES; preserve the
existing bounded-buffer behavior and function contract.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/config/show.ts`:
- Around line 274-337: Sanitize the masked secret values before displaying them
in the config output: wrap config.profile.credentialsMasked in
sanitizeSingleLine within the Credentials row and
config.llmProvider.apiKeyMasked in sanitizeSingleLine within the API Key row.
Leave the existing masking and other display behavior unchanged.

In `@src/commands/doctor.ts`:
- Around line 444-466: Update displayReport to sanitize check.message with
sanitizeSingleLine() before colorizing and logging it, preventing embedded line
breaks from creating extra report rows. Leave check.details using
stripControlChars().split('\n') so verbose details continue to support multiline
output.

In `@src/config/keychain.ts`:
- Around line 271-276: Update the docstring near the expectedDashboardUrl
handling to state that legacy unbound entries are readable only when no expected
URL is supplied, while authenticated use with an expected URL throws and
requires a one-time re-login before the credential is rebound. Keep the
identity-bound mismatch behavior and per-invocation environment-variable note
unchanged.

In `@src/utils/format.ts`:
- Around line 113-128: Update maskUrlUserinfo in src/utils/format.ts (lines
113-128) to fail closed: after detecting credentials, apply the redaction and
return a placeholder such as [URL_WITH_CREDENTIALS] if the substitution leaves
the URL unchanged; src/commands/config/show.ts (lines 198-199) and
src/commands/doctor.ts (lines 214-215) require no direct changes because they
inherit this fix.

---

Outside diff comments:
In `@src/chat/providers/gemini.ts`:
- Around line 227-238: Update the streaming functionCall handling to preserve
the provider-supplied ID and avoid collisions by using part.functionCall.id when
present, otherwise falling back to the existing timestamp-based ID with the
available part index appended. Keep the rest of the yielded toolCall structure
unchanged.

---

Nitpick comments:
In `@src/__tests__/process/fixtures/cli-runner.ts`:
- Around line 108-137: Update runCLIWithSignal to register a child.on('error')
handler that settles the promise when spawn fails, matching the existing
runWithStdin behavior. Ensure the handler clears both timers and rejects or
resolves with the underlying spawn error instead of waiting indefinitely, while
preserving the current close-event result handling.

In `@src/chat/tool-envelope.ts`:
- Around line 64-110: The nested scanning in extractFirstJsonObject can rescan
brace-heavy malformed input quadratically. Refactor this function to scan the
text once while tracking JSON string, escape, and brace-depth state, returning
the first valid object candidate without restarting from each opening brace;
alternatively enforce an explicit maximum scan length before parsing.

In `@src/core/batch-manager.ts`:
- Around line 260-267: The terminal-status logic is duplicated across the batch
manager and watch command. In src/core/batch-manager.ts lines 260-267, export
the client-facing predicate equivalent to private isTerminalStatus alongside
JobStatusType; in src/commands/jobs/watch.ts lines 40-43, remove the local
isTerminalStatus and import and use the shared helper instead.

In `@src/core/http-client.ts`:
- Around line 195-198: In the redirect branch of the HTTP client’s request flow,
cancel the unread response body before invoking handleRedirect, safely ignoring
any cancellation rejection, then preserve the existing recursive redirect
behavior.

In `@src/lib/base-command.ts`:
- Around line 314-343: Bound recursion in redactDebugValue and
redactDebugContext using the same depth limit and WeakSet cycle guard
established by sanitizeErrorValue. Ensure deeply nested values are safely
truncated or redacted and self-referential structures do not recurse
indefinitely, while preserving existing string truncation and sensitive-key
handling.

In `@src/utils/format.test.ts`:
- Around line 124-181: Add a regression test in the maskUrlUserinfo test suite
for a dashboardUrl containing leading or embedded whitespace that new URL()
normalizes while retaining non-empty userinfo. Assert that maskUrlUserinfo masks
the normalized credentials correctly, covering the
malformed/whitespace-normalized URL edge case described for maskUrlUserinfo.

In `@tests/acceptance/lib/artifacts.ts`:
- Line 134: Remove the no-op .replace('Z', 'Z') from the timestamp construction
in the artifact timestamp logic, while preserving the existing removal of
hyphens, colons, and periods.

In `@tests/acceptance/lib/commands.ts`:
- Around line 36-42: Optimize appendBounded by avoiding a full chunks.reduce
recalculation on every call. Maintain the retained byte total across
invocations, decrementing it as chunks are removed and trimming until it is
within MAX_STREAM_BYTES; preserve the existing bounded-buffer behavior and
function contract.

In `@tests/acceptance/scenarios/read.ts`:
- Around line 97-112: read.ts pagination can loop indefinitely when the reported
total is incorrect; add a shared MAX_SITE_PAGES-style cap and throw once it is
exceeded. Apply the guard to cliListAll
(tests/acceptance/scenarios/read.ts:97-112), verifierListAll (114-127),
verifierListUpdates (171-189), and cliListUpdates (191-210), preserving normal
termination when totals are reached or pages are empty.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ee99b1e7-b7c4-4f44-a4e2-0926ddc37a70

📥 Commits

Reviewing files that changed from the base of the PR and between a4228eb and 3e98dd0.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (127)

• .github/workflows/ci.yml
• .gitignore
• CHANGELOG.md
• README.md
• docs/acceptance-testing.md
• docs/workflows/daily-health-check.md
• docs/workflows/input-from-file.md
• docs/workflows/monthly-batch-updates.md
• docs/workflows/plugin-deployment-verification.md
• package.json
• src/__tests__/e2e/batch-polling-flow.test.ts
• src/__tests__/e2e/chat-destructive-flow.test.ts
• src/__tests__/e2e/command-workflows.test.ts
• src/__tests__/e2e/exit-codes.test.ts
• src/__tests__/e2e/json-contract.test.ts
• src/__tests__/e2e/login-abilities-flow.test.ts
• src/__tests__/e2e/non-tty-behavior.test.ts
• src/__tests__/e2e/test-helpers.ts
• src/__tests__/process/abilities-run.test.ts
• src/__tests__/process/auth.test.ts
• src/__tests__/process/batch-wait.test.ts
• src/__tests__/process/config-show.test.ts
• src/__tests__/process/doctor.test.ts
• src/__tests__/process/exit-codes.test.ts
• src/__tests__/process/fixtures/api-responses.ts
• src/__tests__/process/fixtures/cli-runner.ts
• src/__tests__/process/live-api.test.ts
• src/__tests__/process/live-workflow-docs.test.ts
• src/__tests__/process/safety.test.ts
• src/__tests__/process/scenarios.test.ts
• src/chat/chat-engine.test.ts
• src/chat/chat-engine.ts
• src/chat/context-window.test.ts
• src/chat/context-window.ts
• src/chat/providers/anthropic.test.ts
• src/chat/providers/anthropic.ts
• src/chat/providers/gemini.test.ts
• src/chat/providers/gemini.ts
• src/chat/providers/openai-compatible.ts
• src/chat/providers/provider-fetch.test.ts
• src/chat/providers/provider-fetch.ts
• src/chat/providers/provider.test.ts
• src/chat/providers/provider.ts
• src/chat/providers/sse-reader.test.ts
• src/chat/providers/sse-reader.ts
• src/chat/providers/streamed-tool-arguments.test.ts
• src/chat/providers/tool-protocol.test.ts
• src/chat/system-prompt.test.ts
• src/chat/system-prompt.ts
• src/chat/tool-envelope.test.ts
• src/chat/tool-envelope.ts
• src/commands/abilities/list.ts
• src/commands/abilities/run.ts
• src/commands/chat.test.ts
• src/commands/chat.ts
• src/commands/config/show.ts
• src/commands/doctor.ts
• src/commands/jobs/watch.test.ts
• src/commands/jobs/watch.ts
• src/commands/login.ts
• src/commands/profile/delete.ts
• src/config/atomic-write.test.ts
• src/config/fs-utils.test.ts
• src/config/fs-utils.ts
• src/config/keychain.test.ts
• src/config/keychain.ts
• src/config/profile-store.test.ts
• src/config/profile-store.ts
• src/core/abilities-executor.test.ts
• src/core/abilities-executor.ts
• src/core/batch-manager.test.ts
• src/core/batch-manager.ts
• src/core/execute-ability-with-policy.test.ts
• src/core/execute-ability-with-policy.ts
• src/core/http-client.test.ts
• src/core/http-client.ts
• src/core/job-id.ts
• src/core/safety-controller.test.ts
• src/core/safety-controller.ts
• src/lib/base-command.test.ts
• src/lib/base-command.ts
• src/output/formatter.test.ts
• src/output/formatter.ts
• src/output/json-envelope.test.ts
• src/output/json-envelope.ts
• src/utils/audit-logger.permissions.test.ts
• src/utils/audit-logger.test.ts
• src/utils/audit-logger.ts
• src/utils/error-sanitizer.test.ts
• src/utils/error-sanitizer.ts
• src/utils/errors.ts
• src/utils/format.test.ts
• src/utils/format.ts
• src/utils/prompt.ts
• src/utils/redaction.test.ts
• src/utils/redaction.ts
• src/utils/retry.test.ts
• src/utils/retry.ts
• src/utils/terminal-sanitizer.test.ts
• src/utils/terminal-sanitizer.ts
• src/validation/input-sanitizer.test.ts
• src/validation/input-sanitizer.ts
• src/validation/sanitize-schema.test.ts
• src/validation/sanitize-schema.ts
• src/validation/schema-validator.test.ts
• src/validation/schema-validator.ts
• tests/acceptance/agent-run.ts
• tests/acceptance/fixtures.ts
• tests/acceptance/lib/artifacts.ts
• tests/acceptance/lib/cli.ts
• tests/acceptance/lib/commands.ts
• tests/acceptance/lib/env.ts
• tests/acceptance/lib/guards.ts
• tests/acceptance/lib/local-registry.ts
• tests/acceptance/lib/pack.ts
• tests/acceptance/lib/redact.ts
• tests/acceptance/lib/verify.ts
• tests/acceptance/run.ts
• tests/acceptance/scenarios/configuration.ts
• tests/acceptance/scenarios/errors.ts
• tests/acceptance/scenarios/index.ts
• tests/acceptance/scenarios/read.ts
• tests/acceptance/scenarios/safety.ts
• tests/acceptance/scenarios/types.ts
• tests/acceptance/scenarios/writes.ts
• vitest.config.ts
• vitest.process.config.ts

💤 Files with no reviewable changes (2)

• src/utils/retry.ts
• src/utils/retry.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/__tests__/process/fixtures/cli-runner.ts`:
- Around line 122-126: Update the child-process handling around the error
listener and close path so child.on('error') records or handles spawn failures
without resolving the run or clearing timeoutTimer after a failed kill. Keep the
close handler as the terminal resolution path, while preserving appropriate
cleanup for genuine spawn failures and the hard-kill fallback.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f774e196-ba01-4828-8ff9-6a5085af9cbf

📥 Commits

Reviewing files that changed from the base of the PR and between 3e98dd0 and 4d991fc.

📒 Files selected for processing (13)

• src/__tests__/process/fixtures/cli-runner.ts
• src/commands/config/show.ts
• src/commands/doctor.ts
• src/commands/jobs/watch.test.ts
• src/commands/jobs/watch.ts
• src/config/keychain.ts
• src/core/batch-manager.ts
• src/core/http-client.ts
• src/lib/base-command.ts
• src/utils/format.test.ts
• src/utils/format.ts
• tests/acceptance/lib/artifacts.ts
• tests/acceptance/scenarios/read.ts

🚧 Files skipped from review as they are similar to previous changes (12)

• src/utils/format.test.ts
• src/utils/format.ts
• tests/acceptance/lib/artifacts.ts
• src/commands/doctor.ts
• src/lib/base-command.ts
• src/commands/jobs/watch.test.ts
• src/commands/jobs/watch.ts
• src/core/http-client.ts
• src/commands/config/show.ts
• tests/acceptance/scenarios/read.ts
• src/config/keychain.ts
• src/core/batch-manager.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/__tests__/process/fixtures/mock-server.ts`:
- Line 44: Update requestWaiters and waitForRequest() so every queued waiter has
timeout or cancellation handling and cannot remain pending indefinitely when its
request is missed. During reset() and shutdown, reject all pending waiters
before clearing the queue, preserving normal resolution when a matching request
arrives. Ensure the runCLIWithSignal readiness gate receives the rejection
rather than hanging.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 40458767-b01a-490c-9e73-30b0f0aa4a09

📥 Commits

Reviewing files that changed from the base of the PR and between 1de9f1d and 8d34d19.

📒 Files selected for processing (3)

• src/__tests__/process/batch-wait.test.ts
• src/__tests__/process/fixtures/cli-runner.ts
• src/__tests__/process/fixtures/mock-server.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/tests/process/fixtures/cli-runner.ts
• src/tests/process/batch-wait.test.ts

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Settle or time-bound every queued waiter.

waitForRequest() can return a promise that never settles, while reset() simply drops its waiter. If the expected request is missed, the runCLIWithSignal(..., server.waitForRequest(...)) readiness gate can hang indefinitely. Add timeout/cancellation support and reject pending waiters before clearing them during reset or shutdown.

Also applies to: 92-92, 181-187

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/process/fixtures/mock-server.ts` at line 44, Update
requestWaiters and waitForRequest() so every queued waiter has timeout or
cancellation handling and cannot remain pending indefinitely when its request is
missed. During reset() and shutdown, reject all pending waiters before clearing
the queue, preserving normal resolution when a matching request arrives. Ensure
the runCLIWithSignal readiness gate receives the rejection rather than hanging.