Skip to content

fix(session): time out daemon CLI calls#409

Merged
benvinegar merged 2 commits into
mainfrom
fix/session-command-timeouts
Jun 10, 2026
Merged

fix(session): time out daemon CLI calls#409
benvinegar merged 2 commits into
mainfrom
fix/session-command-timeouts

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • Add a shared timeout wrapper for session-daemon HTTP calls.
  • Route session capabilities and every hunk session * API request through that wrapper.
  • Cover hung capability/API calls, including operations that ignore abort signals or hang while parsing the response.

Fixes #406.

Testing

  • bun run lint
  • bun run typecheck
  • bun run format:check
  • bun test ./src ./packages ./scripts ./test/cli ./test/session

This PR description was generated by Pi using OpenAI GPT-5

@benvinegar benvinegar force-pushed the fix/session-command-timeouts branch from 218841b to 95ab368 Compare June 9, 2026 20:21
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a shared timeout wrapper (withSessionDaemonHttpTimeout) and a convenience HTTP helper (requestSessionDaemonHttp) so that all hunk session * daemon calls — including capabilities negotiation — fail fast after 5 seconds instead of hanging indefinitely when the daemon is unresponsive.

  • New src/session/daemonHttp.ts: introduces withSessionDaemonHttpTimeout (races a task against a setTimeout-backed abort) and requestSessionDaemonHttp (wraps fetch + response parsing inside that timeout), with the timeout covering the full lifecycle, not just the initial connect.
  • capabilities.ts and cli.ts refactored: both route their fetch calls through requestSessionDaemonHttp, and readHunkSessionDaemonCapabilities now accepts an optional timeoutMs parameter forwarded from the CLI client constructor.
  • Test coverage: new tests in all three test files verify both the "stub-born task that ignores abort" path in withSessionDaemonHttpTimeout and the "parse hangs after fetch returns" path in requestSessionDaemonHttp; globalThis.fetch is properly saved and restored via afterEach in both cli.test.ts and capabilities.test.ts.

Confidence Score: 5/5

Safe to merge — the change is well-scoped and all existing null-return paths in capabilities.ts are preserved inside the parse callback.

The timeout logic is correctly layered: the AbortController fires first, both the fetch and the Promise.race reject with the same timeoutError, and the finally block suppresses any late rejection from tasks that ignore the abort signal. The parse callback keeps the timeout clock running through body consumption, which is the subtle edge case the PR set out to fix. Test coverage exercises all three layers — stub-born tasks, hung fetches, and hung parses — and globalThis.fetch is properly restored in afterEach in every affected test file.

No files require special attention.

Important Files Changed

Filename Overview
src/session/daemonHttp.ts New file introducing withSessionDaemonHttpTimeout and requestSessionDaemonHttp; implementation correctly covers the full fetch + parse lifecycle, handles tasks that ignore abort signals via Promise.race, and suppresses late rejections from aborted tasks in the finally block.
src/session/capabilities.ts Refactored to route the capabilities fetch through requestSessionDaemonHttp; all existing null-return paths preserved inside the parse callback; adds optional timeoutMs parameter.
src/hunk-session/cli.ts HttpHunkSessionCliClient gains a timeoutMs constructor param (defaulting to HUNK_SESSION_DAEMON_HTTP_TIMEOUT_MS); request() and getCapabilities() both route through the new timeout wrappers; factory function updated to accept optional config object.
src/session/daemonHttp.test.ts New test file covering both the abort-ignoring task path and the hung-parse-after-fetch path; globalThis.fetch is saved and restored correctly within each test.
src/session/capabilities.test.ts Adds a timeout test for readHunkSessionDaemonCapabilities with a mock fetch; correctly saves/restores globalThis.fetch in afterEach alongside the existing real-server cleanup.
src/hunk-session/cli.test.ts Adds AbortSignal assertion to the existing happy-path test and a new timeout test for listSessions; globalThis.fetch cleanup moved to a top-level afterEach, properly covering all test cases in the file.
CHANGELOG.md Adds a user-facing entry for the daemon timeout fix under the current unreleased section.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant withSessionDaemonHttpTimeout
    participant task/fetch
    participant setTimeout

    Caller->>withSessionDaemonHttpTimeout: call(operation, timeoutMs, task)
    withSessionDaemonHttpTimeout->>setTimeout: schedule abort after timeoutMs
    withSessionDaemonHttpTimeout->>task/fetch: start task(signal)
    Note over withSessionDaemonHttpTimeout: Promise.race([taskPromise, timeoutPromise])

    alt Task completes before timeout
        task/fetch-->>withSessionDaemonHttpTimeout: resolve(result)
        withSessionDaemonHttpTimeout->>setTimeout: clearTimeout()
        withSessionDaemonHttpTimeout-->>Caller: return result
    else Timeout fires first
        setTimeout->>withSessionDaemonHttpTimeout: controller.abort(timeoutError)
        setTimeout->>withSessionDaemonHttpTimeout: reject(timeoutError)
        withSessionDaemonHttpTimeout->>task/fetch: abort signal fires
        Note over task/fetch: task may reject later — suppressed via .catch()
        withSessionDaemonHttpTimeout-->>Caller: throw HunkUserError("Timed out…")
    end
Loading

Reviews (1): Last reviewed commit: "fix(session): time out daemon CLI calls" | Re-trigger Greptile

@benvinegar benvinegar force-pushed the fix/session-command-timeouts branch from 95ab368 to 15ac7dd Compare June 9, 2026 22:07
@benvinegar benvinegar enabled auto-merge (squash) June 9, 2026 22:16
@benvinegar benvinegar force-pushed the fix/session-command-timeouts branch from 15ac7dd to 46281f0 Compare June 9, 2026 22:23
@benvinegar benvinegar merged commit ecdc1c4 into main Jun 10, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hunk session * hangs indefinitely when the daemon isn't running (Windows)

1 participant