Skip to content

refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag - #2349

Merged
thymikee merged 4 commits into
mainfrom
claude/2337-commands-rendering-out-of-daemon
Sep 6, 2026
Merged

refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag#2349
thymikee merged 4 commits into
mainfrom
claude/2337-commands-rendering-out-of-daemon

Conversation

@thymikee

@thymikee thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member

Summary

Closes #2337.

src/commands and src/mcp reached into src/daemon for rendering they own — 11 import() sites, 5 file edges. Each moves to its owner:

  • daemon/result-serialization.tscommands/output/result-serialization.ts; the two deployment target resolvers its daemon handlers needed → core/deploy-result-target.ts.
  • daemon/handlers/doctor-output.ts and handlers/status-markers.tssrc/core/, below the daemon handlers, the replay-test reporter and the doctor formatter.
  • daemon/client/doctor-progress.ts (a process-global flag) is deleted, not deprecated. The CLI installs createStderrCommandProgressSink as the request's RequestProgressSink and hands its CommandProgressState to the output formatters; the daemon client only forwards events to the caller's sink.

The MCP edge becomes static. The src/commands edges stay function-scoped: static would take src/cli.ts from 380 to 384 eager modules, which the no-growth budget rejects. No budget raised, no compatibility re-export, no gate or baseline edit needed.

Validation

Tested at e4bc1ea: pnpm install --frozen-lockfile && pnpm build, pnpm typecheck, pnpm lint, pnpm check:affected --run — all runnable checks passed (270 files / 2025 tests, plus daemon-wire-compat).

Graph query over pnpm depgraph output (non-type edges, src/commands|src/mcp → src/daemon): 5 at 27a97ee, 0 at head — query, both outputs and the rename-only stat in the first comment.

pnpm check:layering: OK, 0 R5 back-edges, R2 clean. Eager-closure budgets: 442 pass; all 225 entries unchanged, src/cli.ts 380 → 380.

Doctor CLI output is byte-identical in all three modes (progress streamed, none, custom sink); captures diffed empty. The new CLI-level test was verified failing without the fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ


Generated by Claude Code

…their callers

`formatDoctorCheckSummaryLine`, `formatDoctorCheckDetailLines`, and
`formatCliStatusMarker` are shared by the daemon doctor handlers, the CLI
replay-test reporter, and the doctor CLI output formatter. Owning them in
`src/daemon/handlers/` forced `src/commands` to reach up into `src/daemon`
through a dynamic import; `src/core` sits below every caller.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ
The `serialize*` helpers render command results for the CLI and MCP surfaces,
not for the daemon; only the two deployment target resolvers were shared with
daemon handlers. The serializers move to `src/commands/output/` beside the
formatters that call them, and the resolvers move to `src/core` below both
owners, so `src/commands` and `src/mcp` no longer reach into `src/daemon` for
them. The MCP edge becomes a static import; the `src/commands` edges stay
function-scoped because `src/cli.ts` eagerly evaluates both output modules.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ
…ng the doctor progress flag

Streamed `command` progress was rendered to stderr by the daemon client, which
recorded "doctor progress was rendered" in a process-global flag that
`doctorCliOutput` then consumed to suppress its final summary. The rendering was
commands-side, so it moves there: the CLI installs `createStderrCommandProgressSink`
as the request's `RequestProgressSink` for every non-`test`, non-`--json` command,
and hands its `CommandProgressState` to the output formatters with the result.
The transport now only forwards events to the sink the caller installed.

Behavior is unchanged in the three modes the tests pin: progress streamed to
stderr suppresses doctor's check lines, no progress prints them, and a caller
that installs its own sink (SDK, MCP) writes nothing to this process's stderr
and is not suppressed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ

thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Completion evidence

Zero edges from src/commands / src/mcp into src/daemon

Query over pnpm depgraph output (node ids drop the src/ prefix; kind 1 is type-only, which the condition excludes):

