feat(session): per-turn persistence for crash/interrupt-safe resume - #122
Merged
Conversation
Previously every entry point except Telegram persisted the session only after a successful RunWithMessages return, so Ctrl-C, SIGTERM, or a crash lost the entire in-progress turn even though the loop returns the partial history. A long-horizon task interrupted at minute 40 resumed from the last fully completed turn instead of the last completed step. Loop engine (internal/loop): - New SetMessagesPersistCallback, fired after each tool batch's result messages are appended and after the final assistant message. The snapshot is a freshly-allocated copy so trimContext's in-place mutation cannot corrupt handed-out state. Session store (internal/session): - New Store.SaveNoIndex: identical atomic/redacted/file-capped save that skips the vector-index update (embedding can be a remote HTTP call and must not fire every loop iteration) while refreshing UpdatedAt/Turns. The final end-of-turn Save still indexes. Entry points (cmd/odek): - run --session, continue, REPL, and serve wire the callback to SaveNoIndex; success paths keep one final Save/Append for buffer persistence and vector indexing. - Error/interrupt paths persist the partial history via persistPartialMessages, which drops a trailing assistant message with unanswered tool calls (resuming with dangling tool calls is an invalid OpenAI-compatible request). - If the loop trimmed history in place, the save is skipped so disk never loses richer persisted state. serve keeps its injected-system-message filter on persisted snapshots. An interrupted run (Ctrl-C, SIGTERM, crash) now resumes via 'odek continue' from the last completed step. Telegram already saved partials on cancel and is unchanged. Tests: TestEngine_MessagesPersistCallback (exactly two firings, tool result then final answer), TestStore_SaveNoIndex (persists without indexing; Save indexes). Verified with clean env: go build ./..., go test ./internal/loop ./internal/session ., full ./cmd/odek suite.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
odek | f15df68 | Commit Preview URL Branch Preview URL |
Jul 28 2026, 07:39 PM |
jkyberneees
added a commit
that referenced
this pull request
Jul 28, 2026
…telegram persist, LLM retry resilience, graceful budget exit, stall detection, compaction default-on (#123) * feat(agent): long-horizon readiness — tool heartbeat, shell timeout, bounded probes, telegram per-turn persist Four gaps found in a long-horizon task review (per-turn session persistence for CLI/REPL/serve landed separately in #122): 1. Tool-running heartbeat (internal/loop, internal/render, odek.go, cmd/odek/telegram.go): a single long tool call (e.g. shell running a test suite for minutes) showed a static spinner with zero feedback inside the shell tool's 30-minute buffered window. The loop now wraps each tool call with a watchdog that emits a 'tool_running' SignalEvent every 60s (toolHeartbeatInterval, test-overridable) until the call returns — closed on normal, error, and panic paths and on engine-context cancel, so no goroutine leaks. CLI renders '⏳ <tool> still running (<elapsed>)'; Telegram surfaces it even when skills.verbose is off (a silent multi-minute call looks like a hang); serve forwards it generically. 2. shell timeout_seconds (cmd/odek/shell.go): the LLM can now bound long commands explicitly (schema + Description advise it for builds/test suites). Clamped to [1, 1800]s via clampShellTimeoutSeconds, mirroring parallel_shell's cap; zero/ negative treated as absent. Output stays fully buffered — the heartbeat is the progress signal. 3. Bounded Docker availability probes: dockerAvailable() in cmd/odek/main_test.go and internal/sandbox/sandbox_test.go, plus the 'docker image inspect' probe in internal/sandbox, now use exec.CommandContext with a 5s timeout — a stopped/starting daemon can no longer hang the test suite or sandbox setup. 4. Telegram per-turn persistence (internal/telegram/session.go, cmd/odek/telegram.go): SessionManager.SaveNoIndex mirrors Save via Store.SaveNoIndex (no per-iteration embedding call) but does NOT advance the user-visible TurnCount — only a completed turn's final Save may. handleChatMessage wires SetMessagesPersistCallback with dropDanglingToolCalls (extracted from persistPartialMessages) and a skip-on-trim length check against the latest persisted state. /stop, restarts, and crashes now resume from the last completed step; SIGKILL may still lose the in-flight step (documented in docs/CLI.md). Docs: docs/CLI.md continue-row resume semantics; AGENTS.md testing guidance (scope test runs, -race on changed packages only, buffered shell + timeout_seconds) and shell.go layout line. Tests: TestToolHeartbeat_LongRunningToolEmitsSignals (fires during, stops after), shell timeout/clamp/non-positive tests, TestSessionManager_SaveNoIndex (no vector indexing, no TurnCount inflation, final Save indexes). Verified with clean env: go build ./... + go vet ./..., affected package tests, -race on internal/loop + internal/telegram, full ./cmd/odek suite (24.4s), golangci-lint 0 issues. * feat(agent): 3h+ session readiness — LLM retry resilience, graceful iteration-budget exit, stall detection, compaction default-on A. LLM client resilience (internal/llm) — the retry budget was sized for second-scale blips and killed long unattended runs on the first sustained incident: - Retryable statuses now include 408, 500, and 529 (Anthropic overloaded) alongside 429/502/503/504. - maxRetries 3 -> 7 (8 attempts), exponential backoff 1s..30s with +/-25% jitter (crypto/rand), worst-case ~114s; Retry-After honored verbatim, cap raised 60s -> 120s. - Malformed JSON bodies and zero-choice 200 responses (transient gateway artifacts) now consume retry attempts instead of aborting immediately; validated inside the retry loop via validateCompletionBody with error strings unchanged. - retrySleep package var makes backoff injectable for fast tests. B. Iteration ceiling + stall guard (internal/loop): - max_iterations exhaustion no longer returns a bare error: the engine makes one final tool-less LLM call (30s bound, compaction-style side-call) for a partial-progress summary and returns it as the final answer marked '[Iteration budget reached - partial summary]'. Summarizer failure (or a non-compliant tool-call response) falls back to the original error unchanged. - Stall detection: 3 consecutive identical SUCCESSFUL tool calls (name+args fingerprint) inject a corrective hint and emit a tool_recovery signal, closing the polling-loop blind spot where only failing tools were detected. Hint, not enforcement - never aborts. C. Compaction ON by default + serve digest persistence: - ResolvedConfig seeds compaction=true; explicit false from ~/.odek/config.json, ./odek.json, ODEK_COMPACTION=false, or the new --no-compaction flag still disables (plumbing was already *bool end-to-end). odek.Config/loop.Engine stay explicit. - Config templates: global pins "compaction": true; local omits the key so projects inherit (an explicit false would actively disable). - odek serve's persist filter now preserves compaction digest system messages ([Compacted earlier context: prefix) instead of stripping them - resumed web sessions keep their compacted history. Other injected system messages (skills/memory/episodes) still dropped. - AGENTS.md: corrected stale '300 iterations' claim (actual: 90), documented the partial-summary exit, stall detection, compaction default, and serve digest preservation. docs/CLI.md + docs/CONFIG.md updated for --no-compaction and the new default. Tests: llm retry tests for 529/500/parse/zero-choice + jitter bounds; loop tests for partial-summary success/fallback/tool-call-rejection/ persistence and stall streak + reset; config tests for default-on and explicit-false-wins; serve filter tests for digest preservation. Verified with clean env: go build ./... + go vet ./..., go test -race ./internal/llm ./internal/loop ./internal/config, root package, full ./cmd/odek suite (multiple consecutive -count=1 passes). Note: go test -count=2 ./cmd/odek fails on path-confinement tests identically on clean main (pre-existing cross-test interference, not introduced 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
1 participant
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.
Problem
Every entry point except Telegram persisted the session only after a successful
RunWithMessagesreturn. Ctrl-C, SIGTERM, or a crash lost the entire in-progress turn — even thoughrunLoopreturns the partial history, callers discarded it on the error path. A long-horizon task interrupted at iteration 80 resumed from the last fully completed turn, not the last completed step.Change
Loop engine (
internal/loop)SetMessagesPersistCallback, fired at two points: after a tool batch's result messages are appended, and after the final assistant message.trimContext's in-place mutation can't corrupt handed-out state. Nil callback = no-op.Session store (
internal/session)Store.SaveNoIndex: identical atomic / redacted / file-capped save (incl.index.jsonmetadata andUpdatedAt/Turnsrefresh) but skips the vector-index update — embedding can be a remote HTTP call and must not fire every loop iteration. The final end-of-turnSavestill indexes.Entry points (
cmd/odek)run --session,continue, REPL, andservewire the callback toSaveNoIndex; success paths keep one finalSave/Appendfor buffer persistence and vector indexing.persistPartialMessages, which first drops a trailing assistant message with unanswered tool calls (resuming with dangling tool calls is an invalid OpenAI-compatible request).servekeeps its injected-system-message filter on snapshots.Net effect: an interrupted run (Ctrl-C, SIGTERM, crash) resumes via
odek continuefrom the last completed step. SIGKILL can still lose the single in-flight step — inherent.Tests
TestEngine_MessagesPersistCallback— exactly two firings: first snapshot ends with the tool result, second with the final answer.TestStore_SaveNoIndex— persists without indexing;Saveindexes.env -u ODEK_MODEL -u ODEK_BASE_URL -u ODEK_PROMPT_CACHING -u ODEK_TELEGRAM_BOT_TOKEN):go build ./...,go test ./internal/loop ./internal/session ., full./cmd/odeksuite (22.3s) — all green;-raceon loop + session green; golangci-lint 0 issues.AGENTS.md updated with the new
Agent Loopbullet.