Reduce idle work and improve resource diagnostics with native telemetry#2679
Open
juliusmarminge wants to merge 6 commits into
Open
Reduce idle work and improve resource diagnostics with native telemetry#2679juliusmarminge wants to merge 6 commits into
juliusmarminge wants to merge 6 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 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.
Autofix Details
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.
Contributor
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
2 tasks
- 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 Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/a55b49df
branch
from
038e209 to
8da0859
Compare
Contributor
There was a problem hiding this comment.
🟡 Medium
t3code/apps/web/src/components/settings/SettingsPanels.tsx
Lines 181 to 186 in 8da0859
When a user clears the NumberField for host power monitor intervals, onValueChange receives null. normalizeIntervalSeconds(null) returns 0, bypassing the min={5} constraint and saving Duration.seconds(0) to settings. This violates the minimum interval requirement, which could cause excessive background polling.
Consider enforcing the minimum in normalizeIntervalSeconds or handling the null case explicitly to ensure the saved value respects the min constraint.
function normalizeIntervalSeconds(value: number | null): number {
if (value === null || !Number.isFinite(value)) {
- return 0;
+ return NaN;
}
- return Math.max(0, Math.round(value));
+ return Math.max(5, Math.round(value));
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/settings/SettingsPanels.tsx around lines 181-186:
When a user clears the `NumberField` for host power monitor intervals, `onValueChange` receives `null`. `normalizeIntervalSeconds(null)` returns `0`, bypassing the `min={5}` constraint and saving `Duration.seconds(0)` to settings. This violates the minimum interval requirement, which could cause excessive background polling.
Consider enforcing the minimum in `normalizeIntervalSeconds` or handling the `null` case explicitly to ensure the saved value respects the `min` constraint.
Evidence trail:
SettingsPanels.tsx lines 181-186: normalizeIntervalSeconds returns 0 for null. Lines 778-801: Host power monitor active interval NumberField with min={5} and onValueChange at line 784 that saves Duration.seconds(normalizeIntervalSeconds(value)). Lines 804-835: Idle host power monitor NumberField also min={5}. Lines 212-233: backgroundActivityOverrideSettings performs no validation on the duration value. apps/web/package.json line 16: @base-ui/react ^1.4.1. apps/web/src/components/ui/number-field.tsx: NumberField wraps @base-ui/react NumberFieldPrimitive.Root with props passthrough.
Co-authored-by: codex <codex@users.noreply.github.com> # Conflicts: # apps/server/src/provider/Drivers/CodexDriver.ts # apps/server/src/provider/Drivers/CursorDriver.ts # apps/server/src/provider/Drivers/OpenCodeDriver.ts # apps/server/src/server.ts # apps/server/src/ws.ts # apps/web/src/environments/runtime/service.ts # apps/web/src/lib/gitStatusState.ts # packages/client-runtime/src/wsRpcClient.ts # packages/contracts/src/rpc.ts # packages/shared/src/serverSettings.test.ts
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: VCS scope cwd mismatch
- Added a scopeCwd parameter to makeRemoteRefreshLoop and retainRemotePoller that passes the raw (un-normalized) input.cwd to shouldRunScopeWork, ensuring scope keys match the client's lease scopes.
- ✅ Fixed: Disconnect never clears leases
- Added removeSession method to BackgroundPolicy that removes leases by sessionId, and wired it into the WebSocket disconnect handler's acquireUseRelease release function.
- ✅ Fixed: Advanced dialog clears overrides
- Changed the Advanced dialog's shared policy Select to update baseProfile while keeping profile as 'custom' and preserving existing overrides, instead of calling backgroundActivityProfileSettings which resets everything.
Or push these changes by commenting:
@cursor push 44ee7a6c3a
Preview (44ee7a6c3a)
diff --git a/apps/server/src/background/BackgroundPolicy.ts b/apps/server/src/background/BackgroundPolicy.ts
--- a/apps/server/src/background/BackgroundPolicy.ts
+++ b/apps/server/src/background/BackgroundPolicy.ts
@@ -30,6 +30,7 @@
input: ClientActivityReportInput,
) => Effect.Effect<void>;
readonly removeRpcClient: (rpcClientId: RpcClientId) => Effect.Effect<void>;
+ readonly removeSession: (sessionId: AuthSessionId) => Effect.Effect<void>;
readonly reportHostPowerState: (snapshot: HostPowerSnapshot) => Effect.Effect<void>;
readonly snapshot: Effect.Effect<BackgroundPolicySnapshot>;
readonly streamChanges: Stream.Stream<BackgroundPolicySnapshot>;
@@ -218,6 +219,17 @@
return next;
}).pipe(Effect.andThen(publishSnapshot), Effect.asVoid);
+ const removeSession: BackgroundPolicyShape["removeSession"] = (sessionId) =>
+ Ref.update(leasesRef, (leases) => {
+ const next = new Map(leases);
+ for (const [key, lease] of next) {
+ if (lease.sessionId === sessionId) {
+ next.delete(key);
+ }
+ }
+ return next;
+ }).pipe(Effect.andThen(publishSnapshot), Effect.asVoid);
+
const hasDemand: BackgroundPolicyShape["hasDemand"] = (scope) =>
Effect.map(snapshot, (current) => current.activeScopeKeys.includes(scopeKey(scope)));
@@ -264,6 +276,7 @@
return BackgroundPolicy.of({
reportClientActivity,
removeRpcClient,
+ removeSession,
reportHostPowerState: hostPowerMonitor.report,
snapshot,
streamChanges: Stream.fromPubSub(changes),
diff --git a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts
--- a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts
+++ b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts
@@ -64,6 +64,7 @@
const BackgroundPolicyAlwaysRunLayer = Layer.mock(BackgroundPolicy.BackgroundPolicy)({
reportClientActivity: () => Effect.void,
removeRpcClient: () => Effect.void,
+ removeSession: () => Effect.void,
reportHostPowerState: () => Effect.void,
snapshot: Effect.succeed({
hostPower: {
diff --git a/apps/server/src/provider/Layers/ProviderRegistry.test.ts b/apps/server/src/provider/Layers/ProviderRegistry.test.ts
--- a/apps/server/src/provider/Layers/ProviderRegistry.test.ts
+++ b/apps/server/src/provider/Layers/ProviderRegistry.test.ts
@@ -78,6 +78,7 @@
const BackgroundPolicyAlwaysRunLayer = Layer.mock(BackgroundPolicy.BackgroundPolicy)({
reportClientActivity: () => Effect.void,
removeRpcClient: () => Effect.void,
+ removeSession: () => Effect.void,
reportHostPowerState: () => Effect.void,
snapshot: Effect.succeed({
hostPower: {
diff --git a/apps/server/src/provider/makeManagedServerProvider.test.ts b/apps/server/src/provider/makeManagedServerProvider.test.ts
--- a/apps/server/src/provider/makeManagedServerProvider.test.ts
+++ b/apps/server/src/provider/makeManagedServerProvider.test.ts
@@ -97,6 +97,7 @@
return Layer.mock(BackgroundPolicy.BackgroundPolicy)({
reportClientActivity: () => Effect.void,
removeRpcClient: () => Effect.void,
+ removeSession: () => Effect.void,
reportHostPowerState: () => Effect.void,
snapshot: Effect.succeed({
hostPower: {
diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts
--- a/apps/server/src/server.test.ts
+++ b/apps/server/src/server.test.ts
@@ -777,6 +777,7 @@
Layer.mock(BackgroundPolicy.BackgroundPolicy)({
reportClientActivity: () => Effect.void,
removeRpcClient: () => Effect.void,
+ removeSession: () => Effect.void,
reportHostPowerState: () => Effect.void,
snapshot: Effect.succeed({
hostPower: {
diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts
--- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts
+++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts
@@ -92,6 +92,7 @@
return Layer.mock(BackgroundPolicy.BackgroundPolicy)({
reportClientActivity: () => Effect.void,
removeRpcClient: () => Effect.void,
+ removeSession: () => Effect.void,
reportHostPowerState: () => Effect.void,
snapshot: Effect.succeed({
hostPower: {
diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts
--- a/apps/server/src/vcs/VcsStatusBroadcaster.ts
+++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts
@@ -258,6 +258,7 @@
const makeRemoteRefreshLoop = (
cwd: string,
+ scopeCwd: string,
automaticRemoteRefreshInterval: Effect.Effect<Duration.Duration, never>,
refreshImmediately: boolean,
) => {
@@ -274,7 +275,7 @@
const shouldRun = yield* backgroundPolicy.shouldRunScopeWork({
type: "vcs-status",
- cwd,
+ cwd: scopeCwd,
});
if (!shouldRun) {
return activeInterval;
@@ -322,6 +323,7 @@
const retainRemotePoller = Effect.fn("VcsStatusBroadcaster.retainRemotePoller")(function* (
cwd: string,
+ scopeCwd: string,
automaticRemoteRefreshInterval: Effect.Effect<Duration.Duration, never>,
refreshImmediately: boolean,
) {
@@ -336,7 +338,12 @@
return Effect.succeed([undefined, nextPollers] as const);
}
- return makeRemoteRefreshLoop(cwd, automaticRemoteRefreshInterval, refreshImmediately).pipe(
+ return makeRemoteRefreshLoop(
+ cwd,
+ scopeCwd,
+ automaticRemoteRefreshInterval,
+ refreshImmediately,
+ ).pipe(
Effect.forkIn(broadcasterScope),
Effect.map((fiber) => {
const nextPollers = new Map(activePollers);
@@ -388,6 +395,7 @@
const initialRemote = cachedStatus?.remote?.value ?? null;
yield* retainRemotePoller(
cwd,
+ input.cwd,
options?.automaticRemoteRefreshInterval ??
Effect.succeed(DEFAULT_VCS_STATUS_REFRESH_INTERVAL),
cachedStatus?.remote === null || cachedStatus?.remote === undefined,
diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -1669,6 +1669,7 @@
const request = yield* HttpServerRequest.HttpServerRequest;
const serverAuth = yield* EnvironmentAuth.EnvironmentAuth;
const sessions = yield* SessionStore.SessionStore;
+ const backgroundPolicy = yield* BackgroundPolicy.BackgroundPolicy;
const session = yield* serverAuth.authenticateWebSocketUpgrade(request).pipe(
Effect.catchTags({
ServerAuthInvalidCredentialError: (error) => failEnvironmentAuthInvalid(error.reason),
@@ -1710,7 +1711,11 @@
return yield* Effect.acquireUseRelease(
sessions.markConnected(session.sessionId),
() => rpcWebSocketHttpEffect,
- () => sessions.markDisconnected(session.sessionId),
+ () =>
+ Effect.all([
+ sessions.markDisconnected(session.sessionId),
+ backgroundPolicy.removeSession(session.sessionId),
+ ]).pipe(Effect.asVoid),
);
}).pipe(
Effect.catchTags({
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
@@ -680,7 +680,14 @@
value === "performance" ||
value === "battery-saver"
) {
- updateSettings(backgroundActivityProfileSettings(value));
+ updateSettings({
+ backgroundActivity: {
+ schemaVersion: 1 as const,
+ profile: "custom" as const,
+ baseProfile: value,
+ overrides: settings.backgroundActivity.overrides,
+ },
+ });
}
}}
>You can send follow-ups to the cloud agent here.
Contributor
|
🚀 Expo continuous deployment is ready!
|
Contributor
There was a problem hiding this comment.
🔴 Critical
Line 210 in 79ad071
The new RPC handlers serverReportClientActivity, serverReportHostPowerState, serverGetBackgroundPolicy, and subscribeBackgroundPolicy are missing entries in RPC_REQUIRED_SCOPE, so requiredScopeForMethod() throws Error: RPC method X has no declared authorization scope. when any of these methods are invoked.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/ws.ts around line 210:
The new RPC handlers `serverReportClientActivity`, `serverReportHostPowerState`, `serverGetBackgroundPolicy`, and `subscribeBackgroundPolicy` are missing entries in `RPC_REQUIRED_SCOPE`, so `requiredScopeForMethod()` throws `Error: RPC method X has no declared authorization scope.` when any of these methods are invoked.
Evidence trail:
apps/server/src/ws.ts lines 143-212 (RPC_REQUIRED_SCOPE map — none of the four methods listed); apps/server/src/ws.ts lines 321-327 (requiredScopeForMethod throws if method not in map); apps/server/src/ws.ts lines 1112-1131 (serverReportClientActivity, serverReportHostPowerState, serverGetBackgroundPolicy handlers using observeRpcEffect); apps/server/src/ws.ts lines 1650-1658 (subscribeBackgroundPolicy handler using observeRpcStream); apps/server/src/ws.ts lines 333-345 (observeRpcEffect and observeRpcStream call requiredScopeForMethod).
Replace scheduled process shell probes with a persistent native collector, Electron power/process telemetry, Effect services, diagnostics UI, attribution, and release packaging. Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
t3dotgg
changed the title
t3dotgg
changed the title
- Add fd5 control channel for desktop telemetry demand - Stop gating background work on stale host power - Update telemetry sampling and receiver tests
| if (previous.sampleIntervalMs !== next.sampleIntervalMs) { | ||
| yield* publishHealth; | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
Power updates ignored when degraded
Medium Severity
setHostPowerState and live-subscriber changes always update in-memory collectionControl, but applyCollectionControl only sends setSampleInterval / setStreaming to the sidecar when client status is exactly healthy. While the monitor is degraded yet still running, host suspend or constraint changes will not pause or slow native sampling until the client returns to healthy or reconnects.
Reviewed by Cursor Bugbot for commit 0ebbdad. Configure here.
- Capture child process output in memory during normal runs - Persist buffered output only on failures and bound retained data - Increase trace/event batching windows to reduce flush churn
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Output drain timeout drops logs
- Added Fiber.interrupt for all output fibers after the 250ms drain timeout so that still-running fibers are stopped before persistFailure snapshots and clears the session buffer, preventing late chunks from being lost.
Or push these changes by commenting:
@cursor push 3c6ed7cfa6
Preview (3c6ed7cfa6)
diff --git a/apps/desktop/src/backend/DesktopBackendManager.ts b/apps/desktop/src/backend/DesktopBackendManager.ts
--- a/apps/desktop/src/backend/DesktopBackendManager.ts
+++ b/apps/desktop/src/backend/DesktopBackendManager.ts
@@ -330,6 +330,10 @@
concurrency: "unbounded",
discard: true,
}).pipe(Effect.timeout(DEFAULT_BACKEND_OUTPUT_DRAIN_TIMEOUT), Effect.ignore);
+ yield* Effect.forEach(outputFibers, Fiber.interrupt, {
+ concurrency: "unbounded",
+ discard: true,
+ }).pipe(Effect.ignore);
return exit;
});You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 2b59d67. Configure here.
| yield* Effect.forEach(outputFibers, Fiber.await, { | ||
| concurrency: "unbounded", | ||
| discard: true, | ||
| }).pipe(Effect.timeout(DEFAULT_BACKEND_OUTPUT_DRAIN_TIMEOUT), Effect.ignore); |
Contributor
There was a problem hiding this comment.
Output drain timeout drops logs
Medium Severity
After the backend child exits, runBackendProcess waits at most 250ms for stdout/stderr drain fibers, then finalizeRun calls persistFailure once. If draining or writeOutputChunk takes longer, or fibers still run after the timeout, failure logs can omit trailing output that never gets another persist pass.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2b59d67. Configure here.
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.
Intent
Reduce T3 Code's steady-state resource usage and make CPU, memory, power-state, and I/O regressions observable enough to debug.
Before this PR:
psor PowerShell, adding process churn while still missing short-lived work between samples;This PR replaces those paths with a demand-aware background policy plus a persistent, cross-platform telemetry pipeline.
Runtime architecture
The native monitor is intentionally a standalone executable, not an N-API or FFI module. One persistent child process gives the server a supervised crash boundary, avoids Node/Electron ABI coupling, and eliminates recurring process-table subprocesses. The same protocol and binary layout work for desktop and the published CLI.
What changed
Demand-aware background work
BackgroundPolicyservice that combines:Native and Electron telemetry
native/resource-monitor, a Rust sidecar backed by OS APIs throughsysinfo.node,tsgo, language servers and shells, Electron roots, Electron descendants, and the monitor itself.(pid, startTimeMs), parent/child relationships, command line, CPU, cumulative CPU time, resident/virtual memory, cumulative I/O counters, and collection cost.powerMonitorandapp.getAppMetrics()for battery/AC state, lock/idle, suspend/resume, thermal state, CPU speed limits, Electron CPU/memory, and idle wakeups.Diagnostics, history, and safety
ResourceTelemetryservice that merges native and Electron samples, computes rates from cumulative counters, classifies process groups, and publishes live snapshots.ps/PowerShell implementations entirely.Reliability and packaging
Metric semantics and limits
unknowninstead of falling back to shell probes.Validation
vp checkvp run typecheckcargo fmt --manifest-path native/resource-monitor/Cargo.toml -- --checkcargo test --locked --manifest-path native/resource-monitor/Cargo.toml(6 tests)cargo clippy --manifest-path native/resource-monitor/Cargo.toml -- -D warningsvp run build:desktopNote
High Risk
Large cross-cutting change (desktop IPC, new native binary packaging, diagnostics/signal semantics, and background scheduling) with behavior changes when clients omit startTimeMs or when the sidecar is missing on a platform.
Overview
Adds a Rust
t3-resource-monitorsidecar and wires it through CI/release (per-platform builds, CLI bundle, desktopresourceMonitorPathon backend bootstrap). Process diagnostics and resource history no longer spawnps/PowerShell; they read from a newResourceTelemetrypipeline that merges native snapshots with Electron main metrics sent over inherited fds 4/5 (DesktopTelemetryPublisher→ serverDesktopTelemetryReceiver).Introduces
BackgroundPolicyandHostPowerMonitorso scoped background work (e.g. provider status, VCS) can follow client leases, host power/thermal/battery, and activity profiles instead of fixed 5‑minute provider refresh intervals (thoserefreshIntervalknobs are removed from drivers).Desktop backend child logging is reworked: output is buffered (1 MiB / 256 chunks) and only flushed to disk on failure; graceful stop discards the session. The backend manager drains stdout/stderr before reporting unexpected exits and routes NDJSON control messages on fd5 for diagnostics demand.
Process signal RPCs now require
(pid, startTimeMs)identity validation and block Electron/monitor targets. Trace batch default moves 200 ms → 1 s; trace flushes record logical write attribution viaResourceAttribution.Reviewed by Cursor Bugbot for commit 2b59d67. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add native resource monitor and BackgroundPolicy to reduce idle work and improve diagnostics
native/resource-monitor/) that streams process snapshots and history via a JSON protocol, replacing direct OS command spawning (ps/powershell) inProcessDiagnosticsandProcessResourceMonitor.ResourceTelemetryservice that merges native and desktop (Electron) telemetry into unified snapshots, exposed over WebSocket RPCs and a newResourceTelemetryDiagnosticsUI.BackgroundPolicyservice that gates periodic work — VCS remote refreshes and provider health checks — based on active client leases, host power state, and the user's selected background activity profile.settings.backgroundActivity.DesktopTelemetryPublisherin the Electron main process to stream Electron app metrics and power state to the backend over dedicated FDs (4 and 5).persistFailureis called, reducing log noise on clean exits.ProcessDiagnostics.signalnow requires a matching(pid, startTimeMs)identity and rejects non-backend process categories — existing callers that do not supplystartTimeMswill receive validation failures.Macroscope summarized 2b59d67.