const g = JSON.parse(fs.readFileSync(process.argv[2], 'utf8'));
const KIND = ['value', 'type', 'dynamic'];
const rows = g.edges
  .map(([from, to, kind]) => ({ from: g.nodes[from].id, to: g.nodes[to].id, kind: KIND[kind] }))
  .filter((e) =>
    (e.from.startsWith('commands/') || e.from.startsWith('mcp/')) &&
    e.to.startsWith('daemon/') && e.kind !== 'type');
27a97ee: value+dynamic edges src/commands|src/mcp -> src/daemon: 5
  dynamic  src/commands/capture/output.ts -> src/daemon/result-serialization.ts
  dynamic  src/commands/management/output.ts -> src/daemon/client/doctor-progress.ts
  dynamic  src/commands/management/output.ts -> src/daemon/handlers/doctor-output.ts
  dynamic  src/commands/management/output.ts -> src/daemon/result-serialization.ts
  dynamic  src/mcp/tool-result.ts -> src/daemon/result-serialization.ts

e4bc1ea: value+dynamic edges src/commands|src/mcp -> src/daemon: 0

(5 file edges = the 11 import() sites the issue inventories; the graph dedupes repeats between the same pair.)

Eager-closure budgets

scripts/__tests__/eager-closure-budgets.test.ts: 442 passed. Measured over all 225 entries the gate covers, before vs. after: no entry changed; src/cli.ts stays at 380 modules (daemon/client/doctor-progress.ts leaves the closure, commands/command-progress.ts enters it).

Measured delta for the two src/commands edges kept lazy: making them static takes src/cli.ts 380 → 384 (commands/output/result-serialization.ts, core/doctor-output.ts, core/status-markers.ts, core/deploy-result-target.ts), which the no-growth rule rejects — so the lazy edges stay and no budget was raised. The src/mcp/tool-result.ts edge is under no measured entry's closure, so it became static.

Doctor CLI output, three modes

