Reduce idle work and disk churn with native resource diagnostics#2679

🟢 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.

The obsolete write-queue implementation was removed by the settings persistence refactor in this PR, so reset can no longer detach an in-flight promise chain. Settings writes now flow through the current server-backed settings path.

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.

^{Reviewed by Cursor Bugbot for commit 038e209ececaa278e51971a83a73cf11e7ecdeb8. Configure here.}

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.

^{Reviewed by Cursor Bugbot for commit 038e209ececaa278e51971a83a73cf11e7ecdeb8. Configure here.}

🟡 Medium

https://github.com/pingdotgg/t3code/blob/8da08596ca6d3ee2b5e6ae44164ee396a48bcf3f/apps/web/src/components/settings/SettingsPanels.tsx#L181-L186

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.

Fixed in 24e6ccd404. Host power NumberFields now normalize null and non-finite values to their declared five-second minimum; the integrated UI check cleared the field, reopened the dialog, and verified it persisted as 5 rather than 0.

Advanced dialog clears overrides

Medium Severity

In the advanced Background Activity dialog, changing “Shared policy” calls backgroundActivityProfileSettings, which sets profile to a preset and overrides to {}. Custom interval and pause toggles are wiped instead of updating baseProfile while staying on a custom profile.

^{Reviewed by Cursor Bugbot for commit 79ad07118493b7dfdc96c9205a913b356547f5bb. Configure here.}

VCS scope cwd mismatch

High Severity

Remote VCS polling now gates on shouldRunScopeWork using the broadcaster’s normalized cwd, while client activity leases retain vcs-status with the raw project path. When those strings differ (for example after realPath), scope keys never match, so automatic remote refreshes stay disabled even with an active subscription and foreground client.

^{Reviewed by Cursor Bugbot for commit 79ad07118493b7dfdc96c9205a913b356547f5bb. Configure here.}

Disconnect never clears leases

Medium Severity

removeRpcClient is implemented and tested but never invoked when a WebSocket RPC client disconnects. Activity leases keyed by rpcClientId remain until TTL expiry, so background Git and provider work can keep running briefly after the client is gone.

^{Reviewed by Cursor Bugbot for commit 79ad07118493b7dfdc96c9205a913b356547f5bb. Configure here.}

🔴 Critical

https://github.com/pingdotgg/t3code/blob/79ad07118493b7dfdc96c9205a913b356547f5bb/apps/server/src/ws.ts#L210

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.

Fixed in the rebased stack. The activity and resource telemetry RPC methods now declare authorization scopes in RpcAuthorization.ts, with focused authorization coverage. The integrated diagnostics page no longer reports undeclared-scope errors.

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 0ebbdad6c2e35404789d04c498adeb05858ad981. Configure here.}

Fixed in 24e6ccd404. Collection-control commands are sent while the native client is healthy or degraded, so a recoverable error cannot freeze power policy updates while the sidecar remains connected.

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.

^{Reviewed by Cursor Bugbot for commit 2b59d67c05c980eb48ed34ca04d599dc08f405e3. Configure here.}

Readiness timeout writes failure logs

Medium Severity

The new onReadinessFailure handler calls persistFailure when the HTTP readiness wait times out, even though the backend child is still running and output buffering was meant for real process failures. That writes ERROR failure logs for a slow or stuck bootstrap, and because persistFailure only clears chunks while leaving the session open, a later unexpected exit can flush another failure block for the same run.

^{Reviewed by Cursor Bugbot for commit 4a875dbe519b7891388272f0c81e0498619cc4f4. Configure here.}

Host suspend does not gate work

Medium Severity

isHostConstrained pauses background work for lock, thermal pressure, low-power mode, and battery, but ignores hostPower.suspended even when desktop telemetry reports suspend/resume. Scoped and opportunistic refreshes can keep running while the host is suspended.

^{Reviewed by Cursor Bugbot for commit 4a875dbe519b7891388272f0c81e0498619cc4f4. Configure here.}

🟡 Medium telemetry/DesktopTelemetryPublisher.ts:234

There is a race between reading Ref.get(latest) (line 236) and Stream.fromPubSub(changes) subscribing to the PubSub (line 245). Because the forked sampleOnce loop runs concurrently, a snapshot can be published to changes after latest is read but before the PubSub subscription is established. That snapshot updates the Ref and is published, but no subscriber is listening yet, so the consumer never receives it — it sees the stale latest value and then only snapshots arriving after subscription.

To close the gap, subscribe to the changes PubSub first, then read latest, so any snapshot published during or after the read is captured by the already-active subscription.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

🟢 Low resourceTelemetry/ResourceMonitorBinary.ts:172

