From d043b4458bd6a99cdffac59af4c5e7d6a2631d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 21:22:55 +0200 Subject: [PATCH 1/2] refactor(daemon): route daemon-level diagnostics scopes through one helper withDaemonDiagnosticsScope owns the daemon session scope fields, runs the body, and forces the flush. All seven daemon-runtime call sites use it with their original command, debug flag, and flush timing. --- .../server/daemon-diagnostics-scope.test.ts | 84 +++++++++++ src/daemon/server/daemon-diagnostics-scope.ts | 35 +++++ src/daemon/server/daemon-runtime.ts | 132 +++++++----------- 3 files changed, 166 insertions(+), 85 deletions(-) create mode 100644 src/daemon/server/daemon-diagnostics-scope.test.ts create mode 100644 src/daemon/server/daemon-diagnostics-scope.ts diff --git a/src/daemon/server/daemon-diagnostics-scope.test.ts b/src/daemon/server/daemon-diagnostics-scope.test.ts new file mode 100644 index 0000000000..949a34971b --- /dev/null +++ b/src/daemon/server/daemon-diagnostics-scope.test.ts @@ -0,0 +1,84 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { afterEach, beforeEach, test } from 'vitest'; +import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; +import { withDaemonDiagnosticsScope } from './daemon-diagnostics-scope.ts'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; + +let dir: string; +let logPath: string; + +beforeEach(() => { + dir = mkdtempForTestSync('agent-device-daemon-diagnostics-scope-'); + logPath = path.join(dir, 'daemon.log'); +}); + +afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); +}); + +function readLoggedEvents(): Array> { + if (!fs.existsSync(logPath)) return []; + return fs + .readFileSync(logPath, 'utf8') + .split('\n') + .filter((line) => line.length > 0) + .map((line) => JSON.parse(line) as Record); +} + +test('defaults to a debug daemon scope on the daemon session and returns the body result', async () => { + const result = await withDaemonDiagnosticsScope({ logPath }, () => { + emitDiagnostic({ level: 'warn', phase: 'probe', data: { n: 1 } }); + return 'done'; + }); + + assert.equal(result, 'done'); + const events = readLoggedEvents(); + assert.equal(events.length, 1); + assert.equal(events[0]?.command, 'daemon'); + assert.equal(events[0]?.session, 'daemon'); + assert.equal(events[0]?.phase, 'probe'); +}); + +test('forces the buffered events of a non-debug scope out once the body returns', async () => { + await withDaemonDiagnosticsScope({ logPath, command: 'daemon-startup', debug: false }, () => { + emitDiagnostic({ level: 'warn', phase: 'first' }); + emitDiagnostic({ level: 'warn', phase: 'second' }); + assert.deepEqual(readLoggedEvents(), []); + }); + + const events = readLoggedEvents(); + assert.deepEqual( + events.map((event) => event.phase), + ['first', 'second'], + ); + assert.ok(events.every((event) => event.command === 'daemon-startup')); +}); + +test('leaves buffered events unwritten when the body throws', async () => { + await assert.rejects( + withDaemonDiagnosticsScope({ logPath, debug: false }, () => { + emitDiagnostic({ level: 'warn', phase: 'lost' }); + throw new Error('boom'); + }), + /boom/, + ); + + assert.deepEqual(readLoggedEvents(), []); +}); + +test('flushOnThrow keeps the record of a failed body and still rethrows', async () => { + await assert.rejects( + withDaemonDiagnosticsScope({ logPath, debug: false, flushOnThrow: true }, async () => { + emitDiagnostic({ level: 'warn', phase: 'kept' }); + throw new Error('boom'); + }), + /boom/, + ); + + assert.deepEqual( + readLoggedEvents().map((event) => event.phase), + ['kept'], + ); +}); diff --git a/src/daemon/server/daemon-diagnostics-scope.ts b/src/daemon/server/daemon-diagnostics-scope.ts new file mode 100644 index 0000000000..807ce15e0a --- /dev/null +++ b/src/daemon/server/daemon-diagnostics-scope.ts @@ -0,0 +1,35 @@ +import { + flushDiagnosticsToSessionFile, + withDiagnosticsScope, +} from '@agent-device/host-kit/diagnostics'; + +export type DaemonDiagnosticsScopeOptions = Readonly<{ + logPath: string; + command?: 'daemon' | 'daemon-startup'; + debug?: boolean; + /** Also force the flush when `body` throws, so the record of a failure is kept. */ + flushOnThrow?: boolean; +}>; + +/** + * Runs daemon-level work that has no request scope inside a `daemon` session scope, then forces its + * events to `logPath`. Without a scope `emitDiagnostic` drops every event. + */ +export async function withDaemonDiagnosticsScope( + options: DaemonDiagnosticsScopeOptions, + body: () => Promise | T, +): Promise { + const { logPath, command = 'daemon', debug = true, flushOnThrow = false } = options; + return await withDiagnosticsScope({ command, session: 'daemon', logPath, debug }, async () => { + if (!flushOnThrow) { + const result = await body(); + flushDiagnosticsToSessionFile({ force: true }); + return result; + } + try { + return await body(); + } finally { + flushDiagnosticsToSessionFile({ force: true }); + } + }); +} diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index 4544475bf0..20b3f8c93a 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/src/daemon/server/daemon-runtime.ts @@ -39,6 +39,7 @@ import type { DaemonInvokeFn } from '../daemon-request.ts'; import type { SessionRef, SessionState } from '../session-state.ts'; import type { RuntimeHintValues } from '@agent-device/contracts/application-lifecycle-runtime'; import { createDaemonIdleReap } from './daemon-idle-reap.ts'; +import { withDaemonDiagnosticsScope } from './daemon-diagnostics-scope.ts'; import { createSessionIdleExpiry } from './daemon-session-idle-expiry.ts'; import { resolveSessionIdleExpiryMs } from '../session-idle-expiry.ts'; import { finalizeDaemonLeases } from './daemon-lease-finalizer.ts'; @@ -52,12 +53,7 @@ import { createDaemonShutdownClaimLedger } from './daemon-shutdown-claims.ts'; import { createAudioProbeAdmissionLedger } from '@agent-device/capture-kit/audio-probe-admission-ledger'; import { createPerfCaptureAdmissionLedger } from '@agent-device/capture-kit/perf-capture-admission-ledger'; import { createScreenRecordingAdmissionLedger } from '@agent-device/capture-kit/screen-recording-admission-ledger'; -import { - emitDiagnostic, - flushDiagnosticsToSessionFile, - withDiagnosticsScope, - type ResourceDiagnostic, -} from '@agent-device/host-kit/diagnostics'; +import { emitDiagnostic, type ResourceDiagnostic } from '@agent-device/host-kit/diagnostics'; import { createOwnedProcessRecordStore, type OwnedProcessRecordStore, @@ -231,19 +227,15 @@ export async function flushDaemonStartupDiagnostics( diagnostics: readonly ResourceDiagnostic[], ): Promise { if (diagnostics.length === 0) return; - await withDiagnosticsScope( - { command: 'daemon-startup', session: 'daemon', logPath, debug: false }, - async () => { - for (const diagnostic of diagnostics) { - emitDiagnostic({ - level: 'warn', - phase: diagnostic.phase, - data: { resourcePath: diagnostic.resourcePath, ...diagnostic.data }, - }); - } - flushDiagnosticsToSessionFile({ force: true }); - }, - ); + await withDaemonDiagnosticsScope({ logPath, command: 'daemon-startup', debug: false }, () => { + for (const diagnostic of diagnostics) { + emitDiagnostic({ + level: 'warn', + phase: diagnostic.phase, + data: { resourcePath: diagnostic.resourcePath, ...diagnostic.data }, + }); + } + }); } async function noteSkippedProviderRuntimes( @@ -270,13 +262,9 @@ async function emitDaemonDiagnostic( phase: string, data: Record, ): Promise { - await withDiagnosticsScope( - { command: 'daemon', session: 'daemon', logPath, debug: true }, - async () => { - emitDiagnostic({ level: 'warn', phase, data }); - flushDiagnosticsToSessionFile({ force: true }); - }, - ); + await withDaemonDiagnosticsScope({ logPath }, () => { + emitDiagnostic({ level: 'warn', phase, data }); + }); } async function finishDaemonRegistration(params: { @@ -461,19 +449,15 @@ export async function startDaemonRuntime( let stopMetadataLossWatch: () => void = () => {}; const emitFatalDiagnostic = async (error: unknown): Promise => { - await withDiagnosticsScope( - { command: 'daemon', session: 'daemon', logPath, debug: true }, - async () => { - emitDiagnostic({ - level: 'error', - phase: 'daemon_fatal', - data: { - error: error instanceof Error ? error.message : String(error), - }, - }); - flushDiagnosticsToSessionFile({ force: true }); - }, - ); + await withDaemonDiagnosticsScope({ logPath }, () => { + emitDiagnostic({ + level: 'error', + phase: 'daemon_fatal', + data: { + error: error instanceof Error ? error.message : String(error), + }, + }); + }); }; const shutdownClaimLedger = createDaemonShutdownClaimLedger(); @@ -555,16 +539,7 @@ export async function startDaemonRuntime( // A sweep is out-of-request work, so it has no request scope and `emitDiagnostic` would drop // everything it reports — including the record of what was reclaimed and why a reclaim failed. withinDiagnosticsScope: async (run) => - await withDiagnosticsScope( - { command: 'daemon', session: 'daemon', logPath, debug: true }, - async () => { - try { - return await run(); - } finally { - flushDiagnosticsToSessionFile({ force: true }); - } - }, - ), + await withDaemonDiagnosticsScope({ logPath, flushOnThrow: true }, run), // An expiry can be the event that makes this daemon fully idle, and no request follows it to // arm the process-level reap. onSessionExpired: () => { @@ -794,27 +769,19 @@ export async function startDaemonRuntime( // forced on here because the declines are the point of the record — a handoff that silently // skipped is indistinguishable from a rebuild (#2681). try { - await withDiagnosticsScope( - { command: 'daemon', session: 'daemon', logPath, debug: true }, - async () => { - await applicationLifecycle.detachForDaemonShutdown(); - flushDiagnosticsToSessionFile({ force: true }); - }, - ); + await withDaemonDiagnosticsScope({ logPath }, async () => { + await applicationLifecycle.detachForDaemonShutdown(); + }); } catch {} expiredProviderLeaseReleaser.beginShutdown(); await teardownDaemonSessions(); - await withDiagnosticsScope( - { command: 'daemon', session: 'daemon', logPath, debug: true }, - async () => { - await finalizeDaemonLeases({ - leaseRegistry, - expiredProviderLeaseReleaser, - timeoutMs: DAEMON_LEASE_RELEASE_TIMEOUT_MS, - }); - flushDiagnosticsToSessionFile({ force: true }); - }, - ); + await withDaemonDiagnosticsScope({ logPath }, async () => { + await finalizeDaemonLeases({ + leaseRegistry, + expiredProviderLeaseReleaser, + timeoutMs: DAEMON_LEASE_RELEASE_TIMEOUT_MS, + }); + }); try { await platformDaemonLifecycleOwners.resetAndroidSnapshotHelper(); } catch (error) { @@ -896,25 +863,20 @@ async function reconcileDeviceClaimsForDaemonStartup( ): Promise { // Startup runs outside any diagnostics scope, where emitDiagnostic is a no-op, // so reconciliation has to open one of its own for its events to be recorded. - await withDiagnosticsScope( - { command: 'daemon', session: 'daemon', logPath, debug: true }, - async () => { - try { - const summary = await reconcileOrphanedDeviceClaims(reconcile, stateDir); - if (summary.examined > 0) { - emitDiagnostic({ phase: 'device_claim_reconcile', data: summary }); - flushDiagnosticsToSessionFile({ force: true }); - } - } catch (error) { - emitDiagnostic({ - level: 'warn', - phase: 'device_claim_reconcile_failed', - data: { error: error instanceof Error ? error.message : String(error) }, - }); - flushDiagnosticsToSessionFile({ force: true }); + await withDaemonDiagnosticsScope({ logPath }, async () => { + try { + const summary = await reconcileOrphanedDeviceClaims(reconcile, stateDir); + if (summary.examined > 0) { + emitDiagnostic({ phase: 'device_claim_reconcile', data: summary }); } - }, - ); + } catch (error) { + emitDiagnostic({ + level: 'warn', + phase: 'device_claim_reconcile_failed', + data: { error: error instanceof Error ? error.message : String(error) }, + }); + } + }); } export async function cleanupWebBrowserOrphansForDaemonStartup(params: { From 7262f17b67ae9deab6ca295f685b503f9e4696f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 21:32:54 +0200 Subject: [PATCH 2/2] refactor(daemon): route daemon-level diagnostics through one scope helper withDaemonDiagnosticsScope owns the daemon session scope and the forced flush, which now also runs when the body throws, so a failing body keeps its record. It lives at the src root so both daemon-runtime and daemon-registration-owner use it; the eight hand-written copies and the call-site comments that repeated its contract are gone. --- .../daemon-diagnostics-scope.test.ts | 18 +++--------------- .../server => }/daemon-diagnostics-scope.ts | 11 ++--------- src/daemon-registration-owner.ts | 5 +++-- src/daemon/server/daemon-runtime.ts | 9 ++------- 4 files changed, 10 insertions(+), 33 deletions(-) rename src/{daemon/server => }/daemon-diagnostics-scope.test.ts (78%) rename src/{daemon/server => }/daemon-diagnostics-scope.ts (63%) diff --git a/src/daemon/server/daemon-diagnostics-scope.test.ts b/src/daemon-diagnostics-scope.test.ts similarity index 78% rename from src/daemon/server/daemon-diagnostics-scope.test.ts rename to src/daemon-diagnostics-scope.test.ts index 949a34971b..8e1550462d 100644 --- a/src/daemon/server/daemon-diagnostics-scope.test.ts +++ b/src/daemon-diagnostics-scope.test.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import { afterEach, beforeEach, test } from 'vitest'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { withDaemonDiagnosticsScope } from './daemon-diagnostics-scope.ts'; -import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; +import { mkdtempForTestSync } from './__tests__/test-utils/tmp-dir.ts'; let dir: string; let logPath: string; @@ -56,21 +56,9 @@ test('forces the buffered events of a non-debug scope out once the body returns' assert.ok(events.every((event) => event.command === 'daemon-startup')); }); -test('leaves buffered events unwritten when the body throws', async () => { +test('keeps the record of a failed body and still rethrows', async () => { await assert.rejects( - withDaemonDiagnosticsScope({ logPath, debug: false }, () => { - emitDiagnostic({ level: 'warn', phase: 'lost' }); - throw new Error('boom'); - }), - /boom/, - ); - - assert.deepEqual(readLoggedEvents(), []); -}); - -test('flushOnThrow keeps the record of a failed body and still rethrows', async () => { - await assert.rejects( - withDaemonDiagnosticsScope({ logPath, debug: false, flushOnThrow: true }, async () => { + withDaemonDiagnosticsScope({ logPath, debug: false }, async () => { emitDiagnostic({ level: 'warn', phase: 'kept' }); throw new Error('boom'); }), diff --git a/src/daemon/server/daemon-diagnostics-scope.ts b/src/daemon-diagnostics-scope.ts similarity index 63% rename from src/daemon/server/daemon-diagnostics-scope.ts rename to src/daemon-diagnostics-scope.ts index 807ce15e0a..8af5976315 100644 --- a/src/daemon/server/daemon-diagnostics-scope.ts +++ b/src/daemon-diagnostics-scope.ts @@ -7,25 +7,18 @@ export type DaemonDiagnosticsScopeOptions = Readonly<{ logPath: string; command?: 'daemon' | 'daemon-startup'; debug?: boolean; - /** Also force the flush when `body` throws, so the record of a failure is kept. */ - flushOnThrow?: boolean; }>; /** * Runs daemon-level work that has no request scope inside a `daemon` session scope, then forces its - * events to `logPath`. Without a scope `emitDiagnostic` drops every event. + * events to `logPath`, also when the work throws. Without a scope `emitDiagnostic` drops every event. */ export async function withDaemonDiagnosticsScope( options: DaemonDiagnosticsScopeOptions, body: () => Promise | T, ): Promise { - const { logPath, command = 'daemon', debug = true, flushOnThrow = false } = options; + const { logPath, command = 'daemon', debug = true } = options; return await withDiagnosticsScope({ command, session: 'daemon', logPath, debug }, async () => { - if (!flushOnThrow) { - const result = await body(); - flushDiagnosticsToSessionFile({ force: true }); - return result; - } try { return await body(); } finally { diff --git a/src/daemon-registration-owner.ts b/src/daemon-registration-owner.ts index 37155abdaf..8a62bc8199 100644 --- a/src/daemon-registration-owner.ts +++ b/src/daemon-registration-owner.ts @@ -21,7 +21,8 @@ import { type ProcessLockAttempt, type ProcessLockAcquisition, } from '@agent-device/host-kit/file'; -import { emitDiagnostic, withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; +import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; +import { withDaemonDiagnosticsScope } from './daemon-diagnostics-scope.ts'; import type { DaemonCodeOrigin } from '@agent-device/host-kit/code-signature'; import { resolveDaemonPaths, @@ -525,7 +526,7 @@ async function recordRegistrationWarning( phase: string, error: unknown, ): Promise { - await withDiagnosticsScope({ command: 'daemon', session: 'daemon', logPath, debug: true }, () => { + await withDaemonDiagnosticsScope({ logPath }, () => { emitDiagnostic({ level: 'warn', phase, data: { error: normalizeError(error) } }); }); } diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index 20b3f8c93a..d83ea3cca4 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/src/daemon/server/daemon-runtime.ts @@ -39,7 +39,7 @@ import type { DaemonInvokeFn } from '../daemon-request.ts'; import type { SessionRef, SessionState } from '../session-state.ts'; import type { RuntimeHintValues } from '@agent-device/contracts/application-lifecycle-runtime'; import { createDaemonIdleReap } from './daemon-idle-reap.ts'; -import { withDaemonDiagnosticsScope } from './daemon-diagnostics-scope.ts'; +import { withDaemonDiagnosticsScope } from '../../daemon-diagnostics-scope.ts'; import { createSessionIdleExpiry } from './daemon-session-idle-expiry.ts'; import { resolveSessionIdleExpiryMs } from '../session-idle-expiry.ts'; import { finalizeDaemonLeases } from './daemon-lease-finalizer.ts'; @@ -536,10 +536,7 @@ export async function startDaemonRuntime( // sweep waits, never the settle itself, so a stuck recorder cannot make a sweep hang but also // cannot make an expiry give up on a device that does come free. settleBudgetMs: (session) => resolveDaemonSessionTeardownTimeoutMs(session), - // A sweep is out-of-request work, so it has no request scope and `emitDiagnostic` would drop - // everything it reports — including the record of what was reclaimed and why a reclaim failed. - withinDiagnosticsScope: async (run) => - await withDaemonDiagnosticsScope({ logPath, flushOnThrow: true }, run), + withinDiagnosticsScope: async (run) => await withDaemonDiagnosticsScope({ logPath }, run), // An expiry can be the event that makes this daemon fully idle, and no request follows it to // arm the process-level reap. onSessionExpired: () => { @@ -861,8 +858,6 @@ async function reconcileDeviceClaimsForDaemonStartup( reconcile: DeviceClaimReconciler, stateDir: string, ): Promise { - // Startup runs outside any diagnostics scope, where emitDiagnostic is a no-op, - // so reconciliation has to open one of its own for its events to be recorded. await withDaemonDiagnosticsScope({ logPath }, async () => { try { const summary = await reconcileOrphanedDeviceClaims(reconcile, stateDir);