Skip to content

fix: harden runtime lifecycle boundaries - #704

Merged
yisding merged 6 commits into
mainfrom
agent/fix-subagent-audit-bugs
Jul 30, 2026
Merged

fix: harden runtime lifecycle boundaries#704
yisding merged 6 commits into
mainfrom
agent/fix-subagent-audit-bugs

Conversation

@yisding

@yisding yisding commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Problem

A repeated adversarial subagent audit found correctness gaps across async lifecycle ownership, cleanup retry, provider protocol boundaries, cancellation classification, validation, and debug-bundle recovery. Many failures only appeared under cancellation, concurrent start/stop or connect/disconnect calls, partial provider startup, remote EOF, malformed recovery data, or cleanup failures.

What changed

  • harden Session, VoiceServer, WebRTC, WebTransport, WebSocket, Twilio, outbound-call, STT, and TTS lifecycle state machines
  • retain exact cleanup ownership and make failed/cancelled teardown retryable
  • distinguish caller cancellation from child-task cancellation using per-await baselines
  • make connect/disconnect and persistent provider close paths single-flight where required
  • reject premature provider EOF and invalid terminal/tool event sequences
  • preserve full SQLite journal error/timing data and fail closed on malformed bundle metadata
  • normalize streaming audio geometry and reject unsupported PCM layouts before decoding
  • tighten credential, bearer-token, timeout, capacity, provider-mode, and queue-bound validation
  • make validation artifact redaction fail closed without publishing unsafe output
  • add deterministic regressions for each confirmed behavior

Impact and root causes

The main root cause was optimistic lifecycle state being published before owned async work or cleanup had completed, combined with teardown paths that swallowed errors or lost the exact resource needed for retry. Protocol and recovery paths also accepted incomplete terminal state or normalized malformed evidence instead of failing closed.

The changes prevent false-success startup/teardown, leaked sockets/tasks/sessions, unsafe reconnects, stale provider state, corrupted audio geometry, and incomplete or misleading debug artifacts.

Validation

  • intermediate full local suite: 7227 passed, 306 skipped
  • latest full STT suite: 269 passed, 5 skipped
  • latest full TTS suite: 337 passed, 4 skipped
  • latest WebTransport lifecycle suite: 54 passed, 15 skipped
  • full-source mypy: 278 source files clean
  • changed-file Ruff lint and format checks: clean
  • git diff --check: clean

Draft status

This PR is intentionally draft. The repeated audit is still running, and any remaining confirmed follow-up fixes plus the final full-suite result will be pushed to this same branch before marking it ready.

Summary by CodeRabbit

  • New Features

    • Stronger PCM16/audio validation with safer multichannel resampling/downmixing and frame alignment.
    • Expanded “secret” detection and redaction across validation outputs (including structured and non-UTF8 artifacts).
    • Stricter bundle/journal integrity checks with clearer, line-specific failure reporting.
    • Improved credential usability handling and playground config behavior.
  • Bug Fixes

    • Whitespace-only API keys/tokens are now treated as missing (affects configuration, doctor diagnostics, and provider availability).
    • Improved shutdown, cancellation, reconnection, draining, and retry correctness across server sessions/transports and streaming STT/TTS.
  • Tests

    • Added extensive regression coverage for validation, redaction, bundle loading, audio, replay, and lifecycle edge cases.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yisding, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 21 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c50a39a5-5db8-428f-a91e-0507acc2ffac

📥 Commits

Reviewing files that changed from the base of the PR and between 11cdd9d and 297fd29.

📒 Files selected for processing (6)
  • src/easycat/integrations/agents/responses_api.py
  • src/easycat/validation/_runner_support.py
  • tests/debug/test_replay_and_bundle.py
  • tests/integrations/agents/test_responses_api_bridge.py
  • tests/validation/test_runner_support.py
  • tests/validation/test_slice_runner.py
📝 Walkthrough

Walkthrough