When a candidate exists but is not executable, the code immediately returns ResourceMonitorBinaryNotExecutable, aborting the loop and preventing later fallback candidates from being checked. If a valid executable exists further down the list, it is never reached. Consider using continue to keep checking the remaining candidates.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

Stuck flush flag drops batched logs

High Severity

The flushScheduled flag can become out of sync with the actual timer, particularly if scheduling is interrupted or fails. This causes subsequent writes to incorrectly assume a flush is pending, leading to events remaining buffered in memory longer than the batch window, especially for low-volume streams.

^{Reviewed by Cursor Bugbot for commit 2f8211fa857c9fbe01a1e852a5f707efe98835fd. Configure here.}

Flush failure disables thread logging

Medium Severity

In drainPending, a single failed append adds the thread segment to failedSegments, clears the entire pending queue, and later flushes skip that segment permanently. A transient disk error therefore stops all native and canonical logging for that thread for the rest of the process, with no recovery path.

^{Reviewed by Cursor Bugbot for commit 2f8211fa857c9fbe01a1e852a5f707efe98835fd. Configure here.}

Process diagnostics use stale cache

Medium Severity

ProcessDiagnostics.read now maps directly from ResourceTelemetry.latest without sampling or refresh. Legacy callers that only hit the process diagnostics RPC get a cached snapshot, which can lag or stay empty when live telemetry is not subscribed, unlike the old on-demand ps query.

^{Reviewed by Cursor Bugbot for commit 8e2d5d326a0532e87806c217f8108d6b981bd823. Configure here.}

🟢 Low Layers/EventNdjsonLogger.ts:168

resolveStreamLabel defaults to "CANON" for every stream except "native", so "orchestration" events are mislabeled as "CANON" in the log output. Add an explicit branch for "orchestration" that returns a distinct label such as "ORCH".

Fixed in 24e6ccd404. Orchestration records now use the distinct ORCH label, and the event logger test asserts that label for global orchestration events.

Disconnect leaves activity leases

High Severity

serverReportClientActivity records leases keyed by RPC client id, and BackgroundPolicy.removeRpcClient exists to clear them, but the websocket layer never calls it on disconnect. Leases keep scoped demand alive until TTL expiry, so background VCS/provider work can continue after the client is gone.

^{Reviewed by Cursor Bugbot for commit deb3d48317e7f69bbc7c37fccda09d6e075e6218. Configure here.}

Fixed in 24e6ccd404. Each websocket layer tracks the RPC client IDs it observed and removes all corresponding BackgroundPolicy leases in the connection-scope finalizer.

🟡 Medium src/main.rs:592

When SampleNow is received, next_sample_at is reset to Instant::now() + interval, which restarts the periodic sampling timer from scratch. If on-demand SampleNow requests arrive more frequently than the configured sample_interval, the periodic deadline is continually postponed and periodic snapshots are never emitted, breaking the configured streaming/history cadence. An on-demand sample should not extend an existing periodic deadline — only set one if it doesn't already exist.

Fixed in 24e6ccd404. SampleNow preserves an existing periodic deadline and only creates one when none exists. A native unit test verifies on-demand samples do not postpone the scheduled cadence.

🟡 Medium settings/SettingsPanels.tsx:249

Deleting an override key from nextOverrides does not clear the setting — the server merges the returned patch with deepMerge(current.backgroundActivity, backgroundActivity), so a missing key is retained from the current overrides. As a result, when a caller passes undefined for an override (e.g. resetting an interval), the old custom value stays in place and the reset has no effect. The patch must explicitly replace the entire overrides record or use a deletion representation understood by the merge layer.

🟡 Medium resourceTelemetry/Model.ts:287

applyLifecycleCounters attributes each delta only by the process's current category. When a process's category changes between samples (e.g. first classified as server-child and later as electron-main after desktop metrics arrive), the CPU/I/O deltas already accumulated under backend stay there permanently while subsequent deltas go to electron. This permanently corrupts both group aggregates — the backend group retains CPU time and I/O bytes that belong to the electron group, and the electron group never recovers them. Consider detecting identity keys whose category differs from the previous sample and rebalancing the prior contributions out of the old group and into the new one.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

🟡 Medium background/BackgroundPolicy.ts:200

reportClientActivity unconditionally overwrites the existing lease and ignores input.observedAt, so an older activity report processed out of order can replace a newer one's visibility, focus, and scopes while still getting a fresh server-side expiry. This causes background work to be incorrectly enabled or suppressed until the next heartbeat. Consider comparing input.observedAt against the existing lease's timestamp and discarding stale reports before overwriting.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

🟡 Medium resourceTelemetry/Model.ts:365

