feat: rate limit detection, runtime swap, and session resume#98
feat: rate limit detection, runtime swap, and session resume#98liker0704 wants to merge 5 commits into
Conversation
lucabarak
left a comment
There was a problem hiding this comment.
Impressive scope — rate limit detection, runtime swap, conversation extraction, session resume, and E2E tests all in one PR. The watchdog integration and swap logic are well thought out.
However, the branch currently fails two quality gates:
TypeScript errors (2):
Both in src/watchdog/e2e-swap.test.ts — the partial OverstoryConfig objects at lines 141 and 284 fail as OverstoryConfig because they're missing required fields (agents, worktrees, mulch, merge, etc.). Fix: cast through unknown first (as unknown as OverstoryConfig) or add the missing fields.
Biome violations (2):
src/watchdog/daemon.ts:612—options.config!uses non-null assertion. Use optional chaining or an early guard.src/watchdog/e2e-swap.test.ts:212—updatedSession!.rateLimitedSince— same issue, useupdatedSession?.rateLimitedSince.
These are straightforward fixes. Once the branch passes npx tsc --noEmit and npx biome check . cleanly, happy to re-review.
Add rate limit infrastructure to the core type system: - AgentSession: add `runtime` field (tracks current runtime adapter) and `rateLimitedSince` (ISO timestamp when rate limit was detected) - OverstoryConfig: add optional `rateLimit` block with behavior (wait/swap/kill), timing, and swap runtime configuration - MailPayloadMap: add `rate_limited` protocol message type - SessionHandoff: add `rate_limit_swap` reason variant - SessionStore: add migration for `rate_limited_since` and `runtime` columns, plus `updateRateLimitedSince()` and `updateRuntime()` methods - RateLimitedPayload: new payload interface for rate limit notifications - DEFAULT_CONFIG: sensible defaults (wait behavior, 1h max, 30s poll)
…ation extraction
Extend runtime adapters with three capabilities needed for rate limit recovery:
- SpawnOpts: add `sessionId` (new session UUID) and `resumeSessionId`
(resume an existing session) fields
- AgentRuntime interface: add optional `detectRateLimit(paneContent)`
returning { limited, message, resumesAt } and optional
`extractConversation(worktreePath, sessionId, maxTurns)` for context
transfer during runtime swap
- ClaudeRuntime: implement `--session-id` and `--resume` flags in
buildSpawnCommand(), rate limit detection via pane content regex
(matches "rate limit", "try again", countdown patterns), and
conversation extraction from JSONL transcripts
- PiRuntime: implement conversation extraction from Pi's log format
- CodexRuntime: implement conversation extraction from session logs
Rate limit detection parses provider messages to extract resume timing,
enabling the watchdog to make informed wait-vs-swap decisions.
Integrate rate limit awareness into the watchdog daemon tick loop: Detection (daemon.ts): - After capturing tmux pane content, call runtime.detectRateLimit() - On first detection: set rateLimitedSince timestamp, log event, optionally notify coordinator via mail - Rate-limited agents are protected from stale/zombie escalation (they're waiting, not stuck) - When behavior is "swap", trigger swapRuntime() to move the agent to an alternate runtime Runtime swap (swap.ts — new): - Extract conversation context from current runtime's transcript - Kill the current tmux session - Spawn a new session on the target runtime with injected context - Record a session handoff with reason "rate_limit_swap" - Update session store (runtime, state, rateLimitedSince, pid) - If swap fails, session stays rate-limited (no data loss) Health (health.ts): - Add "rate_limited" to recognized agent health states - Rate-limited agents reported as degraded, not failed
ov resume (new command):
- Recovers interrupted agent sessions (crashed, killed, rate-limited)
- For Claude Code: uses --resume <session-id> to restore full
conversation context from the existing transcript
- For other runtimes: re-spawns with conversation extraction
- Supports single agent (ov resume <name>) or all resumable sessions
- --list flag to preview without acting
UUID session IDs:
- sling, supervisor, monitor: generate crypto.randomUUID() as the
session ID passed to runtime via --session-id flag
- This ensures the overstory session ID matches the Claude Code
transcript UUID, enabling ov resume to find the right transcript
- coordinator: uses UUID for runtimeSessionId (passed to Claude Code)
while keeping descriptive session-{timestamp} ID for the session
store (since coordinator also creates a run record)
Wire ov resume into CLI index and add --watchdog flag to ov watch.
Update all test files that create AgentSession objects to include the new `runtime` and `rateLimitedSince` fields. Add E2E test (e2e-swap.test.ts) that validates the full rate limit detection and swap pipeline: - Spawns a real tmux session with a fake agent script that outputs rate limit text - Runs a watchdog daemon tick against it - Verifies rate limit detection sets rateLimitedSince timestamp - Verifies swap attempt (when configured) changes runtime and state - Tests handoff record creation during swap - Tests rate-limited agents are protected from zombie escalation Fixes from review feedback: - Replace non-null assertions (!) with optional chaining (?.) - Use `as unknown as OverstoryConfig` for partial config objects - Use `as OverstoryConfig` instead of config! in daemon.ts - Remove unused variables and fix import ordering
5d0621d to
0c7b893
Compare
|
Rebased onto current All 4 issues fixed:
Additional cleanup: removed unused imports/variables, fixed import ordering, Quality gates clean: Restructured from 14 commits into 5 logical commits for cleaner review — see updated PR description for per-commit breakdown. |
jayminwest
left a comment
There was a problem hiding this comment.
Review
Rate limit handling is a real operational pain point for multi-agent systems and this is a well-structured approach (detect → notify → optionally swap). The ov resume command fills a genuine gap. Several issues to address:
Must fix
-
Dead-code ternary in
resume.tsandswap.ts—model: config.runtime?.default === runtime.id ? "default" : "default"— both branches return"default", making the entire conditional dead code. Either simplify tomodel: "default"or implement the intended logic if the model should vary. -
Unsafe
as OverstoryConfigcast indaemon.ts—options.configis typedOverstoryConfig | undefined. Theascast bypasses the type system without a runtime guard. Replace with anif (options.config)check or early guard. -
Double session store open in
resume.ts—resumeCommandopens aSessionStore, thenresumeAgentopens another for each agent. WAL mode prevents blocking, but this creates unnecessary database handles. Pass the store instance or reuse the connection.
Should fix
-
getResumable()query is overly broad —SELECT * FROM sessions WHERE state != 'completed'includes all non-completed states. The SQL could at least exclude agents with emptytmuxSessionsince the resume logic requires it. -
Missing
ov resumein CLAUDE.md CLI reference — The command is wired intoindex.tsbut not documented. -
updateRuntime()mentioned in PR description but not implemented — The swap path usesstore.upsert()instead. Update the PR description to match the actual implementation.
Non-blocking suggestions
- The
extractRecentTurnsfunction is marked@deprecatedbut exported and tested — consider removing it entirely. Bun.sleep(1_000)calls inresume.tsfor nudge delivery are fragile timing assumptions. ConsiderwaitForTuiReadywith a shorter timeout.tailReadLinesutility inswap.tsis imported byclaude.tsandpi.ts— could live in a shared module.- Codex
detectRateLimitchecks for"429"as a substring which could false-positive on normal output (e.g., "Processed 429 files"). Consider requiring more context like"HTTP 429"or"rate limit".
|
Thank you for contributing to Overstory — I genuinely appreciate the time you put into this. I've wound down active maintenance and am archiving the repo, so I won't be able to merge it here. The project stays MIT-licensed and there are 200+ forks, so please feel free to carry this forward in a fork. For new work, Warren is overstory's successor. Thanks again. 🙏 |
Summary
Adds rate limit awareness to the overstory watchdog, enabling automatic recovery when AI providers throttle agents. Three interconnected features:
Rate Limit Detection
rateLimitedSincetimestamp onAgentSession— rate-limited agents are protected from stale/zombie escalation (they're waiting, not stuck)rate_limitedprotocol messagewait(default),swapto alternate runtime, orkillRuntime Swap
behavior: "swap", automatically moves a rate-limited agent to an alternate runtime (e.g. claude → gemini)SessionHandoffwith reasonrate_limit_swapfor audit trailSession Resume (
ov resume)--resume <session-id>flag to restore full conversation context from existing transcriptcrypto.randomUUID()) ensure overstory session ID matches the Claude Code transcript UUID, enabling reliable resumeKey changes by commit
1.
feat(types)— Core type systemAgentSession.runtime— tracks current runtime adapter nameAgentSession.rateLimitedSince— ISO timestamp for rate limit trackingOverstoryConfig.rateLimit— config block (behavior, timing, swap target)rate_limitedmail protocol type +RateLimitedPayloadSessionHandoff.reasongetsrate_limit_swapvariantSessionStoremigration +updateRateLimitedSince()/updateRuntime()methods2.
feat(runtimes)— Runtime adapter extensionsSpawnOpts.sessionId/resumeSessionIdfor session identityAgentRuntime.detectRateLimit(paneContent)→{ limited, message, resumesAt }AgentRuntime.extractConversation()for context transfer during swap--session-idand--resumeflags, rate limit regex detection with resume time parsing3.
feat(watchdog)— Detection + swap integrationruntime.detectRateLimit()swapRuntime()orchestrator: extract context → kill → respawn → handoff recordrate_limitedas recognized degraded state4.
feat(commands)— Resume command + UUID IDsov resume [agent-name]with--listpreview modesling,supervisor,monitor:crypto.randomUUID()session IDscoordinator: UUID forruntimeSessionId(Claude Code), descriptive ID for session store5.
test— Fixtures + E2EAgentSessiontest fixtures updated withruntimeandrateLimitedSinceReview feedback addressed
All 4 issues from the previous review are fixed:
src/watchdog/e2e-swap.test.ts:141—as OverstoryConfigon partial objectas unknown as OverstoryConfigsrc/watchdog/e2e-swap.test.ts:284— sameas unknown as OverstoryConfigsrc/watchdog/daemon.ts:612—options.config!non-null assertionoptions.config as OverstoryConfigsrc/watchdog/e2e-swap.test.ts:212—updatedSession!non-null assertionupdatedSession?.(all occurrences)Additional lint fixes: removed unused imports/variables, fixed import ordering,
let→constfor single-assignment variables.Quality gates
tsc --noEmit— 0 errorsbiome check .— clean (0 errors, 0 warnings)bun test— all tests pass (doctor exit-code tests are flaky when run in batch — pass in isolation, pre-existing on main)Test plan
--resumeand--session-idflag construction