Both sides drive readDaemonSocketProgressResponse and doctorCliOutput exactly as their own CLI wires them (main: no sink, the client renders and sets the global flag; here: the CLI's createStderrCommandProgressSink plus its state). Captured stderr + text + data, diff empty:

--- mode 1 progress streamed to stderr ---
stderr:
✓ agent-device: agent-device 0.17.9 using /tmp/agent-device
! device: No booted device.
text:
Doctor: pass
No blockers found.
--- mode 2 no progress ---
stderr:
text:
Doctor: pass
✓ agent-device: agent-device 0.17.9 using /tmp/agent-device
! device: No booted device.
  hint: Boot a simulator.
--- mode 3 custom sink ---
stderr:
text:
Doctor: pass
✓ agent-device: agent-device 0.17.9 using /tmp/agent-device
! device: No booted device.
  hint: Boot a simulator.
custom sink events:[{"type":"command",…},{"type":"command",…}]

Mode 1 and 2 also get an end-to-end CLI pin (src/__tests__/cli-network.test.ts), verified failing with the suppression branch disabled; mode 3 is pinned at both seams (command-progress.test.ts, management/output.test.ts).

Rename-only proof

git diff -M90% --stat origin/main...HEAD — 30 files, 407 insertions, 231 deletions:

 src/{daemon/handlers => core}/doctor-output.ts     |   0
 src/{daemon/handlers => core}/status-markers.ts    |   0
 src/{daemon => commands/output}/result-serialization.ts      |  21 +---
 src/{daemon/__tests__ => commands/output}/snapshot-serialization.test.ts |   2 +-

The two pure moves are content-identical. result-serialization.ts loses only the two resolver functions extracted to core/deploy-result-target.ts; its test file splits the same way (the 132-line delete / 102-line add pair), so both halves carry their existing assertions unchanged.


Generated by Claude Code

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.49 MB 4.49 MB -160 B
Package (unpacked) 4.49 MB 4.49 MB -160 B
Package (download) 1.33 MB 1.33 MB -7 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.4 ms 25.0 ms -0.3 ms
CLI --help 73.4 ms 73.4 ms -0.0 ms

The end-to-end doctor progress cases landed in `cli-network.test.ts`, which is
already past the 1,000-line tripwire, so the test-file-size ratchet rejected the
growth. They are doctor-output cases rather than network ones: they move to
`cli-doctor-progress.test.ts`, and `cli-network.test.ts` returns to its
merge-base size.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eddu5VC45qAXUkXNZJyruZ
@thymikee

thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Clean review at 29f3527. The shared helpers and serializers have clear owners, and the doctor progress state is local to each CLI run. Streamed output avoids duplicates; custom sinks still get the full result. The tests cover both streamed and unstreamed CLI output, and the old global flag is gone.

Ready for human review. Remaining CI is still running, so this is not yet a merge-ready claim.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 6, 2026
@thymikee
thymikee merged commit 6768a04 into main Sep 6, 2026
18 checks passed
@thymikee
thymikee deleted the claude/2337-commands-rendering-out-of-daemon branch September 6, 2026 09:30
@github-actions

github-actions Bot commented Sep 6, 2026

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

thymikee added a commit that referenced this pull request Sep 6, 2026
…line

* origin/main:
  fix(ios): stop charging every capture for a slow Simulator app discovery (#2331)
  refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag (#2349)

# Conflicts:
#	CHANGELOG.md
thymikee added a commit that referenced this pull request Sep 6, 2026
…tation-evidence

* origin/main:
  fix(ios): stop charging every capture for a slow Simulator app discovery (#2331)
  refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag (#2349)
thymikee pushed a commit that referenced this pull request Sep 6, 2026
Takes #2347's move of the daemon code-signature walker into
`@agent-device/host-kit` together with its workspace-specifier resolution,
and drops this branch's own copy of that fix: one resolver, the one on
main. The claim this branch owed keeps its own test beside the walker —
the daemon source graph stamps `packages/command-registry/src/registry.ts`,
`catalog.ts` and the manifest, measured at 1,461 modules, and touching the
registry changes the signature.

`src/cli/commands/router-types.ts`: #2349's `CommandProgressState` import
alongside this branch's catalog specifier. A fixture string in the
relocated CLI-route test also goes back to the specifier it illustrates;
the move rewrote it as if it were a real import.

Three review points, recorded here because their commits are pushed:

- Dropping `registry.ts` and `platform-execution-entry.ts` from
  `HUB_ENTRY_FILES` is not only bookkeeping: the package manifest now
  publishes them, so the gate discovers them as domain facades and
  `denyPlatformImplementations` flips false to true. Both now carry the
  ADR-0019 assertion that they evaluate no concrete platform
  implementation, and both pass it. That strengthening is intended.
- The 150 this branch reports against the issue's 154 is a filter
  difference, not a discrepancy: `pnpm depgraph` collapses to one edge per
  file pair, and 150 counts value edges only. Counting type-only and
  dynamic edges on the same graph gives 174 into 67 root files, which is
  the shape the issue's own per-file breakdown uses; both readings drop by
  exactly the 36 the issue predicts.
- The six tests that stay in root do so for one root symbol each:
  parity needs STRUCTURED_BATCH_COMMAND_NAMES, DAEMON_COMMAND_DESCRIPTORS,
  canRunReplayScopedAction and DaemonRequest; ref-frame-effect needs
  DAEMON_COMMAND_DESCRIPTORS, resolveRefFrameEffect and DaemonRequest;
  timeout-policy needs DEFAULT_STABLE_TIMEOUT_MS; platform-execution-cli-route
  needs CLI_INJECTED_DAEMON_DISPATCHES and CliInjectedRoute;
  post-action-observation needs AgentDeviceClient, getCliCommandSchema,
  readInputFromCli, findCommandMetadata, the two command-family registry
  listers, SettleCapableClientOptionCommands, buildActionDetails and
  COMMAND_OUTPUT_SCHEMAS; shutdown-runtime-execution imports nothing but
  reads the daemon and root host sources whose text it pins.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016DkNZCMWtm1KUbseKANADi
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.

refactor(commands): move commands-side rendering out of src/daemon and retire the doctor progress flag

2 participants