When an Electron process is first observed only in desktop metrics, syntheticNativeSample creates a synthetic native sample using metric.creationTimeMs as startTimeMs. If the same PID later appears in the native monitor snapshot, its startTimeMs may differ by up to ELECTRON_IDENTITY_TOLERANCE_MS (2 seconds) — a difference the code explicitly tolerates for matching. But processIdentityKey is built from exact startTimeMs equality, so the later native sample gets a different identity key. This causes the process to be counted as an exit plus a new start, resets its CPU/IO rate history, and produces incorrect lifecycle telemetry for what is actually the same process. Consider deriving the synthetic sample's startTimeMs from the native sample when one later arrives, or using a fuzzy identity key that accounts for the tolerated time difference.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

🟡 Medium background/BackgroundPolicy.ts:45

reportClientActivity always uses the hard-coded DEFAULT_LEASE_TTL_MS (45s) as the lease TTL fallback and never reads ResolvedBackgroundActivitySettings.idleClientTtl. That setting is otherwise unused, so changing the configured idle-client TTL has no effect: leases remain active for 45 seconds regardless of the configured value, causing background work to run for the wrong duration after a client becomes idle. The fix is to source the default TTL from the resolved settings rather than the hard-coded constant.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

🟡 Medium resourceTelemetry/ResourceMonitorBinary.ts:167

resolve checks only that a candidate path exists and (on Unix) that it has an execute bit — it never verifies the path is a regular file. A T3CODE_RESOURCE_MONITOR_PATH or resourceMonitorPath pointing at a directory is returned as the monitor executable, so the caller later tries to spawn a directory and telemetry silently fails instead of getting a resolution error. Consider checking stat.isFile() (or the equivalent on the FileSystem API) before accepting a candidate.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

🟡 Medium resourceTelemetry/NativeTelemetryClient.ts:504

runAttempt waits for the handshake using only Deferred.await(helloDeferred) with a 5-second timeout, without racing it against eventFiber. If the child process exits, stdout closes, or a protocol/decode error occurs before the hello event arrives, eventFiber has already failed but the attempt stays blocked for the full timeout and then reports NativeTelemetryHandshakeTimedOut, hiding the real failure and delaying every restart by 5 seconds. Consider racing Deferred.await(helloDeferred) against Fiber.join(eventFiber) so the actual error propagates immediately.

No longer relevant as of 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0

🟡 Medium

https://github.com/pingdotgg/t3code/blob/8f3849bcf0d1db5b439017f24840a6131f66e486/apps/desktop/src/backend/DesktopBackendManager.ts#L506

Every backend instance routes its telemetry control stream into the same desktopTelemetryPublisher.handleControl, and each stream's Effect.ensuring unconditionally sends enabled: false when it closes. When two instances are running, stopping or restarting either one sends enabled: false to the shared publisher and clears diagnostics demand for the other live backend, so Electron telemetry sampling stops for a backend that is still running. Demand needs to be tracked per instance or aggregated across subscribers rather than funneled into one global boolean.

✅ Resolved in 56b763edb5f03ea538e7b72ee9699b19e44424cb

🟡 Medium Layers/EventNdjsonLogger.ts:381

enforceRetention can never delete a log file once its sink has been opened, because drainPending adds every entry from the sinks map to activeFilePaths and sinks are never removed from that map for the lifetime of the store. This means maxTotalBytes is not enforced for any file that has been written to: once opened sinks' combined size exceeds the budget (e.g. ~52 thread logs at the default 10 MiB each exceed the 512 MiB limit), those files are permanently protected and total disk usage grows without bound as new threads create new logs.

The activeFilePaths set should protect only the file currently being written in this drain cycle, not every sink the store has ever opened. Consider building it from the segments that have pending records in this drain, or from the specific RotatingFileSink currently being written to, rather than from all entries in the sinks map.

✅ Resolved in 56b763edb5f03ea538e7b72ee9699b19e44424cb

🟡 Medium resourceTelemetry/NativeTelemetryClient.ts:652

