Skip to content

feat: add configurable activity-aware ACP request timeout#1429

Open
brittianwarner wants to merge 1 commit intomainfrom
feat/activity-aware-acp-timeout
Open

feat: add configurable activity-aware ACP request timeout#1429
brittianwarner wants to merge 1 commit intomainfrom
feat/activity-aware-acp-timeout

Conversation

@brittianwarner
Copy link
Copy Markdown

Summary

  • The ACP client timeout is now activity-aware: the timer resets whenever any inbound JSON-RPC message arrives from the agent process, making it an inactivity threshold rather than a wall-clock cap
  • Default timeout bumped from 120s to 15 minutes (900,000ms) to accommodate long-running inference
  • New public acpTimeoutMs option on both AgentOsOptions (VM-wide default) and CreateSessionOptions (per-session override)
  • Input validation rejects acpTimeoutMs <= 0 with a clear error message

Problem

Agents actively streaming session/update notifications during long prompts would hit the fixed 120s timeout and get killed despite being healthy and making progress. Server logs confirmed: Pi process alive (exitCode=null), actively streaming (session/update x20), times out at exactly 120s.

Solution

_resetPendingTimers() clears and recreates all pending request timers whenever any valid JSON-RPC message (response, notification, or inbound request) arrives in _startReading(). This makes the timeout measure silence, not total elapsed time:

  • Active agent: timer resets constantly, never hits the cap
  • Waiting-for-inference agent: 15min silence threshold is generous
  • Dead/hung agent: detected within the timeout window after last activity

Changes

packages/core/src/acp-client.ts

  • DEFAULT_TIMEOUT_MS: 120,000 → 900,000 (15 min)
  • PendingRequest interface: added method field for timeout error messages
  • _pending map key narrowed from number | string | null to number (only auto-increment IDs)
  • New _resetPendingTimers(): _closed guard, clears + recreates timers, JSDoc
  • Response lookup uses type-safe narrowing for numeric msg.id
  • Constructor validates timeoutMs > 0

packages/core/src/agent-os.ts

  • AgentOsOptions.acpTimeoutMs?: number — VM-wide default
  • CreateSessionOptions.acpTimeoutMs?: number — per-session override
  • _acpTimeoutMs private field, set in create(), threaded to AcpClient in createSession()

packages/core/tests/acp-protocol.test.ts

  • "activity-aware timeout resets on inbound notifications" — 1200ms of 300ms-interval notifications succeeds under 2s timeout (1700ms CI margin)
  • "activity-aware timeout still fires after activity then silence" — notification at T+400ms, timeout at T+900ms, wall-clock assertion elapsed >= 750ms proves the timer was actually reset

packages/core/README.md

  • Updated stale AgentOsOptions and CreateSessionOptions field lists

Verified

  • Type-check: clean
  • All 4 timeout/activity tests pass (existing 3 + 2 new)
  • No new lint errors
  • Timer reference safety: no double-fire, no leaks, no races (single-threaded JS)
  • _rejectAll() on close() clears timers created by _resetPendingTimers

Follow-ups

  • Mirror acpTimeoutMs in Rivet actor layer (actor parity requirement)

Copy link
Copy Markdown
Author

brittianwarner commented Apr 4, 2026

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