This pull request adds stricter validation and fail-closed handling across audio, providers, journals, artifacts, streaming agents, sessions, transports, and outbound calls. It also adds cancellation-safe cleanup, retry ownership, concurrency coordination, and extensive regression tests.

Changes

EasyCat hardening

Layer / File(s) Summary
Validation and configuration contracts
src/easycat/_numeric.py, src/easycat/_provider_catalog.py, src/easycat/audio_format.py, src/easycat/timeouts.py, src/easycat/validation/*, src/easycat/tts/*
Adds finite-number, credential, audio geometry, provider-alias, timeout, stream-mode, queue-size, endpoint-threshold, and runtime-secret validation.
Journal and bundle integrity
src/easycat/debug/*, tests/debug/*, tests/cli/test_bundles.py, tests/runtime/test_replay.py
Strictly validates journal encoding, JSON shape, sequence ordering, metadata, artifact references, and partial SQLite recovery.
Audio framing and normalization
src/easycat/_audio_utils.py, src/easycat/noise_reduction.py, src/easycat/tts/base.py, src/easycat/stages/vad.py, src/easycat/debugger/*
Adds PCM16 validation, frame carry handling, channel-aware resampling/downmixing/upmixing, and source-format metadata preservation.
Agent and provider streaming
src/easycat/integrations/agents/*, src/easycat/stt/*, src/easycat/tts/*
Adds cancellation-aware SSE processing, tool lifecycle checks, replay state, terminal-event validation, retained socket cleanup, and retryable provider lifecycle operations.
Session, server, and transport lifecycle
src/easycat/session/*, src/easycat/server/*, src/easycat/transports/*, src/easycat/runtime/scope.py
Serializes lifecycle operations, prevents self-cancellation, retains incomplete cleanup, and coordinates staged teardown and reconnect behavior.
Outbound call transactions
src/easycat/telephony/outbound.py
Serializes placement, tracks lifecycle epochs, preserves REST ownership across cancellation, and reconciles stale and terminal calls.
Artifact redaction pipeline
src/easycat/validation/redaction.py, src/easycat/validation/_slice_runner.py, src/easycat/validation/_latency_runner.py
Redacts secrets from text, JSON, JSONL, and binary artifacts; failed redaction excludes evidence and produces validation failures.

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

Possibly related PRs

Suggested reviewers: claude, charliecreates

Poem

A rabbit hops through cleanup bright,
While journals parse in strict moonlight.
Audio frames march in line,
Secret crumbs are made benign.
Sockets retry, old tasks flee—
Carrots for concurrency!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.41% 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 clearly matches the PR’s main focus on hardening lifecycle boundaries across runtime components.
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 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-subagent-audit-bugs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@yisding
yisding marked this pull request as ready for review July 29, 2026 20:49

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e99841a798

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/easycat/stt/websocket_base.py Outdated
Comment thread src/easycat/stt/websocket_base.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 31

Caution

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

⚠️ Outside diff range comments (1)
src/easycat/cli/serve.py (1)

132-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stale docstring: per-connection factory no longer builds EasyConfig.browser.

The docstring still states "Each client connection builds a fresh EasyConfig.browser through the per-connection factory," but the factory at line 127 was changed to build a plain EasyConfig(transport=transport, agent=agent). The validator's own EasyConfig.browser() call is unaffected (still valid for the credential pre-check), but the description of the per-connection factory's behavior is now inaccurate.

📝 Proposed docstring fix
     """Surface the playground's credential requirement before serving.

-    Each client connection builds a fresh ``EasyConfig.browser`` through the
+    Each client connection builds a fresh ``EasyConfig`` through the
     per-connection factory, so a missing ``OPENAI_API_KEY`` would otherwise only
     fail server-side when the first client connects — after the CLI has already
     printed the Open URL and started listening. Construct the same browser preset
-    once up front (the per-connection agent/transport do not affect the
-    credential check) so the catalogued missing-key error (``EASYCAT_E203``)
+    once up front via ``EasyConfig.browser()`` (the per-connection agent/transport
+    do not affect the credential check) so the catalogued missing-key error
+    (``EASYCAT_E203``)
     fails at startup instead. The throwaway config builds no network clients, so
     it is safe to discard.
     """
🤖 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/easycat/cli/serve.py` around lines 132 - 146, Update the docstring of
_validate_playground_config to remove the inaccurate claim that each client
connection builds EasyConfig.browser; describe the per-connection factory as
constructing a plain EasyConfig while preserving the explanation that
EasyConfig.browser() is invoked here to pre-check credentials at startup.
🤖 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/easycat/_audio_utils.py`:
- Around line 604-628: Optimize the de-interleave and re-interleave logic around
resampled_channels by replacing the nested per-frame bytearray slice assignments
with one struct-based unpack and strided channel slicing, then pack the
resampled samples back in one operation. Preserve channel order, sample width,
frame count, and the existing output-length validation while removing the
O(frames × channels) Python loop from the audio hot path.

In `@src/easycat/debug/_bundle_loader.py`:
- Around line 324-345: Update the journal ordering logic around
is_degraded_sentinel so the strict sequence comparison is skipped when the
current record is the journal_degraded sentinel. Do not assign the sentinel’s -1
sequence to previous_sequence; continue validating and tracking all non-sentinel
records in their existing order.

In `@src/easycat/debug/bundle.py`:
- Around line 338-341: The from_partial_journal validation currently enforces
artifact references even when no artifact_root was provided. In
src/easycat/debug/bundle.py lines 338-341, pass the artifact index only when
artifact_root exists, otherwise pass None. In
tests/debug/test_replay_and_bundle.py lines 1393-1409, provide an artifact_root
for the missing-artifact assertion and add coverage confirming recovery succeeds
without artifact_root.
- Around line 479-480: Update the tags handling in the record-building logic to
first validate that row[15] is a string and non-empty, then split it into tags;
otherwise leave the tags field unset. Remove the redundant truthiness comparison
and ensure non-string legacy or partially written values cannot raise
AttributeError from this recovery path.

In `@src/easycat/integrations/agents/responses_api.py`:
- Around line 152-172: In the iterator loop around `next_line` and
`asyncio.wait`, replace the non-cancellation branch’s `next_line.result()`
access with awaiting `next_line`, while preserving the existing
`StopAsyncIteration` handling and line-yielding flow. This ensures the task is
complete even when `cancel_wait` caused `FIRST_COMPLETED` to return.

In `@src/easycat/reconnecting_ws.py`:
- Around line 328-351: The _connection_cleanup_error field in
_close_retained_connection and _release_connection_after_close is redundant
because cleanup failures are already propagated by
_retry_pending_connection_closes. Remove the field’s assignments and any related
initialization, preserving the existing pending-connection ownership tracking
and exception propagation as the single source of truth.

In `@src/easycat/server/transports.py`:
- Around line 270-273: Extract the shared max_sessions validation currently in
the transport initialization into a reusable validator near _validate_timeout,
preserving the bool exclusion, integer requirement, minimum value, and existing
error messages. Replace the duplicated checks in VoiceServerConfig.__post_init__
and the transport code with calls to that validator.
- Around line 47-56: Update _validate_poll_interval to accept a name parameter
like _validate_timeout, and use that parameter in its ValueError message instead
of hardcoding poll_interval_s. Update its existing callers to pass the
appropriate field name, preserving the current positive finite-number
validation.
- Around line 413-415: The drain() flow validates poll_interval_s without using
it; remove this parameter and its _validate_poll_interval call from the drain
signature and implementation, then update all callers and related interfaces to
stop supplying it while preserving the existing drain_timeout_s and
force_timeout_s escalation behavior.

In `@src/easycat/session/_session.py`:
- Around line 1365-1369: Update stop() around the _start_lock acquisition so
stop(force=True) can interrupt a startup that is blocked in _start_task,
including _warmup.run(), transport connection/warmup, health checks, or ingress
startup. Cancel the tracked _start_task or use a bounded lock wait, then proceed
with teardown without waiting indefinitely; preserve normal serialized
startup/teardown behavior for non-forced stops.

In `@src/easycat/stt/base.py`:
- Around line 551-565: The _finish_failed_start() cleanup loop currently
swallows caller cancellation. Preserve the first asyncio.CancelledError while
awaiting the shielded cleanup task, complete cleanup despite subsequent
cancellations, then return the retained cancellation; update
_retry_failed_start_cleanup() to clear cleanup state before re-raising that
cancellation so start_stream() preserves the caller’s cancellation.

In `@src/easycat/telephony/outbound.py`:
- Around line 387-390: Make the intentional tuple-slot coupling explicit in the
pre-gate branch of the outbound placement flow around
_placement_epoch_is_current: document that _lifecycle_changed_error() must
remain in the create_error position so place_call uses _finish_call_placement
and avoids the stale-path call_sid assertion. Preserve the existing tuple
contract and behavior.

In `@src/easycat/transports/twilio_media.py`:
- Around line 1468-1494: Replace the load-bearing assert in the connect flow
with explicit initialization and validation around _lifecycle_lock: initialize
leader and connect_task before entering the lock, assign them within the lock,
and raise a clear RuntimeError if connect_task remains None before awaiting it.
Preserve the existing leader and secondary-caller behavior.

In `@src/easycat/transports/webrtc.py`:
- Around line 692-707: The always-raising helper
_raise_cancelled_offer_after_peer_close must declare a NoReturn return type.
Import NoReturn from typing if needed, update the method annotation, and
preserve its unconditional raise behavior so _handle_offer_locked can rely on
it.
- Around line 339-351: Preserve caller cancellation while shielded connection
rollback completes: in src/easycat/transports/webrtc.py lines 339-351 and
src/easycat/transports/webtransport.py lines 1373-1392, track the cancellation
observed around the webRTC/webTransport connect rollback task, including the
Task.cancelling() delta, and re-raise it after rollback settles, chained from
startup_error unless startup_error is already a CancelledError. Keep the
existing cleanup_error chaining and bare re-raise behavior for non-cancellation
failures.

In `@src/easycat/transports/webtransport.py`:
- Around line 1818-1821: Update the server shutdown logic around
server.wait_closed to probe whether the wait_closed capability exists before
awaiting it, and only skip the call when the attribute is absent. Do not wrap
the await itself in a broad AttributeError handler, so AttributeError raised
during wait_closed propagates normally.

In `@src/easycat/tts/_multi_context_ws.py`:
- Around line 392-398: Bound the graceful close-frame send in the teardown flow
around _aclose_transaction by applying the existing _CANCEL_SEND_TIMEOUT to the
_send_frames(close_frames) await. Preserve the current exception suppression and
only wrap the graceful close-frame transmission, ensuring a wedged lock or
socket cannot block aclose indefinitely.

In `@src/easycat/tts/_ws_base.py`:
- Around line 88-114: Update the TTS base class close() method to wrap manager
and WebSocket shutdown in a try/finally, ensuring _drain_emit_tasks() always
runs even when _mgr.aclose() or _close_ws() raises. Preserve the existing
exception propagation and cleanup behavior while matching the guarded shutdown
pattern used by STTBase.close().

In `@src/easycat/tts/base.py`:
- Around line 41-53: Centralize PCM16 validation in a shared
validate_pcm16_format helper in src/easycat/_audio_utils.py, preserving the
encoding, sample-width, and positive non-bool channel checks and existing error
text. In src/easycat/tts/base.py lines 41-53, remove the local
_validate_pcm16_format implementation and delegate while retaining
output_format/source_format labels; in src/easycat/noise_reduction.py lines
158-171, replace the inline guard with the shared helper call.

In `@src/easycat/tts/elevenlabs_tts.py`:
- Around line 377-381: The terminal-response guard is duplicated across
WebSocket TTS providers; centralize it in the shared _WSTTSBase helper while
preserving each provider’s terminal label. In
src/easycat/tts/elevenlabs_tts.py:377-381, replace _require_terminal_response’s
implementation with the shared helper using isFinal/error; in
src/easycat/tts/cartesia_tts.py:241-245, remove _require_terminal_response and
call the helper with done/error; in src/easycat/tts/deepgram_tts.py:301-305,
remove _require_terminal_cycle and call the helper with Flushed/Error.

In `@src/easycat/validation/_slice_runner.py`:
- Around line 122-155: Extract the shared artifact-redaction loop, including
fail-closed redaction and ValidationFailure construction, into a reusable
helper. In src/easycat/validation/_slice_runner.py lines 122-155, retain only
the slice-specific artifact table and delegate to that helper; in
src/easycat/validation/_latency_runner.py lines 101-133, replace the duplicated
loop with the same helper using the latency artifact table. Ensure both
write_redacted implementations preserve their existing stdout/stderr handling
and return failures consistently.

In `@tests/audio/test_noise_reduction.py`:
- Line 62: Remove the redundant `@pytest.mark.asyncio` decorators from the
affected asynchronous tests in test_noise_reduction.py, including the tests near
lines 62, 95, 124, 148, and 175, so they consistently rely on the repository’s
automatic asyncio mode.

In `@tests/cli/test_validate_runner.py`:
- Around line 181-209: Update
test_validation_runner_redacts_exact_runtime_secret_values to parameterize
directly from easycat.validation._environment.RUNTIME_SECRET_ENV_VARS, importing
it alongside runtime_secret_values. Remove the duplicated hard-coded
environment-variable list so newly added runtime secrets are automatically
covered.

In `@tests/debug/test_bundle_loader_boundary.py`:
- Around line 245-269: Extend
test_loader_rejects_invalid_or_non_monotonic_sequences coverage with valid
journal_degraded sentinel payloads using sequence -1, kind degraded, and name
journal_degraded. Add cases placing the sentinel before and after valid live
records as required by the loader behavior, and assert RunBundle.load accepts
them rather than raising INVALID_JOURNAL.

In `@tests/integrations/agents/test_responses_api_bridge.py`:
- Around line 600-612: Increase the asyncio.wait_for timeout around the pending
stream task in the cancellation test to a more generous value such as 5 seconds,
while preserving the existing terminal-state and cleanup assertions.

In `@tests/runtime/test_replay.py`:
- Around line 760-766: Update the test around RunBundle.load so executed_records
is meaningfully validated: either remove the unused sequence 3/5 tool records
and the vacuous assertion, or wire an executor that records executions and
invoke replay after a successful load. Preserve the INVALID_JOURNAL assertion
while ensuring the test genuinely proves no tool records execute.

In `@tests/server/test_capacity_gate_drain.py`:
- Around line 163-234: Add invalid poll_interval_s coverage to
test_drain_rejects_invalid_timeouts, using the same invalid-value cases already
exercised by test_wait_drained_rejects_invalid_poll_interval; call
CapacityGate.drain with valid drain and force timeouts, and assert ValueError
matching poll_interval_s.

In `@tests/stages/test_stages.py`:
- Around line 1253-1256: Update the assertion following the stage_complete
record lookup to parse state_after into a structured value with
ast.literal_eval, then assert that the decision field equals 1 instead of
matching dict repr text. Preserve the existing type validation and use the
parsed snapshot’s structured field.

In `@tests/telephony/test_outbound_integration.py`:
- Around line 619-621: Update the test fixture setup around manager lifecycle
state to initialize _reconciling_call_sids, _terminal_reconciliation_call_sids,
and _synthetic_failure_event_ids, preferably by reusing the shared helper from
tests/telephony/test_outbound.py. Keep the existing _pending_cleanup_call_sids,
_started, and _lifecycle_epoch initialization intact.

In `@tests/telephony/test_outbound.py`:
- Around line 427-433: Extract a module-level _make_bare_manager(**overrides) in
tests/telephony/test_outbound.py that constructs OutboundCallManager via __new__
and initializes every lifecycle field, including all reconciliation and
synthetic-failure sets, _lifecycle_epoch, _started, and _place_call_lock;
replace all four inline setups in that file with the factory while preserving
overrides. In tests/telephony/test_outbound_integration.py at lines 619-621,
import and reuse this shared factory instead of maintaining a partial field
list.

In `@tests/tts/test_ws_base.py`:
- Around line 156-176: Update
test_one_shot_provider_stop_retries_exact_failed_socket to parametrize provider
factories or constructors instead of eagerly-created CartesiaTTS, DeepgramTTS,
and ElevenLabsTTS instances. Instantiate the selected provider inside the test
body with persistent_ws=False so each invocation creates a fresh instance within
the active event loop and avoids shared _ws or emit-task state.

---

Outside diff comments:
In `@src/easycat/cli/serve.py`:
- Around line 132-146: Update the docstring of _validate_playground_config to
remove the inaccurate claim that each client connection builds
EasyConfig.browser; describe the per-connection factory as constructing a plain
EasyConfig while preserving the explanation that EasyConfig.browser() is invoked
here to pre-check credentials at startup.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 4912a88c-2228-4940-9bb7-9132a770e799

📥 Commits

Reviewing files that changed from the base of the PR and between cd1c6aa and e99841a.

📒 Files selected for processing (100)
  • src/easycat/_audio_utils.py
  • src/easycat/_numeric.py
  • src/easycat/_provider_catalog.py
  • src/easycat/_provider_helpers.py
  • src/easycat/audio_format.py
  • src/easycat/cli/diagnose/doctor.py
  • src/easycat/cli/serve.py
  • src/easycat/config/easy.py
  • src/easycat/debug/_bundle_loader.py
  • src/easycat/debug/bundle.py
  • src/easycat/debugger/_aec_routes.py
  • src/easycat/debugger/server.py
  • src/easycat/integrations/agents/_agent_runner.py
  • src/easycat/integrations/agents/base.py
  • src/easycat/integrations/agents/responses_api.py
  • src/easycat/noise_reduction.py
  • src/easycat/reconnecting_ws.py
  • src/easycat/runtime/scope.py
  • src/easycat/server/auth.py
  • src/easycat/server/config.py
  • src/easycat/server/transports.py
  • src/easycat/server/voice_server.py
  • src/easycat/server/webrtc_routes.py
  • src/easycat/session/_session.py
  • src/easycat/session/_turn_runner.py
  • src/easycat/session_manager.py
  • src/easycat/stages/tts.py
  • src/easycat/stages/turn.py
  • src/easycat/stages/vad.py
  • src/easycat/stt/base.py
  • src/easycat/stt/deepgram_provider.py
  • src/easycat/stt/elevenlabs_provider.py
  • src/easycat/stt/openai_realtime_provider.py
  • src/easycat/stt/websocket_base.py
  • src/easycat/telephony/outbound.py
  • src/easycat/timeouts.py
  • src/easycat/transports/_base.py
  • src/easycat/transports/twilio_media.py
  • src/easycat/transports/webrtc.py
  • src/easycat/transports/websocket.py
  • src/easycat/transports/webtransport.py
  • src/easycat/tts/_multi_context_ws.py
  • src/easycat/tts/_ws_base.py
  • src/easycat/tts/base.py
  • src/easycat/tts/cartesia_tts.py
  • src/easycat/tts/deepgram_tts.py
  • src/easycat/tts/elevenlabs_tts.py
  • src/easycat/turn_manager.py
  • src/easycat/validation/_environment.py
  • src/easycat/validation/_latency_runner.py
  • src/easycat/validation/_slice_runner.py
  • src/easycat/validation/redaction.py
  • tests/audio/test_audio_format.py
  • tests/audio/test_audio_utils.py
  • tests/audio/test_noise_reduction.py
  • tests/cli/test_bundles.py
  • tests/cli/test_doctor.py
  • tests/cli/test_latency_runner.py
  • tests/cli/test_serve.py
  • tests/cli/test_validate_runner.py
  • tests/config/test_debug_errors.py
  • tests/core/test_timeouts.py
  • tests/debug/test_bundle_loader_boundary.py
  • tests/debug/test_replay_and_bundle.py
  • tests/debugger/test_aec_diagnostics.py
  • tests/debugger/test_route_boundaries.py
  • tests/integrations/agents/test_agent_runner.py
  • tests/integrations/agents/test_responses_api_bridge.py
  • tests/providers/test_providers.py
  • tests/runtime/test_replay.py
  • tests/runtime/test_scope.py
  • tests/server/test_auth.py
  • tests/server/test_capacity_gate_drain.py
  • tests/server/test_voice_server_lifecycle.py
  • tests/session/test_session_lifecycle_teardown.py
  • tests/stages/test_stages.py
  • tests/stt/test_stt_base.py
  • tests/stt/test_stt_deepgram.py
  • tests/stt/test_stt_elevenlabs.py
  • tests/stt/test_stt_openai.py
  • tests/stt/test_stt_openai_realtime.py
  • tests/stt/test_websocket_base.py
  • tests/telephony/test_outbound.py
  • tests/telephony/test_outbound_integration.py
  • tests/transports/test_transport_conformance.py
  • tests/transports/test_twilio_transport.py
  • tests/transports/test_webrtc_lifecycle_server.py
  • tests/transports/test_websocket_transport.py
  • tests/transports/test_webtransport_connection_transport.py
  • tests/transports/test_webtransport_server_protocol.py
  • tests/tts/test_multi_context_ws.py
  • tests/tts/test_tts_base.py
  • tests/tts/test_tts_cartesia.py
  • tests/tts/test_tts_deepgram.py
  • tests/tts/test_tts_elevenlabs.py
  • tests/tts/test_ws_base.py
  • tests/turns/test_turn_manager.py
  • tests/validation/test_redaction_property.py
  • tests/validation/test_slice_runner.py
  • tests/websocket/test_reconnecting_ws.py
💤 Files with no reviewable changes (1)
  • src/easycat/stt/openai_realtime_provider.py

Comment thread src/easycat/_audio_utils.py Outdated
Comment thread src/easycat/debug/_bundle_loader.py Outdated
Comment thread src/easycat/debug/bundle.py
Comment thread src/easycat/debug/bundle.py Outdated
Comment thread src/easycat/integrations/agents/responses_api.py
Comment thread tests/server/test_capacity_gate_drain.py
Comment thread tests/stages/test_stages.py Outdated
Comment thread tests/telephony/test_outbound_integration.py Outdated
Comment thread tests/telephony/test_outbound.py Outdated
Comment thread tests/tts/test_ws_base.py
@yisding

yisding commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

Review follow-up is pushed in 11cdd9d5. I also addressed the CodeRabbit review-summary note outside the diff by correcting the playground cleanup docstring in src/easycat/cli/serve.py. All 33 inline threads have individual dispositions and are resolved. Local validation: 7,273 passed, 386 skipped, 39 deselected; source-wide mypy passed; changed-file Ruff format/lint and git diff --check passed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
src/easycat/reconnecting_ws.py (1)

412-435: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Do not report permanent close as complete before late connector cleanup is resolved.

A cancellation-resistant connector can return after close() finishes Line 640. Its background close then makes one attempt; if that fails, the connection stays in _pending_connection_closes, but no later connect() is possible and the successful close() caller has no indication that cleanup remains incomplete. Track unsettled connector tasks as teardown-owned work and either finish/retry their retained close before reporting success or surface an explicit incomplete-cleanup failure.

Also applies to: 624-640

🤖 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/easycat/reconnecting_ws.py` around lines 412 - 435, Update the permanent
close flow to account for connector tasks that can complete after close begins.
Track those unsettled tasks as teardown-owned work, and ensure their retained
connections are fully closed or retried before close reports success; otherwise
propagate an explicit incomplete-cleanup failure. Coordinate this with
_close_late_connection, _close_late_retained_connection, and
_pending_connection_closes so late cleanup cannot remain silently pending.
🤖 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/easycat/validation/_runner_support.py`:
- Around line 64-70: Update the artifact handling around
redact_runtime_secrets_in_file so an existing file returning False is treated as
an artifact_redaction failure and excluded from publication. Preserve the
current exception handling, but allow genuinely absent optional artifacts to
continue without failure; use the existing artifact reference and
failure-reporting flow in the surrounding runner logic.

---

Outside diff comments:
In `@src/easycat/reconnecting_ws.py`:
- Around line 412-435: Update the permanent close flow to account for connector
tasks that can complete after close begins. Track those unsettled tasks as
teardown-owned work, and ensure their retained connections are fully closed or
retried before close reports success; otherwise propagate an explicit
incomplete-cleanup failure. Coordinate this with _close_late_connection,
_close_late_retained_connection, and _pending_connection_closes so late cleanup
cannot remain silently pending.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 7365d4ea-2d6d-4901-8360-d1ffe318dcc1

📥 Commits

Reviewing files that changed from the base of the PR and between e99841a and 11cdd9d.

📒 Files selected for processing (46)
  • pyproject.toml
  • src/easycat/_audio_utils.py
  • src/easycat/cli/serve.py
  • src/easycat/debug/_bundle_loader.py
  • src/easycat/debug/bundle.py
  • src/easycat/integrations/agents/responses_api.py
  • src/easycat/noise_reduction.py
  • src/easycat/reconnecting_ws.py
  • src/easycat/server/config.py
  • src/easycat/server/transports.py
  • src/easycat/session/_session.py
  • src/easycat/stt/base.py
  • src/easycat/stt/deepgram_provider.py
  • src/easycat/stt/websocket_base.py
  • src/easycat/telephony/outbound.py
  • src/easycat/transports/_base.py
  • src/easycat/transports/twilio_media.py
  • src/easycat/transports/webrtc.py
  • src/easycat/transports/webtransport.py
  • src/easycat/tts/_multi_context_ws.py
  • src/easycat/tts/_ws_base.py
  • src/easycat/tts/base.py
  • src/easycat/tts/cartesia_tts.py
  • src/easycat/tts/deepgram_tts.py
  • src/easycat/tts/elevenlabs_tts.py
  • src/easycat/validation/_latency_runner.py
  • src/easycat/validation/_runner_support.py
  • src/easycat/validation/_slice_runner.py
  • tests/audio/test_noise_reduction.py
  • tests/cli/test_validate_runner.py
  • tests/debug/test_bundle_loader_boundary.py
  • tests/debug/test_replay_and_bundle.py
  • tests/integrations/agents/test_responses_api_bridge.py
  • tests/runtime/test_replay.py
  • tests/session/test_session_lifecycle_teardown.py
  • tests/stages/test_stages.py
  • tests/stt/test_stt_base.py
  • tests/stt/test_stt_deepgram.py
  • tests/stt/test_websocket_base.py
  • tests/telephony/test_outbound.py
  • tests/telephony/test_outbound_integration.py
  • tests/transports/test_webrtc_lifecycle_server.py
  • tests/transports/test_webtransport_connection_transport.py
  • tests/transports/test_webtransport_server_protocol.py
  • tests/tts/test_multi_context_ws.py
  • tests/tts/test_ws_base.py
💤 Files with no reviewable changes (2)
  • tests/audio/test_noise_reduction.py
  • pyproject.toml

Comment thread src/easycat/validation/_runner_support.py Outdated
@yisding
yisding merged commit 3069431 into main Jul 30, 2026
15 checks passed
@yisding
yisding deleted the agent/fix-subagent-audit-bugs branch July 30, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant