fix(gastown): fix town deletion DO/container leaks and terminal stability#1196
Merged
fix(gastown): fix town deletion DO/container leaks and terminal stability#1196
Conversation
Contributor
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (57 files)
Reviewed by gpt-5.4-20260305 · 603,343 tokens |
2459643 to
58bc05b
Compare
58bc05b to
f642a18
Compare
marius-kilocode
approved these changes
Mar 18, 2026
f642a18 to
d525289
Compare
…1182) Town deletion via tRPC only removed rows from GastownUserDO, leaving TownDO, TownContainerDO, and AgentDOs running indefinitely. Six compounding root causes: 1. tRPC deleteTown now calls TownDO.destroy() before removing user rows 2. TownDO.destroy() now destroys TownContainerDO (kills container) 3. Alarm has an exit condition: skips re-arming when town:id is missing 4. armAlarmIfNeeded() checks for town:id to prevent alarm resurrection 5. Compatibility date updated to 2026-02-24 so deleteAll() clears alarms 6. Integration tests for the full deletion flow Closes #1182
… control frame filter (#1195) Three compounding bugs caused terminal instability: 1. PTY WebSockets had no reconnection logic — container sleep/restart permanently froze the terminal. Now reconnects with exponential backoff (1s→2s→4s→8s, 8 attempts) and re-creates PTY sessions. 2. Undebounced ResizeObserver fired resize storms during CSS transitions, causing PTY/xterm dimension mismatches (blacked-out cells). Now debounced at 150ms. 3. SDK cursor metadata (0x00 prefix + JSON) written as raw text to xterm. Now filtered on both binary and string WebSocket frames. Additionally: - Deduplicated terminal setup: MayorTerminalPane and AgentTerminalPane now use the shared useXtermPty hook instead of duplicating ~150 lines of xterm/WebSocket/resize logic each. - Added visual connection status indicator (green=connected, yellow=reconnecting, gray=disconnected) to all terminal panes. Closes #1195
d525289 to
8621796
Compare
| JSON.parse(data); | ||
| // Valid JSON on the PTY WebSocket = SDK control message. | ||
| // Actual PTY output that starts with '{' would be part | ||
| // of a larger escape sequence and wouldn't parse as JSON. |
Contributor
There was a problem hiding this comment.
WARNING: Valid JSON can be legitimate terminal output
Programs often emit JSON lines verbatim (for example {"status":"ok"} from CLI tools). This branch now parses any text frame that starts with { and drops it, so those commands will silently lose output in the terminal. The filter needs a stronger sentinel than "parses as JSON" before returning here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two Gastown bugs from Phase 3:
Bug: Deleting a town doesn't clean up TownDO, TownContainerDO, or AgentDOs — resources leak indefinitely #1182 — Town deletion leaks DOs and containers: The tRPC
deleteTownmutation only removed rows fromGastownUserDO, leavingTownDO,TownContainerDO, andAgentDOinstances running indefinitely (burning Cloudflare resources). Fixed by: (1) callingTownDO.destroy()from the tRPC path, (2) destroyingTownContainerDOinTownDO.destroy(), (3) adding alarm exit conditions to prevent resurrection after destroy, (4) bumping compat date to2026-02-24sodeleteAll()clears alarms.Fix terminal stability — WebSocket reconnection, resize debounce, control frame filtering #1195 — Terminal stability: Three compounding issues degraded the xterm.js terminal: (1) PTY WebSockets had no reconnection logic—container sleep/restart permanently froze the terminal, (2) undebounced
ResizeObservercaused resize storms during CSS transitions (blacked-out cells), (3) SDK cursor metadata (0x00prefix + JSON) was written as visible text to the terminal. All three are fixed, and the duplicated terminal setup code (~150 lines each inMayorTerminalPaneandAgentTerminalPane) is consolidated into the shareduseXtermPtyhook.Closes #1182
Closes #1195
Verification
pnpm typecheck— passes across all packages (gastown + root)pnpm test:integration(gastown) — new town-deletion tests written; pre-existing failures inhttp-api.test.tsandconvoy-dag.test.tsunchanged (22 failures existed before this PR on main)pnpm test(gastown unit) — passestowns.handler.ts) also benefits fromTownDO.destroy()now destroying the containerVisual Changes
N/A
Reviewer Notes
cloud-agent-nextpackage has 51 pre-existing lint errors that blockgit push— used--no-verifyas approved by the user.rig-do.test.tsthat prevents test files alphabetically after it from running. The newtown-deletion.test.tstests are structurally sound and follow the exact patterns fromrig-alarm.test.ts.2026-01-27→2026-02-24) enablesdeleteAll()to also clear alarms. The installed Miniflare runtime only supports up to2026-01-28, so tests fall back gracefully.useXtermPtydeduplication removes ~300 lines of duplicated code fromTerminalBar.tsxwhile adding reconnection, resize debounce, and control frame filtering in one place.