Skip to content

feat(server): make provider session reaper timing configurable - #5525

Open
kraptor23 wants to merge 2 commits into
pingdotgg:mainfrom
kraptor23:feat/configurable-session-reaper
Open

feat(server): make provider session reaper timing configurable#5525
kraptor23 wants to merge 2 commits into
pingdotgg:mainfrom
kraptor23:feat/configurable-session-reaper

Conversation

@kraptor23

@kraptor23 kraptor23 commented Aug 6, 2026

Copy link
Copy Markdown

What

Adds providerSessionInactivityThreshold and providerSessionSweepInterval server settings (persisted as millis, defaults unchanged: 30 min / 5 min) consumed by ProviderSessionReaper.

Implements #5523. Related: #4198.

Why

Both reaper values are hardcoded; ProviderSessionReaperLiveOptions exists but the shipped layer is built with no arguments, and nothing user-facing feeds it. Users with long-running threads currently have no supported way to keep sessions warm longer than 30 minutes.

How

  • Schema in packages/contracts/src/settings.ts (flat fields with Schema.DurationFromMillis + decoding defaults, patch keys in ServerSettingsPatch), placed outside the legacy-marked block. Whether these should instead live in the backgroundActivity profile system is raised in Provider session reaper inactivity threshold and sweep interval are not configurable #5523 — happy to rework.
  • Duration fields get the same deepMerge/strip special-casing as automaticGitFetchInterval (ATOMIC_SETTINGS_KEYS, makeTest, applyServerSettingsPatch).
  • The reaper resolves the threshold from settings on every sweep (settings edits apply live); the sweep interval is resolved once at start() and documented as restart-required. A failed settings read fails the sweep — logged by the existing sweep-failed handler and retried next sweep — instead of silently reaping with a default the user overrode; the interval read falls back to the default with a settings-fallback warning so startup never blocks.
  • Explicit ProviderSessionReaperLiveOptions keep precedence over settings, so existing test seams are unchanged. ServerSettingsLayerLive already sits above ProviderRuntimeLayerLive in server.ts, so no composition changes.

Tests

  • Reaper reads timing from settings when constructed without options: a session idle ~5 s is reaped under a 1 s settings threshold (would time out under the hardcoded 30 min default) and survives under a 30-day threshold.
  • Explicit options beat settings with the idle time placed between the two candidate thresholds, so an inverted precedence fails the test.
  • vp test run on the touched test files (26 tests) and tsgo --noEmit for contracts/shared/server pass.

Positioning notes

Written by Claude Fable 5 via Claude Code.


Note

Medium Risk
Changes when idle provider sessions are stopped (user-visible session lifetime) and adds asymmetric failure handling for settings reads during sweeps vs startup.

Overview
Provider session reaper timing is no longer hardcoded in ProviderSessionReaper. New persisted server settings providerSessionInactivityThreshold and providerSessionSweepInterval (defaults 30 min / 5 min, unchanged behavior) are defined in contracts, patchable via applyServerSettingsPatch, and treated as atomic duration fields like other git/health intervals.

The reaper resolves inactivity threshold on every sweep so edits apply without restart; sweep interval is read once at start() (restart to change). Explicit ProviderSessionReaperLiveOptions still override settings for tests. Failed threshold reads fail that sweep (logged, retried next time) instead of silently using a default; sweep interval read falls back to the contract default with a warning so startup does not block.

Tests cover settings-driven reaping, a high threshold that skips reaping, and option-over-settings precedence, with harness wiring for ServerSettings.layerTest.

Reviewed by Cursor Bugbot for commit 47869a8. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Make provider session reaper timing configurable via server settings

  • Adds providerSessionInactivityThreshold and providerSessionSweepInterval fields to ServerSettings schema in settings.ts, defaulting to 30 minutes and 5 minutes respectively.
  • ProviderSessionReaper now reads these values from ServerSettingsService instead of using hardcoded constants; explicit ProviderSessionReaperLiveOptions still override server settings.
  • The sweep interval is resolved once at startup (with a warning + fallback on failure); the inactivity threshold is re-read from settings on every sweep.
  • applyServerSettingsPatch in serverSettings.ts propagates the new fields when patching.
  • Risk: a settings read failure for the inactivity threshold causes that sweep run to fail and be logged, rather than silently using a fallback.

Macroscope summarized 47869a8.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0107b2d6-a788-4550-847e-d5d7da32114b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 6, 2026

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the changed Effect service code (ProviderSessionReaper, serverSettings, contracts/shared settings) against the service conventions. One finding: an aliased named import that erases the service module namespace. Dependency acquisition (yield* ServerSettingsService), layer construction, options-as-pure-configuration, and the new schema fields all look consistent with the conventions.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/provider/Layers/ProviderSessionReaper.test.ts Outdated
Comment thread apps/server/src/provider/Layers/ProviderSessionReaper.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d7bb9b1. Configure here.

Comment thread apps/server/src/provider/Layers/ProviderSessionReaper.ts
@macroscopeapp

macroscopeapp Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved f7fea00

This PR makes existing hardcoded provider session reaper timing values configurable via server settings, preserving the same defaults. The change is additive, well-tested, and doesn't alter runtime behavior unless explicitly configured.

You can customize Macroscope's approvability policy. Learn more.

kraptor23 and others added 2 commits August 6, 2026 15:44
The idle session reaper's 30-minute inactivity threshold and 5-minute
sweep interval were hardcoded; the only recourse for long-running
sessions being stopped between messages was patching the bundle.

Add providerSessionInactivityThreshold and providerSessionSweepInterval
server settings (millis in settings.json, RPC-patchable) consumed by
ProviderSessionReaper. The threshold is re-read every sweep so edits
apply live; the sweep interval is fixed when the reaper starts. Explicit
ProviderSessionReaperLiveOptions still override both, preserving
existing test seams.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ailable

Review feedback: start() yielded the failable threshold read before
forking, so a failed settings read could abort the reactors.start
startup phase even though the sweep re-reads the threshold anyway.
Resolve it non-fatally for the started log only. Also use the module
namespace import for serverSettings in the test and Effect DateTime
instead of a global Date in test fixtures.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant