Add background activity policy and host power monitoring#2679
Draft
juliusmarminge wants to merge 1 commit into
Draft
Add background activity policy and host power monitoring#2679juliusmarminge wants to merge 1 commit into
juliusmarminge wants to merge 1 commit into
Conversation
- Track client leases, host power, and background work eligibility - Route provider snapshot refreshes through shared background policy - Add tests for the new policy and related server settings wiring
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Comment on lines
+9
to
+30
| export function createServerSettingsWriteQueue(input: { | ||
| readonly updateSettings: (patch: ServerSettingsPatch) => Promise<ServerSettings>; | ||
| readonly applySettings: (settings: ServerSettings) => void; | ||
| readonly onError: (error: unknown) => void; | ||
| }): ServerSettingsWriteQueue { | ||
| let queue: Promise<void> = Promise.resolve(); | ||
|
|
||
| return { | ||
| enqueue: (patch) => { | ||
| queue = queue | ||
| .catch(() => undefined) | ||
| .then(async () => { | ||
| const settings = await input.updateSettings(patch); | ||
| input.applySettings(settings); | ||
| }) | ||
| .catch(input.onError); | ||
| }, | ||
| reset: () => { | ||
| queue = Promise.resolve(); | ||
| }, | ||
| drain: () => queue, | ||
| }; |
Contributor
There was a problem hiding this comment.
🟢 Low hooks/serverSettingsWriteQueue.ts:9
reset() breaks serialization by replacing queue with Promise.resolve() while prior operations are still in-flight. The old chain continues running and will invoke applySettings and onError, but new enqueue() calls start immediately from the resolved promise, so their operations run concurrently with the old ones. This allows applySettings callbacks to fire out-of-order, with stale settings potentially overwriting newer ones.
return {
enqueue: (patch) => {
queue = queue
.catch(() => undefined)
.then(async () => {
const settings = await input.updateSettings(patch);
input.applySettings(settings);
})
.catch(input.onError);
},
- reset: () => {
- queue = Promise.resolve();
- },
+ reset: () => {
+ const currentQueue = queue;
+ queue = queue.then(() => {});
+ return currentQueue;
+ },
drain: () => queue,
};🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/hooks/serverSettingsWriteQueue.ts around lines 9-30:
`reset()` breaks serialization by replacing `queue` with `Promise.resolve()` while prior operations are still in-flight. The old chain continues running and will invoke `applySettings` and `onError`, but new `enqueue()` calls start immediately from the resolved promise, so their operations run concurrently with the old ones. This allows `applySettings` callbacks to fire out-of-order, with stale settings potentially overwriting newer ones.
Evidence trail:
apps/web/src/hooks/serverSettingsWriteQueue.ts lines 16-32 (reset() implementation at lines 28-30, enqueue() at lines 18-25); apps/web/src/hooks/useSettings.ts line 237-244 (__resetClientSettingsPersistenceForTests calls serverSettingsWriteQueue.reset()); apps/web/src/localApi.ts lines 194-204 (__resetLocalApiForTests calls __resetClientSettingsPersistenceForTests); git_grep for 'serverSettingsWriteQueue\.reset' shows only one caller in test cleanup code.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Legacy patch fields silently override explicit backgroundActivity patch
- Added a guard to skip computing backgroundActivityPatch when the explicit backgroundActivity field is present in the patch, ensuring new-style patches take precedence over legacy fields.
- ✅ Fixed: Duplicated utility functions across settings panel files
- Extracted BackgroundActivityOverridePatch, durationToSeconds, normalizeIntervalSeconds, backgroundActivityOverrideSettings, and BackgroundPolicyTooltip into a shared backgroundActivityUtils.tsx module imported by both SettingsPanels.tsx and SourceControlSettings.tsx.
Or push these changes by commenting:
@cursor push 6d184f0660
Preview (6d184f0660)
diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx
--- a/apps/web/src/components/settings/SettingsPanels.tsx
+++ b/apps/web/src/components/settings/SettingsPanels.tsx
@@ -1,7 +1,6 @@
import {
ArchiveIcon,
ArchiveX,
- InfoIcon,
LoaderIcon,
PlusIcon,
RefreshCwIcon,
@@ -106,6 +105,12 @@
SettingsSection,
useRelativeTimeTick,
} from "./settingsLayout";
+import {
+ BackgroundPolicyTooltip as PolicyTooltip,
+ backgroundActivityOverrideSettings,
+ durationToSeconds,
+ normalizeIntervalSeconds,
+} from "./backgroundActivityUtils";
import { ProjectFavicon } from "../ProjectFavicon";
import { useServerObservability, useServerProviders } from "../../rpc/serverState";
@@ -137,11 +142,6 @@
};
type BackgroundActivityProfileOption = BackgroundActivityProfile | "advanced";
-type BackgroundActivityOverridePatch = Partial<{
- [K in keyof BackgroundActivitySettings["overrides"]]:
- | BackgroundActivitySettings["overrides"][K]
- | undefined;
-}>;
const BACKGROUND_ACTIVITY_PROFILE_OPTION_LABELS: Record<BackgroundActivityProfileOption, string> = {
...BACKGROUND_ACTIVITY_PROFILE_LABELS,
@@ -174,17 +174,6 @@
{ key: "pauseWhenOnBattery", label: "Pause on battery" },
];
-function durationToSeconds(duration: Duration.Duration): number {
- return Math.round(Duration.toMillis(duration) / 1_000);
-}
-
-function normalizeIntervalSeconds(value: number | null): number {
- if (value === null || !Number.isFinite(value)) {
- return 0;
- }
- return Math.max(0, Math.round(value));
-}
-
function resolveBackgroundActivityProfileOption(settings: {
readonly backgroundActivity: BackgroundActivitySettings;
}): BackgroundActivityProfileOption {
@@ -209,50 +198,6 @@
};
}
-function backgroundActivityOverrideSettings(
- current: BackgroundActivitySettings,
- overrides: BackgroundActivityOverridePatch,
-) {
- const nextOverrides: BackgroundActivityOverridePatch = {
- ...current.overrides,
- ...overrides,
- };
- for (const [key, value] of Object.entries(nextOverrides)) {
- if (value === undefined) {
- delete nextOverrides[key as keyof typeof nextOverrides];
- }
- }
- return {
- backgroundActivity: {
- schemaVersion: 1 as const,
- profile: "custom" as const,
- baseProfile: getBackgroundActivityBaseProfile(current),
- overrides: nextOverrides as BackgroundActivitySettings["overrides"],
- },
- };
-}
-
-function PolicyTooltip({ children }: { readonly children: string }) {
- return (
- <Tooltip>
- <TooltipTrigger
- render={
- <button
- type="button"
- className="inline-flex size-5 items-center justify-center rounded-sm text-muted-foreground hover:text-foreground"
- aria-label="Background policy details"
- >
- <InfoIcon className="size-3.5" />
- </button>
- }
- />
- <TooltipPopup side="top" className="max-w-72">
- {children}
- </TooltipPopup>
- </Tooltip>
- );
-}
-
function withoutProviderInstanceKey<V>(
record: Readonly<Record<ProviderInstanceId, V>> | undefined,
key: ProviderInstanceId,
diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx
--- a/apps/web/src/components/settings/SourceControlSettings.tsx
+++ b/apps/web/src/components/settings/SourceControlSettings.tsx
@@ -1,9 +1,8 @@
-import { ChevronDownIcon, GitPullRequestIcon, InfoIcon, RefreshCwIcon } from "lucide-react";
+import { ChevronDownIcon, GitPullRequestIcon, RefreshCwIcon } from "lucide-react";
import * as Duration from "effect/Duration";
import * as Option from "effect/Option";
import { useState, type ReactNode } from "react";
import type {
- BackgroundActivitySettings,
SourceControlProviderKind,
SourceControlDiscoveryResult,
SourceControlProviderAuth,
@@ -55,6 +54,12 @@
} from "../Icons";
import { RedactedSensitiveText } from "./RedactedSensitiveText";
import { SettingResetButton, SettingsPageContainer, SettingsSection } from "./settingsLayout";
+import {
+ BackgroundPolicyTooltip,
+ backgroundActivityOverrideSettings,
+ durationToSeconds,
+ normalizeIntervalSeconds as normalizeFetchIntervalSeconds,
+} from "./backgroundActivityUtils";
const EMPTY_DISCOVERY_RESULT: SourceControlDiscoveryResult = {
versionControlSystems: [],
@@ -75,44 +80,7 @@
const SOURCE_CONTROL_SKELETON_ROWS = ["primary", "secondary"] as const;
const GIT_FETCH_INTERVAL_STEP_SECONDS = 5;
-type BackgroundActivityOverridePatch = Partial<{
- [K in keyof BackgroundActivitySettings["overrides"]]:
- | BackgroundActivitySettings["overrides"][K]
- | undefined;
-}>;
-function durationToSeconds(duration: Duration.Duration): number {
- return Math.round(Duration.toMillis(duration) / 1_000);
-}
-
-function normalizeFetchIntervalSeconds(value: number | null): number {
- if (value === null || !Number.isFinite(value)) {
- return 0;
- }
- return Math.max(0, Math.round(value));
-}
-
-function BackgroundPolicyTooltip({ children }: { readonly children: string }) {
- return (
- <Tooltip>
- <TooltipTrigger
- render={
- <button
- type="button"
- className="inline-flex size-5 items-center justify-center rounded-sm text-muted-foreground hover:text-foreground"
- aria-label="Background policy details"
- >
- <InfoIcon className="size-3.5" />
- </button>
- }
- />
- <TooltipPopup side="top" className="max-w-72">
- {children}
- </TooltipPopup>
- </Tooltip>
- );
-}
-
function optionLabel(value: Option.Option<string>): string | null {
return Option.getOrNull(value);
}
@@ -335,28 +303,6 @@
);
const canResetFetchInterval =
automaticGitFetchIntervalSeconds !== defaultAutomaticGitFetchIntervalSeconds;
- const backgroundActivityOverrideSettings = (
- current: BackgroundActivitySettings,
- overrides: BackgroundActivityOverridePatch,
- ) => {
- const nextOverrides: BackgroundActivityOverridePatch = {
- ...current.overrides,
- ...overrides,
- };
- for (const [key, value] of Object.entries(nextOverrides)) {
- if (value === undefined) {
- delete nextOverrides[key as keyof typeof nextOverrides];
- }
- }
- return {
- backgroundActivity: {
- schemaVersion: 1 as const,
- profile: "custom" as const,
- baseProfile: getBackgroundActivityBaseProfile(current),
- overrides: nextOverrides as BackgroundActivitySettings["overrides"],
- },
- };
- };
return (
<div className="grid gap-3">
diff --git a/apps/web/src/components/settings/backgroundActivityUtils.tsx b/apps/web/src/components/settings/backgroundActivityUtils.tsx
new file mode 100644
--- /dev/null
+++ b/apps/web/src/components/settings/backgroundActivityUtils.tsx
@@ -1,0 +1,67 @@
+import { InfoIcon } from "lucide-react";
+import * as Duration from "effect/Duration";
+import type { BackgroundActivitySettings } from "@t3tools/contracts";
+import { getBackgroundActivityBaseProfile } from "@t3tools/shared/backgroundActivitySettings";
+
+import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";
+
+export type BackgroundActivityOverridePatch = Partial<{
+ [K in keyof BackgroundActivitySettings["overrides"]]:
+ | BackgroundActivitySettings["overrides"][K]
+ | undefined;
+}>;
+
+export function durationToSeconds(duration: Duration.Duration): number {
+ return Math.round(Duration.toMillis(duration) / 1_000);
+}
+
+export function normalizeIntervalSeconds(value: number | null): number {
+ if (value === null || !Number.isFinite(value)) {
+ return 0;
+ }
+ return Math.max(0, Math.round(value));
+}
+
+export function backgroundActivityOverrideSettings(
+ current: BackgroundActivitySettings,
+ overrides: BackgroundActivityOverridePatch,
+) {
+ const nextOverrides: BackgroundActivityOverridePatch = {
+ ...current.overrides,
+ ...overrides,
+ };
+ for (const [key, value] of Object.entries(nextOverrides)) {
+ if (value === undefined) {
+ delete nextOverrides[key as keyof typeof nextOverrides];
+ }
+ }
+ return {
+ backgroundActivity: {
+ schemaVersion: 1 as const,
+ profile: "custom" as const,
+ baseProfile: getBackgroundActivityBaseProfile(current),
+ overrides: nextOverrides as BackgroundActivitySettings["overrides"],
+ },
+ };
+}
+
+export function BackgroundPolicyTooltip({ children }: { readonly children: string }) {
+ return (
+ <Tooltip>
+ <TooltipTrigger
+ render={
+ <button
+ type="button"
+ className="inline-flex size-5 items-center justify-center rounded-sm text-muted-foreground hover:text-foreground"
+ aria-label="Background policy details"
+ >
+ <InfoIcon className="size-3.5" />
+ </button>
+ }
+ />
+ <TooltipPopup side="top" className="max-w-72">
+ {children}
+ </TooltipPopup>
+ </Tooltip>
+ );
+}
diff --git a/packages/shared/src/serverSettings.ts b/packages/shared/src/serverSettings.ts
--- a/packages/shared/src/serverSettings.ts
+++ b/packages/shared/src/serverSettings.ts
@@ -89,26 +89,28 @@
...patchForMerge
} = patch;
const backgroundActivityPatch =
- backgroundActivityProfile !== undefined
- ? {
- schemaVersion: 1 as const,
- profile: backgroundActivityProfile,
- overrides: {},
- }
- : automaticGitFetchInterval !== undefined || providerHealthRefreshInterval !== undefined
+ backgroundActivity !== undefined
+ ? undefined
+ : backgroundActivityProfile !== undefined
? {
schemaVersion: 1 as const,
- profile: "custom" as const,
- baseProfile: getBackgroundActivityBaseProfile(current.backgroundActivity),
- overrides: {
- ...current.backgroundActivity.overrides,
- ...(automaticGitFetchInterval !== undefined ? { automaticGitFetchInterval } : {}),
- ...(providerHealthRefreshInterval !== undefined
- ? { providerHealthRefreshInterval }
- : {}),
- },
+ profile: backgroundActivityProfile,
+ overrides: {},
}
- : undefined;
+ : automaticGitFetchInterval !== undefined || providerHealthRefreshInterval !== undefined
+ ? {
+ schemaVersion: 1 as const,
+ profile: "custom" as const,
+ baseProfile: getBackgroundActivityBaseProfile(current.backgroundActivity),
+ overrides: {
+ ...current.backgroundActivity.overrides,
+ ...(automaticGitFetchInterval !== undefined ? { automaticGitFetchInterval } : {}),
+ ...(providerHealthRefreshInterval !== undefined
+ ? { providerHealthRefreshInterval }
+ : {}),
+ },
+ }
+ : undefined;
const next = deepMerge(current, patchForMerge);
const nextWithReplacementsBase = {
...next,You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 038e209. Configure here.
| : {}), | ||
| ...(backgroundActivityPatch !== undefined | ||
| ? { backgroundActivity: backgroundActivityPatch } | ||
| : {}), |
Contributor
There was a problem hiding this comment.
Legacy patch fields silently override explicit backgroundActivity patch
Medium Severity
When a patch contains both an explicit backgroundActivity object AND a legacy field like automaticGitFetchInterval, the object spread ordering causes backgroundActivityPatch (derived from legacy fields) to silently overwrite the already-merged backgroundActivity value. The explicit new-style patch is lost because it's spread first and then immediately overwritten by the legacy-derived patch on lines 118-120.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 038e209. Configure here.
| [K in keyof BackgroundActivitySettings["overrides"]]: | ||
| | BackgroundActivitySettings["overrides"][K] | ||
| | undefined; | ||
| }>; |
Contributor
There was a problem hiding this comment.
Duplicated utility functions across settings panel files
Low Severity
SourceControlSettings.tsx defines BackgroundActivityOverridePatch, durationToSeconds, normalizeFetchIntervalSeconds, backgroundActivityOverrideSettings, and BackgroundPolicyTooltip which are identical (or trivially renamed) copies of the same types, functions, and components already defined in SettingsPanels.tsx. If the override-building logic needs a bug fix, it must be applied in both places.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 038e209. Configure here.
Contributor
ApprovabilityVerdict: Needs human review Introduces a new background activity policy system with host power monitoring, client activity tracking, new RPC endpoints, and configurable profiles—a substantial new feature affecting runtime behavior across multiple subsystems. An unresolved medium-severity comment also flags potential settings patch ordering issues. You can customize Macroscope's approvability policy. Learn more. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
bun fmtbun lintbun typecheckbun run testNote
High Risk
Introduces new background activity gating that changes when provider health checks and Git remote refreshes run, based on client demand and host power state; mistakes could unintentionally pause important refreshes or increase background load across platforms.
Overview
Adds a new shared background activity policy on the server (
BackgroundPolicy) that tracks per-connection client activity leases + requestedBackgroundScopes and combines that with host power constraints to decideshouldRunScopeWorkand opportunistic work.Introduces a host power monitoring service (
HostPowerMonitor) with platform-specific layers (including adaptive polling on macOS shell commands) and wires it into the server runtime; the policy publishes snapshots/streams and is exposed over new WS RPC methods (serverReportClientActivity,serverReportHostPowerState,serverGetBackgroundPolicy,subscribeBackgroundPolicy).Updates background callers to consult the policy: provider health refresh in
makeManagedServerProvideris now interval-driven by resolved background-activity settings and skips periodic refresh unlessprovider-statusdemand is present, andVcsStatusBroadcasternow gates automatic remote polling onshouldRunScopeWork({ type: "vcs-status", cwd }).Expands contracts + settings with background activity schemas (profiles, overrides, snapshots,
RpcClientId) and adds shared resolver/normalizer logic (shared/backgroundActivitySettings), with server settings patching updated to keep legacy flat fields in sync.Adds web support: a background activity reporter periodically sends client activity/scopes (and retains scopes for Git status subscriptions), settings UI gains background activity profile + advanced interval controls, and server settings writes are serialized via a new
serverSettingsWriteQueue. Tests are updated/added to cover policy behavior and demand-gated refreshes.Reviewed by Cursor Bugbot for commit 038e209. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add background activity policy and host power monitoring to gate background work
BackgroundPolicyservice that tracks client activity leases, host power state, and profile settings to decide whether opportunistic or scoped background work (git fetch, provider health checks, VCS status refreshes) should run.HostPowerMonitorwith platform-specific behavior: macOS reads idle time, battery state, low-power mode, and thermal state viaioreg/pmset; other platforms report unknown state.backgroundActivityto server settings with three built-in profiles (balanced,performance,battery-saver) and per-field overrides; legacy flat fields (automaticGitFetchInterval,providerHealthRefreshInterval) are kept in sync as aliases.reportClientActivity,reportHostPowerState,getBackgroundPolicy,subscribeBackgroundPolicy) and wires the web client to report activity and retained scopes (e.g.vcs-status) periodically and on visibility/focus changes.BackgroundPolicy.shouldRunScopeWorkreturns false, effectively pausing background I/O when no foreground client demands it.createServerSettingsWriteQueueto prevent out-of-order updates.automaticGitFetchIntervalandproviderHealthRefreshIntervalare now derived from the resolvedbackgroundActivityobject; any direct patches to these legacy fields are converted to custom overrides, which may affect existing settings.json files on upgrade.📊 Macroscope summarized 038e209. 27 files reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues