feat(server): make provider session reaper timing configurable - #5525
feat(server): make provider session reaper timing configurable#5525kraptor23 wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ 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.
ApprovabilityVerdict: 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. |
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>
f7fea00 to
47869a8
Compare

What
Adds
providerSessionInactivityThresholdandproviderSessionSweepIntervalserver settings (persisted as millis, defaults unchanged: 30 min / 5 min) consumed byProviderSessionReaper.Implements #5523. Related: #4198.
Why
Both reaper values are hardcoded;
ProviderSessionReaperLiveOptionsexists 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
packages/contracts/src/settings.ts(flat fields withSchema.DurationFromMillis+ decoding defaults, patch keys inServerSettingsPatch), placed outside the legacy-marked block. Whether these should instead live in thebackgroundActivityprofile system is raised in Provider session reaper inactivity threshold and sweep interval are not configurable #5523 — happy to rework.deepMerge/strip special-casing asautomaticGitFetchInterval(ATOMIC_SETTINGS_KEYS,makeTest,applyServerSettingsPatch).start()and documented as restart-required. A failed settings read fails the sweep — logged by the existingsweep-failedhandler and retried next sweep — instead of silently reaping with a default the user overrode; the interval read falls back to the default with asettings-fallbackwarning so startup never blocks.ProviderSessionReaperLiveOptionskeep precedence over settings, so existing test seams are unchanged.ServerSettingsLayerLivealready sits aboveProviderRuntimeLayerLiveinserver.ts, so no composition changes.Tests
vp test runon the touched test files (26 tests) andtsgo --noEmitfor contracts/shared/server pass.Positioning notes
providerSessionInactivityThresholdis the practical mitigation for long-running agent sessions; that is the immediate user demand behind Provider session reaper inactivity threshold and sweep interval are not configurable #5523.providerSessionSweepIntervalbounds the worst-case reap lag (a session becomes reap-eligible at most one interval before the next sweep observes it), which covers the configurability motivation of Replace provider session reaper polling with deadline-driven scheduling #2351 while deliberately keeping the simple polling model — the deadline-driven alternative was previously closed with real defects.start()and applies after a restart (documented in the schema comment).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 settingsproviderSessionInactivityThresholdandproviderSessionSweepInterval(defaults 30 min / 5 min, unchanged behavior) are defined in contracts, patchable viaapplyServerSettingsPatch, 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). ExplicitProviderSessionReaperLiveOptionsstill 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
providerSessionInactivityThresholdandproviderSessionSweepIntervalfields toServerSettingsschema in settings.ts, defaulting to 30 minutes and 5 minutes respectively.ProviderSessionReapernow reads these values fromServerSettingsServiceinstead of using hardcoded constants; explicitProviderSessionReaperLiveOptionsstill override server settings.applyServerSettingsPatchin serverSettings.ts propagates the new fields when patching.Macroscope summarized 47869a8.