[myleshorton]

Summary

The first Soar dial after a pause of more than 2 minutes fails within about one round trip with soar: server lost the session. That is the usual pattern in Lantern, where Soar requests are minutes apart. This PR makes the client handshake again in that case instead of reusing a session the server has already ended.

Why it fails:

• A client with no streams open sends nothing (client.go, fillLocked: no polls when idle).
• The server ends a session that has sent nothing for its IdleTimeout (2 minutes by default; server.go, sweep).
• Client.session() still returned the old session, because nothing on the client side had closed it.
• The next stream's first query got the signed unknown-session NXDOMAIN, and the stream failed.

sequenceDiagram
    autonumber
    participant K as kindling<br/>race
    participant C as Client<br/>client.go
    participant S as Server<br/>server.go

    K->>C: DialContext, request 1
    C->>S: data queries on sid N
    S-->>C: answers, lastAuth = t0
    Note over C: no streams left, so no polls are sent
    Note over S: server.go sweep<br/>lastSeen older than IdleTimeout of 2m<br/>session N ended ⚠️
    K->>C: DialContext, request 2, minutes later
    rect rgba(255, 200, 200, 0.3)
        Note over C: client.go session()<br/>sid N not closed locally, so it is reused 🐛
        C->>S: data query on sid N
        S-->>C: signed NXDOMAIN, unknown session
        C-->>K: soar: server lost the session
    end

Loading

Seen in production. In SigNoz client traces from 10.0.3, Soar attempts failed about 0.1–0.2 s after starting with connect_failed / "server lost the session". This happened on 2026-10-14 and twice on 2026-10-15 at 10:46 and 15:03 UTC; those two were the first Soar attempts to join the race on kindling's new 8 s start delay. Soar never got a usable attempt in those requests.

Change

• New Config.SessionIdle, default 90 s.
• session() closes the current session and starts a new handshake when that session meets all three conditions:
  • It has no streams open and nothing in flight. This is the same busy test fillLocked uses, moved into clientSession.busy().
  • It has had no authenticated answer for SessionIdle.
  • It has not already been closed.
• The check and the close run under the session's lock (retireIfIdle), so another Dial can't open a stream between them.
• A Dial whose session was retired after it got the session, but before it opened its stream, handshakes again instead of failing (errSessionIdle).
• A session with streams open is never retired. It keeps polls outstanding, so lastAuth stays fresh, and the 15 s liveness check already covers it if it goes dead.
• SPEC §7.4 gains a SHOULD for clients, describing this behavior.

The fix costs one handshake round trip after a long pause, instead of a failed request. A server restart, or any other loss the client can't foresee, still ends the in-flight stream as before.

Tests

• TestIdleSessionRetiredBeforeServerForgetsIt (e2e): the server has IdleTimeout = 1 s and the client has SessionIdle = 500 ms. After a 2 s pause, the first dial must work. Without the fix it fails with exactly the production error, soar: server lost the session.
• TestDialRetiresOnlyAnIdleSession: a session that isn't idle yet is kept, a session with a stream open is kept, and an idle session is closed and replaced. Both guards were mutation-checked: removing either one fails the test.
• The Dial retry after a concurrent retirement has no deterministic test, because the window between session() returning and openStream can't be forced without a test hook. A stress test with concurrent dials hit it in only about 10–15% of bursts, so it was dropped. The check and close are atomic under cs.mu, which the stream-open case above covers.
• go test -race ./... passes. TestIdleSessionSurvivesResume also still passes: a 17 s pause stays under 90 s, so the session is kept.

The Rust client has the same bug. The matching fix is getlantern/soar-rs#4.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QQFpfCXs3JqrfyDuiGabF5

Summary by CodeRabbit

• New Features
  • Added a configurable idle period for retiring inactive sessions. When an idle session is retired, the client reconnects before opening a stream.
  • Documented guidance for clients to reconnect after extended inactivity.
• Bug Fixes
  • Avoided stream failures when the server has already forgotten an inactive session.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 97df4025-abde-46a6-a62a-995fb377688e

📥 Commits

Reviewing files that changed from the base of the PR and between ff8c86a and f9fda5f.

📒 Files selected for processing: 4

• SPEC.md
• client.go
• client_gaps_test.go
• e2e_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The client adds a configurable idle threshold and retires idle sessions that have no open streams, queued control frames, or in-flight data. DialContext retries session acquisition and stream opening when another dial retires the session as idle. Tests cover session reuse and replacement.

Changes: Idle session handling

Layer / File(s)	Summary
Idle threshold and guidance client.go, SPEC.md	Config.SessionIdle defaults to 90 seconds. Section 7.4 recommends a new handshake when an idle client approaches the server’s idle timeout.
Session retirement and reuse client.go, client_gaps_test.go	The client checks session activity under the session lock and retires an idle session only when it is not busy. Tests cover reuse of sessions that are not idle or have an open stream, and replacement of an idle session.
Stream-opening retry client.go, e2e_test.go	DialContext reacquires a session and retries when stream opening fails because another dial retired the session as idle. The end-to-end test verifies that a dial after inactivity uses a different session.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant DialContext
  participant session
  participant clientSession
  Client->>DialContext: Request a stream
  DialContext->>session: Acquire a session
  session->>clientSession: Check and retire if idle
  session-->>DialContext: Return the current session
  DialContext->>clientSession: Open a stream
  clientSession-->>DialContext: Return errSessionIdle if another dial retired the session
  DialContext->>session: Reacquire a session and retry

Loading

Merge Risk: ⚪ Minimal · up to f9fda

The idle-session change is ready to merge after normal checks; no unresolved issue was established.

Security architecture risk: 🔵 Low · up to f9fda

Session replacement preserves server authentication and protects active streams and pending delivery work. No introduced security weakness was identified. The main remaining uncertainty is concurrent and interrupted execution of the updated lifecycle.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed authority is confined to selecting and retiring a session owned by one Client instance. It does not add a destination-selection mechanism or increase the privileges of the returned tunnel connection.

Trust Boundaries and Controls

• observed — Network answers must match a pending question. A signed unknown-session response can close only its originating session; ordinary data must authenticate before refreshing that session's activity timestamp. These controls predate the PR and remain on the replacement path.
• inferred — Delayed authenticated answers remain associated with the pending query's original session. They cannot install that retired object as the current session or refresh the replacement session's identity.

Resilience and Maintainability Implications

• inferred — Retirement and stream admission are serialized by the same session lock: admission either establishes live work that blocks retirement or observes closure. Retrying only the idle-retirement sentinel preserves failure containment for unrelated closure and stream-opening errors.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: retiring idle client sessions instead of reusing sessions ended by the server.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟢 Approval recommended

The synchronized retirement and retry logic have targeted regression coverage, with no blocking issues identified.

0 open findings

What changed in this PR

Prevents failed dials after long idle periods by replacing inactive client sessions before reuse.

Changes:

• Adds configurable session retirement with a 90-second default.
• Synchronizes retirement with stream creation and retries concurrent retirement.
• Documents the behavior and adds regression coverage.

File	Description
SPEC.md	Documents idle-session reconnection guidance.
e2e_test.go	Tests successful dialing after the server’s idle timeout.
client.go	Implements idle retirement and safe dial retries.
client_gaps_test.go	Tests retirement eligibility and active-session preservation.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.