Add configurable session idle timeout option#1093
Add configurable session idle timeout option#1093MackinnonBuck wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Node SDK client option to configure a server-wide session idle timeout and wires it through to the spawned Copilot CLI runtime via a new --session-idle-timeout argument, with accompanying documentation updates.
Changes:
- Adds
sessionIdleTimeoutMs?: numbertoCopilotClientOptionswith documented defaults/minimums. - Sets a default of
0(disabled) and forwards--session-idle-timeout <ms>when configured to a positive value. - Updates session persistence docs to describe the new default behavior and configuration caveats (only applies when the SDK spawns the runtime).
Show a summary per file
| File | Description |
|---|---|
| nodejs/src/types.ts | Introduces the new CopilotClientOptions.sessionIdleTimeoutMs option and documents intended semantics. |
| nodejs/src/client.ts | Applies defaulting and forwards the new option to the CLI as --session-idle-timeout. |
| docs/features/session-persistence.md | Documents the new option, updated defaults, and cliUrl caveat. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
| // Default useLoggedInUser to false when githubToken is provided, otherwise true | ||
| useLoggedInUser: options.useLoggedInUser ?? (options.githubToken ? false : true), | ||
| telemetry: options.telemetry, | ||
| sessionIdleTimeoutMs: options.sessionIdleTimeoutMs ?? 0, | ||
| }; |
There was a problem hiding this comment.
sessionIdleTimeoutMs is documented as having a minimum of 300000ms, but the client currently forwards any positive value to the runtime without validation. This can lead to unexpected runtime failures or behavior when users pass values < 300000, negative numbers, non-integers, or NaN. Consider validating in the constructor (e.g., finite number, integer, >= 0, and if > 0 then >= 300000) and throwing a clear error when invalid.
| if (this.options.sessionIdleTimeoutMs !== undefined && this.options.sessionIdleTimeoutMs > 0) { | ||
| args.push("--session-idle-timeout", this.options.sessionIdleTimeoutMs.toString()); | ||
| } |
There was a problem hiding this comment.
No automated test currently asserts that sessionIdleTimeoutMs results in the expected --session-idle-timeout <ms> CLI argument (and that 0/unset does not). Adding a unit test that mocks spawn and inspects the argv would prevent regressions in the option-to-flag mapping.
Add a new optional sessionIdleTimeoutMs field to CopilotClientOptions that allows consumers to configure the server-wide session idle timeout. When set to a positive value, the SDK passes --session-idle-timeout to the CLI process. Sessions have no idle timeout by default (infinite lifetime). The minimum configurable value is 300000ms (5 minutes). Also updates the session persistence documentation to reflect the new default behavior and configuration option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MackinnonBuck
force-pushed
the
mackinnonbuck/add-session-idle-timeout-option
branch
from
f3008df to
ede32da
Compare
Summary
Adds SDK-side support for configurable session idle timeout, addressing github/copilot-sdk-partners#28.
The corresponding runtime changes that add the
--session-idle-timeoutCLI arg are in github/copilot-agent-runtime#6414.Changes
nodejs/src/types.tssessionIdleTimeoutMs?: numbertoCopilotClientOptionswith JSDoc documenting default (0/disabled), minimum (300,000ms / 5 minutes), and behavior.nodejs/src/client.tssessionIdleTimeoutMsto the defaults object (defaulting to0).--session-idle-timeout <ms>to the CLI process on startup.docs/features/session-persistence.mdsessionIdleTimeoutMsoption with usage example.cliUrl).Notes
CopilotClientOptionsfield (server/process level), not a per-session config.