Concurrent calls to setHostPowerState or changeLiveSubscriberCount can overwrite the sidecar with stale values. Each call atomically commits the new control state to collectionControl via Ref.modify, then calls applyCollectionControl to send the corresponding commands — but these two steps are not serialized together. Two overlapping calls can commit in order A then B (so the ref holds B's state), yet have applyCollectionControl acquire commandMutex in order B then A, causing the older call A to send stale interval/streaming values to the sidecar last. The sidecar then runs at the wrong sampling interval or streaming mode until the next control change or restart, even though the in-memory ref reflects the newer state.

The same problem affects setExternalProcesses: Ref.set and the subsequent writeCommand are independent, so concurrent callers can have their commandMutex acquisition order differ from their ref write order, leaving the sidecar configured with an older process list while externalProcesses holds the newest.

Consider serializing the entire read-modify-send sequence (both the ref update and the command writes) under a shared mutex so that commits and their corresponding sidecar commands always apply in the same order.

Fixed in 24e6ccd404. A dedicated control semaphore serializes the state transition and sidecar command application as one unit, preventing older concurrent control calls from being written last.

The controlMutex correctly fixes the race for setHostPowerState and changeLiveSubscriberCount. However, setExternalProcesses (lines 729–739) still has the same race: Ref.set(externalProcesses, ...) and the subsequent writeCommand are not serialized together, so concurrent calls can still commit in one order but send commands in another. Consider wrapping setExternalProcesses under the same mutex (or a dedicated one) to serialize both the ref update and the command write.

🟠 High Layers/EventNdjsonLogger.ts:37

enforceRetention deletes any file matching /\.log(?:\.\d+)?$/u in the log directory, including at startup when activeFilePaths is empty. If makeEventNdjsonLogStore is pointed at a shared log directory, retention deletes unrelated *.log files from other components. Consider scoping the pattern to a provider-specific prefix so only files owned by this store are eligible for deletion.

No longer relevant as of 56b763edb5f03ea538e7b72ee9699b19e44424cb

🟡 Medium background/BackgroundPolicy.ts:238

BackgroundPolicy.make never subscribes to serverSettings.streamChanges, so changing the background profile or pause flags does not update the policy's streamChanges output until an unrelated lease/host-power event or the 15-second cleanup timer triggers publishSnapshot. Callers observing streamChanges receive stale policy state — and may act on it — after a settings update. Consider adding a Stream.runForEach(serverSettings.streamChanges, () => publishSnapshot) subscription alongside the existing host-power watcher.

✅ Resolved in 56b763edb5f03ea538e7b72ee9699b19e44424cb

🟡 Medium app/DesktopObservability.ts:447

persistFailure clears the buffered chunks but leaves sessionRef as Some, so a second call in the same run (e.g. finalizeRun after a readiness failure) writes a duplicate START/END failure record. The first call should reset sessionRef to Option.none() so the session is consumed and cannot be persisted twice.

✅ Resolved in 56b763edb5f03ea538e7b72ee9699b19e44424cb

🟡 Medium Layers/EventNdjsonLogger.ts:129

failedSegments permanently blacklists a thread segment after a single sink construction or write failure. drainPending skips every subsequent record for that segment and clears the pending queue, so a transient condition such as a temporary disk-full or permission error silently drops all future provider event log records for that thread for the remainder of the store lifetime — even after the filesystem recovers. Consider retrying the sink on a later flush or expiring entries from failedSegments instead of retaining them indefinitely.

✅ Resolved in 56b763edb5f03ea538e7b72ee9699b19e44424cb

🟡 Medium resourceTelemetry/DesktopTelemetryReceiver.ts:362

The stale checker overwrites newer telemetry with an older stale copy. A fresh message can update latest after the checker's Ref.get(latest) on line 357 but before its Ref.set(latest, ...) on line 366, so the checker replaces that newer snapshot with the stale one derived from the stale value it read earlier. Consumers then receive regressed telemetry incorrectly marked stale until the next sample arrives.

This happens because the read, staleness check, and write are split across multiple effects with a yield point on DateTime.now. Use an atomic Ref.modify that rechecks the current snapshot's sampledAtUnixMs before replacing it, so a newer sample is never clobbered.

✅ Resolved in 56b763edb5f03ea538e7b72ee9699b19e44424cb

Wrong hardcoded sampling interval label

Low Severity

The resource overview hardcodes Sampling every 5 seconds, but live diagnostics keep a subscriber so native sampling is typically 1s on AC (and sampleIntervalMs on the snapshot already carries the real interval). The label misreports collector cadence while users are watching the page.

^{Reviewed by Cursor Bugbot for commit 3f9ae0646c6f221e911a1ed4ede46a45cb361fc0. Configure here.}

Telemetry stream drops PubSub updates

Medium Severity

encoded builds its live stream by reading latest first and only then subscribing via Stream.fromPubSub(changes). Samples published in that gap are dropped, so a newly spawned backend can miss power or Electron metric updates until the next sample.

^{Reviewed by Cursor Bugbot for commit 38797e97c7798797cab825fca8004f5362bba3b4. Configure here.}

🟡 Medium background/BackgroundPolicy.ts:80

isHostConstrained never checks hostPower.suspended, so when the host is suspended but has no lock, low-power, battery, or thermal flags set, the function returns false and background work is allowed to run instead of being paused. Consider adding a check for hostPower.suspended === "true" alongside the existing host constraint conditions.

✅ Resolved in 56b763edb5f03ea538e7b72ee9699b19e44424cb