Skip to content

Revert "fix(cloud-agent): use execution heartbeat for idle cleanup instead of kiloServerLastActivity"#3186

Merged
eshurakov merged 1 commit into
mainfrom
revert-3176-fix/cloud-agent-idle-cleanup
May 11, 2026
Merged

Revert "fix(cloud-agent): use execution heartbeat for idle cleanup instead of kiloServerLastActivity"#3186
eshurakov merged 1 commit into
mainfrom
revert-3176-fix/cloud-agent-idle-cleanup

Conversation

@eshurakov
Copy link
Copy Markdown
Contributor

Reverts #3176

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented May 11, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This PR reverts #3176 (execution-heartbeat-based idle cleanup) and re-introduces the kiloServerLastActivity timestamp approach, but with improved correctness over the original implementation.

Key design decisions look sound:

  • kiloServerLastActivity is set at session prepare time (both async path via runPreparationAsync:1165 and sync path via session-prepare.ts:705) and at each execution start (orchestrator.ts:172-182), ensuring the clock is always fresh when a server is genuinely in use.
  • cleanupIdleKiloServer correctly guards against stopping a server mid-run by checking activeExecutionId after the idle threshold check, preventing needless DO queries when the server is still fresh.
  • Clearing kiloServerLastActivity to undefined after a successful stop (CloudAgentSession.ts:1790) prevents the alarm from repeatedly attempting to stop an already-stopped server.
  • Error handling for recordKiloServerActivity is non-fatal in all callers, which is the right tradeoff — a missed timestamp write shouldn't block an execution.

One minor observation (not blocking): When cleanupIdleKiloServer skips cleanup because activeExecutionId !== null (line 1743), it returns without updating lastActivity. The next alarm will see a stale (older) timestamp. In practice this is benign because recordKiloServerActivity is called at the start of each execution in the orchestrator, resetting the clock. But if an execution somehow fails to call that (e.g. the orchestrator's withDORetry silently swallows the error), a long-running execution followed by a quiet alarm could trigger an immediate shutdown on the next check. This is already mitigated by the non-fatal error handling and the active-execution guard, so it's not a real risk.

Files Reviewed (5 files)
  • services/cloud-agent-next/src/execution/orchestrator.ts
  • services/cloud-agent-next/src/persistence/CloudAgentSession.ts
  • services/cloud-agent-next/src/persistence/schemas.ts
  • services/cloud-agent-next/src/persistence/types.ts
  • services/cloud-agent-next/src/router/handlers/session-prepare.ts

Fix these issues in Kilo Cloud


Reviewed by claude-4.6-sonnet-20260217 · 835,402 tokens

@eshurakov eshurakov merged commit 2b16ea5 into main May 11, 2026
13 checks passed
@eshurakov eshurakov deleted the revert-3176-fix/cloud-agent-idle-cleanup branch May 11, 2026 19:52
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.

2 participants