Skip to content

CloudAgentNext - budget ingest frame size and harden wrapper resume#4475

Open
eshurakov wants to merge 2 commits into
mainfrom
grateful-apparel
Open

CloudAgentNext - budget ingest frame size and harden wrapper resume#4475
eshurakov wants to merge 2 commits into
mainfrom
grateful-apparel

Conversation

@eshurakov

Copy link
Copy Markdown
Contributor

Prevents wrapper death after transport failures with layered defenses.

  • Ingest frame budgeting (src/shared/ingest-frame.ts): every wrapper→DO event is trimmed, serialized, measured, and when over 1 MiB compacted → reduced → dropped, so no frame exceeds platform/SQLite-safe limits. Terminal lifecycle events are never silently dropped.
  • Non-fatal global feed: oversized/backpressured events are dropped and logged instead of throwing and killing the feed loop.
  • Reconnect grace: liveness checks are deferred during the disconnect-grace window, and stale in-flight pings are cleared on reconnect so an already-expired deadline no longer becomes an instant wrapper_ping_timeout / pkill.
  • Observability: 1009 closes are logged, and an internal wrapper_event_truncated signal is counted as liveness activity.

Root cause: an oversized event closed the /ingest socket with 1009, the global feed threw on backpressure, and an already-expired ping deadline turned the grace window into an immediate physical wrapper stop — leaving the session unrecoverable on revisit.

…sume

A wrapper could die after a transport failure: an oversized event closed
the /ingest socket with 1009, the global feed threw on backpressure, and
an already-expired ping deadline turned the disconnect grace into an
immediate wrapper_ping_timeout physical stop. Apply a layered fix:

- Ingest frame budgeting (src/shared/ingest-frame.ts): trim, serialize,
  measure, then compact/reduce/drop so no frame exceeds 1 MiB. Terminal
  lifecycle events are never silently dropped.
- Non-fatal global feed: drop oversized/backpressured events and keep
  consuming instead of throwing.
- Reconnect grace: defer liveness checks while the current connection is
  inside its disconnect-grace window, and clear stale in-flight pings on
  reconnect so a fresh ping is scheduled.
- Observability: log 1009 closes and emit an internal wrapper_event_truncated
  signal counted as liveness activity.
Comment thread services/cloud-agent-next/src/shared/ingest-frame.ts Outdated
Comment thread services/cloud-agent-next/src/shared/ingest-frame.ts
Comment thread services/cloud-agent-next/src/session/wrapper-supervisor.ts
Comment thread services/cloud-agent-next/wrapper/src/global-feed.ts
Comment thread services/cloud-agent-next/wrapper/src/connection.ts
@kilo-code-bot

kilo-code-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The latest commit fixes all previously flagged issues (unguarded JSON.stringify, stale no-output deadline on reconnect, unrecoverable global-feed backpressure, and duplicated event-name helper) with matching test coverage, and introduces no new issues in the changed code.

Files Reviewed (8 files)
  • services/cloud-agent-next/src/session/wrapper-runtime-state.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.test.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.ts
  • services/cloud-agent-next/src/shared/ingest-frame.test.ts
  • services/cloud-agent-next/src/shared/ingest-frame.ts
  • services/cloud-agent-next/wrapper/src/connection.ts
  • services/cloud-agent-next/wrapper/src/global-feed.test.ts
  • services/cloud-agent-next/wrapper/src/global-feed.ts
Previous Review Summary (commit cb69a52)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit cb69a52)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/shared/ingest-frame.ts 316 Unguarded JSON.stringify calls in prepareIngestFrame (lines 316, 329, 342) can throw uncaught instead of degrading to the drop path, defeating the module's crash-safety goal
services/cloud-agent-next/src/session/wrapper-supervisor.ts 470 recordReconnectAccepted clears the stale ping deadline but not the analogous stale no-output deadline, so the same unrecoverable-after-reconnect bug can still occur via wrapper_no_output
services/cloud-agent-next/wrapper/src/global-feed.ts 278 Sustained backpressure now has no recovery path (throw removed with no replacement timeout/reconnect mechanism), risking silent permanent event drops

SUGGESTION

File Line Issue
services/cloud-agent-next/src/shared/ingest-frame.ts 50 safeString truncates by UTF-16 code-unit length, not byte length, risking a lone surrogate at the truncation boundary
services/cloud-agent-next/wrapper/src/connection.ts 154 kiloEventNameForLog duplicates kiloEventNameOf from ingest-frame.ts; connection.ts already imports from src/shared elsewhere
Files Reviewed (13 files)
  • services/cloud-agent-next/src/persistence/CloudAgentSession.ts
  • services/cloud-agent-next/src/session/wrapper-runtime-state.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.test.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.ts - 1 issue
  • services/cloud-agent-next/src/shared/ingest-frame.test.ts
  • services/cloud-agent-next/src/shared/ingest-frame.ts - 2 issues
  • services/cloud-agent-next/src/shared/protocol.ts
  • services/cloud-agent-next/src/websocket/ingest.ts
  • services/cloud-agent-next/src/websocket/types.ts
  • services/cloud-agent-next/test/unit/wrapper/connection.test.ts
  • services/cloud-agent-next/wrapper/src/connection.ts - 1 issue
  • services/cloud-agent-next/wrapper/src/global-feed.test.ts
  • services/cloud-agent-next/wrapper/src/global-feed.ts - 1 issue

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5 · Input: 48 · Output: 9.1K · Cached: 1.7M

Review guidance: REVIEW.md from base branch main

…ale liveness

Address PR #4475 review comments:

- Guard prepareIngestFrame JSON.stringify calls so unserializable events
  (e.g. circular refs) degrade to the compaction/drop ladder instead of
  crashing the send path; report unserializable_ingest_event on drop
- Extend the stale no-output deadline, not just the ping deadline, when a
  reconnect is accepted during grace, preventing an immediate
  wrapper_no_output before buffered output can drain
- Fail the global feed attempt after sustained backpressure so the outer
  loop reconnects on a fresh socket instead of dropping events forever
- Reuse exported kiloEventNameOf in connection.ts instead of duplicating
  the event/type extraction logic
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