fix: close SessionDB FDs on timeout and lazy recall paths (#72782) - #72804
fix: close SessionDB FDs on timeout and lazy recall paths (#72782)#72804webtecnica wants to merge 4 commits into
Conversation
…tup (NousResearch#72763) Two related bugs in hermes photon setup / gateway_setup: Bug 1 -- stale token reused (401) ---------------------------------- _cmd_setup reused an existing dashboard token without validation. The device token has a short TTL (~3-4 days observed); reusing a stale token caused every management API call (find_project_by_name, regenerate_project_secret, etc.) to fail with 401. The operator saw confusing "spectrum provisioning failed: 401" errors. Fix: check GET /api/auth/get-session before using the stored token. On 401/403, clear the stale token with clear_photon_token() and fall back to a fresh device-login flow automatically. Bug 2 -- channel left disabled after successful setup ----------------------------------------------------- After all five provisioning steps completed, config.yaml still had photon.enabled: false, so the gateway never loaded the Photon adapter. Every inbound iMessage hit Photon's offline auto-responder without the operator being notified. Fix: call write_platform_config_field('photon', 'enabled', True, raw=True) as a final setup step so the gateway picks up the freshly configured channel on its next start. New public API in auth.py: - clear_photon_token() -- discard stored token from auth.json - check_photon_token_valid(token) -- lightweight session-check test References: NousResearch#72763
|
Thanks for identifying the two SessionDB ownership gaps. The production direction is sound: current Problems
Suggested changes
Automated hermes-sweeper review. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two open PRs address #72782. Both close late SessionDB results after cron initialization timeouts and track lazy-versus-injected database ownership, but #72822 isolates those fixes and adds targeted regression coverage while #72804 includes unrelated changes.
Related pull requests
- #72804
fixes— (+210/-158) — fixes, but duplicated and unfocused: The SessionDB future callback and ownership-aware teardown address both reported leak causes, but the diff also changes model aliases, Photon setup/authentication, and Himalaya documentation and adds no regression tests for #72782. Despite the MAINTAINER-BOT keep_open verdict on #72804, its suggested salvage is the focused production commit 20fb735, while #72822 already contains the equivalent production fix with dedicated tests. - #72822
best fix— (+211/-1) — focused fix with a concrete salvage path: It addresses both leak paths in agent/agent_init.py, cron/scheduler.py, and run_agent.py, and adds tests for late-result closure, constructor failure, lazy-versus-injected ownership, and idempotent teardown.
Duplicates
#72804 and #72822 implement essentially the same production changes for both SessionDB ownership gaps; #72822 is the focused, test-backed duplicate target, while #72804 carries unrelated changes.
Suggested consolidation
Keep #72822 open with a salvage path: retain its focused three-file production fix and targeted regression tests for both causes in #72782. Close #72804 as a duplicate of #72822 despite its MAINTAINER-BOT keep_open verdict, because the complete diff is unfocused and untested for this issue, while the verdict's salvageable production commit 20fb735 is represented by the equivalent tested implementation in #72822; preserve attribution as appropriate.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I72782(["issue #72782 (open)"])
subgraph Dup72804 ["PRs duplicating each other"]
P72804["PR #72804 (open)"]
P72822["PR #72822 (open)"]
end
P72804 -->|fixes| I72782
class I72782 open
class P72804 open
class P72822 open
class P72822 best
class P72804 target
click I72782 "https://github.com/NousResearch/hermes-agent/issues/72782"
click P72804 "https://github.com/NousResearch/hermes-agent/pull/72804"
click P72822 "https://github.com/NousResearch/hermes-agent/pull/72822"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 36 kB of PR diffs, 7 kB of issue/PR text, 5 kB of discussion (1 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
Fixes #72782: Two ownership gaps in SessionDB lifecycle were leaking SQLite file descriptors (.db, WAL, SHM) on cron-heavy gateways.
Changes
1.
cron/scheduler.py— Timeout path FD leakrun_job()constructsSessionDBin a one-worker executor withfuture.result(timeout=N). When initialization exceeds the timeout, the worker was abandoned and its eventual successful result was never retrieved or closed — the returnedSessionDB's FDs leaked for the process lifetime.Fix: Keep the future reference. On
TimeoutError, register a done callback (_close_late_session_db_fds) that retrieves and closes the result when the constructor eventually completes.2.
agent/agent_init.py+run_agent.py— Lazy recall ownershipWhen
AIAgentreceivessession_db=None,_get_session_db_for_recall()opens its ownSessionDBbutAIAgent.close()never closed it. Caller-injected gateway/CLI DBs must remain shared.Fix:
agent_init.py: Initialize_owns_session_db = Falsefor injected stores.run_agent.py: Set_owns_session_db = Truewhen lazily creating the fallback.run_agent.py: Inclose(), step 8, close_session_dbonly when the agent owns it.Testing
All existing cron tests pass (verified against a clean base):
test_sessiondb_init_hang.py(4 tests)test_scheduler_mcp_init.pytest_cron_inactivity_timeout.py(11 tests)