Skip to content

refactor(daemon): route daemon-level diagnostics through one scope helper - #3242

Merged
thymikee merged 2 commits into
mainfrom
claude/daemon-diagnostics-scope-helper
Oct 6, 2026
Merged

thymikee merged 2 commits into
mainfrom
claude/daemon-diagnostics-scope-helper

Conversation

@thymikee

@thymikee thymikee commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Summary

Daemon-level work that runs outside a request needs its own diagnostics scope, or emitDiagnostic drops its events. That wrapper (withDiagnosticsScope({ session: 'daemon', logPath, … }) plus a forced flush) was written out eight times: seven in daemon-runtime.ts and one in daemon-registration-owner.ts. withDaemonDiagnosticsScope now owns it, and every site uses it.

  • Each site keeps its command name and debug flag. The flush now also runs when the body throws, so a failing body keeps its record. Before, only the idle-expiry sweep did that; the startup-diagnostics flush would have lost its events.
  • The helper lives at src/daemon-diagnostics-scope.ts. daemon-runtime.ts already imports the registration module, so a root module avoids reversing that dependency.
  • The two call-site comments that repeated the helper's contract are gone. daemon-runtime.ts shrinks by about 50 lines.

4 files, 242 gross lines.

Validation

At 7262f17b67: pnpm check:affected --run passes. The helper test covers the scope fields, the default and startup command names, and a throwing body that keeps its events and rethrows. Log output is otherwise unchanged: in a debug scope with a log path every event is written as it is emitted, so the extra flushes write nothing.

Review in cubic

…elper

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.
…lper

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Re-trigger cubic

@thymikee

thymikee commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

This PR is ready at 7262f17. The daemon diagnostics refactor reads clean, and no conflicts are known.

CI has not shown a result on this head, because all 9 non-passing jobs were cancelled and left no logs. The diff touches daemon startup and shutdown paths that Typecheck & Package, Lint & Format, Coverage and the Smoke Tests exercise, so please re-run CI on this head before merge. I did not run the typecheck, the new test, or the layering and fallow gates, so whether the src/daemon/server import from the src/ root is allowed is still unchecked. No live daemon run is needed for this internal change.

Not blocking: the new test sits beside its source in src/, while the registration-owner test lives in src/tests/, and no test goes through the daemon-runtime call sites, including the flush-on-throw path of the sweep's run. You can take or leave both.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 6, 2026
@thymikee
thymikee merged commit 5aff6bb into main Oct 6, 2026
10 of 19 checks passed
@thymikee
thymikee deleted the claude/daemon-diagnostics-scope-helper branch October 6, 2026 05:32
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-06 05:33 UTC

@thymikee

thymikee commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

At 7262f17b67, pnpm check:layering and pnpm check:fallow both pass, so the src/daemon/server → src/ root import is allowed. The cancelled CI jobs are re-running.

I'm leaving the two optional notes as they are:

  • The test sits next to its source, as other src/ root modules do (for example src/backend-snapshot-options.test.ts). src/__tests__/ is the older layout.
  • The idle sweep's run is the helper itself, and the helper test covers a body that throws. A test through daemon-runtime would add little.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.07 MB 5.07 MB -266 B
Package (unpacked) 5.07 MB 5.07 MB -266 B
Package (download) 1.52 MB 1.52 MB +4 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 24.6 ms 24.5 ms -0.1 ms
CLI --help 71.3 ms 74.6 ms +3.3 ms

thymikee added a commit to okwasniewski/agent-device that referenced this pull request Oct 6, 2026
* origin/main: (77 commits)
  fix(apple-runner): fence prep spawns behind a start-owned admission (callstack#3239)
  0.21.22
  test(apple): own the simctl settings plan tests in simctl-settings.test.ts (callstack#3244)
  fix(limrun): report the session device id in iOS settings refusals (callstack#3243)
  0.21.21
  feat(remote): add a host-allocated macos-app lease backend (callstack#3236)
  test(android): bound the screenshot write wait by wall time, not event-loop turns (callstack#3250)
  feat(recording): cap the touch overlay frame rate at the caller's --fps (callstack#3241)
  fix(ad-script): let .ad scripts carry scroll --until and wait capture flags (callstack#3197) (callstack#3234)
  feat(provider-webdriver): keyboard enter, dismiss, and status over WebDriver (callstack#3233)
  feat(selectors): match role= against snapshot kind with a node-scoped alias window (callstack#3232)
  fix(provider-webdriver): read field values, placeholders, secure fields, and checked state from page source (callstack#3231)
  feat(replay): accept --test-ime on test and replay so flow-owned Android opens opt into the test IME (callstack#3235)
  refactor(daemon): route daemon-level diagnostics through one scope helper (callstack#3242)
  docs(adr): correct ADR 0031 pointer event delivery evidence (callstack#3245)
  fix(ios): stop a tap's post-gesture lookup from recording an XCTest failure (callstack#3060) (callstack#3237)
  fix(recording): render the touch overlay at most 30 fps and inside the record request (callstack#3219)
  fix(daemon): keep an idle daemon alive only for retained leases (callstack#3227)
  fix(provider-webdriver): send an empty JSON object on bodyless POSTs (callstack#3230)
  Feat/maestro repeat while (callstack#3214)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant