fix: error context preservation, WAL checkpoint, hook timeout

[aaronlab]

Summary

This PR addresses three reliability gaps found during iteration #4 of a deep code audit:

• Preserve original error context in call_llm retry chain (agent/auxiliary_client.py): When the max_tokens retry also fails with a payment error, the original error was silently overwritten via first_err = retry_err (line 2089), losing the initial diagnostic context. Now uses retry_err.__cause__ = first_err for proper Python exception chaining, preserving the full error sequence for debugging.

• Guard payment fallback provider call (agent/auxiliary_client.py): The fallback API call (line 2105) had no try/except. If the fallback provider also failed, there was zero logging and no indication that a fallback was even attempted. Added error handling with a warning log message identifying which fallback provider failed.

• Add WAL checkpoint to holographic memory store (plugins/memory/holographic/store.py): WAL mode was enabled (line 130) but no checkpoint mechanism existed anywhere in the class, unlike hermes_state.py which has proper _try_wal_checkpoint(). This causes unbounded WAL file growth over long-running sessions. Added periodic PASSIVE checkpoint every 50 writes and a final checkpoint on close(), following the established pattern.

• Add timeout protection to plugin hook invocation (hermes_cli/plugins.py): Hook callbacks had good exception isolation (try/except per callback) but no timeout protection. A misbehaving plugin could block the agent loop indefinitely with a blocking call. Added a 30-second timeout using ThreadPoolExecutor, with warning logging on timeout.

Files Changed

File	Change
agent/auxiliary_client.py	Exception chaining for error context + fallback error handling
plugins/memory/holographic/store.py	WAL checkpoint method + periodic + on-close checkpoint
hermes_cli/plugins.py	30s timeout for hook callbacks via ThreadPoolExecutor

Test plan

• Simulate primary provider max_tokens error → payment retry → verify original error in __cause__
• Simulate fallback provider failure → verify warning log is emitted
• Insert 100+ facts into holographic store → verify WAL file stays bounded
• Create a plugin with time.sleep(60) in pre_llm_call hook → verify 30s timeout warning
• Run existing test suite: pytest tests/

🤖 Generated with Claude Code

[alt-glitch]

Related to #16510 (WAL checkpoint pattern) and #6684 (write_count race in hermes_state.py). WAL checkpoint portion overlaps with existing work.

[alt-glitch]

Related to #16510 (WAL checkpoint pattern) and #6684 (write_count race in hermes_state.py). WAL checkpoint portion overlaps with existing work.

[teknium1]

Thanks for the reliability audit and for identifying the hook and holographic-store failure modes. The current patch needs substantial rework before it can be safely salvaged.

Problems

• The hook timeout does not actually bound execution: with ThreadPoolExecutor(...) waits during executor shutdown after future.result(timeout=...) times out, so the blocked callback still blocks the caller. The current dispatch remains synchronous at hermes_cli/plugins.py:1913-1925.
• wal_checkpoint(PASSIVE) does not shrink the WAL high-water mark. Main commit 46b2afc56b79 changed SessionDB from PASSIVE to TRUNCATE specifically for this reason. The proposed counter also covers only add_fact, while update_fact, remove_fact, and record_feedback commit independently at plugins/memory/holographic/store.py:286-392.
• agent/auxiliary_client.py has been extensively refactored since this branch. Current sync and async max-token retry paths are at :6651-6656 and :7202-7207, and fallback calls now use _call_fallback_candidate_sync at :6930-6952; the direct fallback-call hunk no longer applies.

Suggested changes

• Rework the timeout so its timeout path cannot wait for the callback worker, and add a blocked-hook regression test.
• Port the WAL work to the current SessionDB TRUNCATE pattern and cover every public write path.
• Rework and test exception chaining across both sync and async auxiliary fallback paths.

Automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Four open PRs reference this audit-derived reliability complex, but they address distinct causes: #6622 covers error chaining, fallback logging, hook timeouts, and WAL checkpointing; #6627 covers concurrent-tool timeouts, file handling, and RPC decoding; #6629 covers token estimation, context-length diagnostics, and batch durability; #6635 covers Anthropic tool-prefix handling and parse/shutdown observability. Each diff contains superseded, ineffective, incorrect, or incomplete changes, so none currently provides a mergeable consolidation target.

Related pull requests

• #6622 related — (+670/-4) — needs substantial rework: The error-context and fallback diagnostics target code that has since been refactored, the ThreadPoolExecutor context still waits for a timed-out hook, and PASSIVE checkpointing neither truncates the WAL nor covers all write paths. The contributor keep_open review supports salvaging the hook and holographic-store concerns only after rebasing, using genuinely non-blocking timeout dispatch, applying an appropriate checkpoint policy across every committed mutation, and adding tests.
• #6627 related — (+639/-4) — narrow salvage only: The concurrent-tool timeout and cwd handle fixes are already present on main, while replacement decoding can silently corrupt structured JSON and still fails for malformed syntax. Consistent with the keep_open review, retain strict UTF-8 and salvage only explicit RPC protocol-error handling with a malformed-response regression test.
• #6629 related — (+647/-3) — needs focused rework: The token-estimation correction has already landed, but the fallback diagnostic and fsync durability concerns remain relevant; the proposed Pool.join(timeout=10) calls are invalid and the configuration guidance is inaccurate. Consistent with the keep_open review, remove the landed and unrelated material, use valid worker cleanup, and test fallback logging, durable trajectory writes, and interruption paths before merge.
• #6635 related — (+635/-1) — needs focused rework: OAuth prefix stripping already exists on main, the gateway hunk inspects a write-only map rather than the adapters' actual queued follow-ups, and malformed argument logging covers only one of two fallback paths. Consistent with the keep_open review, remove the landed prefix hunk, instrument both parse fallbacks and the real pending-message stores, and add focused regression tests.

Duplicates

#6622, #6627, #6629, and #6635 duplicate the same unrelated AUDIT_ITERATION_2.md and AUDIT_ITERATION_2_SUMMARY.txt additions; their substantive code changes are not duplicates and should not be consolidated into one PR.

Suggested consolidation

Merge none as-is. Keep #6622, #6627, #6629, and #6635 open only for the focused salvage described in their contributor keep_open reviews, remove the duplicated audit documents and already-landed hunks, and split the remaining unrelated fixes into independently tested PRs; no substantive PR can currently be closed as a duplicate of another.

Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 110 kB of PR diffs, 8 kB of issue/PR text, 6 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.