Skip to content

refactor(daemon): own the boot/shutdown device-status response in one place - #3557

Merged
thymikee merged 3 commits into
mainfrom
refactor/daemon-session-handler-cleanup
Oct 13, 2026
Merged

thymikee merged 3 commits into
mainfrom
refactor/daemon-session-handler-cleanup

Conversation

@thymikee

@thymikee thymikee commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Summary

No behavior change. Cleans up three leftovers from the session-state split (#3455–#3458, #3472):

  • packages/contracts/src/device.ts: the BootCommandResult and ShutdownCommandResult doc comments cited the retired session-appstate.ts; each now names its owning handler and the shared builder that owns the literal (session-boot.ts / session-shutdown.ts via session-device-status-response.ts).
  • The bootedDeviceResponse builders duplicated in session-boot.ts and session-shutdown.ts become one owner, deviceStatusReportResponse in session-device-status-response.ts — it answers "what does this device report on the wire after a readiness command": public identity fields plus the command's status field (booted / shutdown) and the gated additive appleOs. Wire key order is unchanged and pinned by test.
  • The boot, shutdown, and appstate pass-through wrappers in session.ts are deleted; the dispatch table maps straight to the handlers, whose RuntimeCommandHandlerParams accept SessionCommandParams directly.

New module claimed by daemon-sessions in the daemon layer manifest (separate chore(gates) commit). 7 files, +112/−97 gross.

Validation

Tested at head 020bb45: pnpm exec tsc -b clean; pnpm check:affected --run passed on the head under the gate lock; pnpm exec vitest run scripts/__tests__/eager-closure-budgets.test.ts passed (838 tests); pnpm check:coverage-crap --base origin/main over a unit-core coverage run reports no touched function at or above 30. New session-device-status-response.test.ts is revert-sensitive: removing the platform gate turns the stray-appleOs negative test red, and swapping the status/appleOs spreads turns the pinned Object.keys wire order red — both observed, green again after restore. Boot/shutdown/appstate wire-shape tests pass unchanged. Cold-context review ran; its one finding (shared device fixtures) was adopted. Cubic P3s both fixed at 020bb45. Integration/Coverage lanes stay GitHub-owned.

… place

- Point the BootCommandResult and ShutdownCommandResult doc comments in
  packages/contracts/src/device.ts at their owning handlers (session-boot.ts,
  session-shutdown.ts) instead of the retired session-appstate.ts.
- Replace the duplicated bootedDeviceResponse wire-shape builders in the boot
  and shutdown handlers with one deviceStatusReportResponse owner.
- Map the boot, shutdown, and appstate dispatch-table entries in session.ts
  straight to their handlers; the pass-through wrappers only copied fields the
  handlers already accept.

Follow-ups from #3455, #3472. No behavior change.

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

All reported issues were addressed across 7 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/contracts/src/device.ts Outdated
Comment thread src/daemon/handlers/__tests__/session-device-status-response.test.ts Outdated
…d builder

Review follow-up (#3557): the contracts doc comments now cite the shared
deviceStatusReportResponse the literals moved into, and the boot-shape test
pins Object.keys so a wire key-order change is a test failure.
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.17 MB 5.17 MB -612 B
Package (unpacked) 5.17 MB 5.17 MB -612 B
Package (download) 1.55 MB 1.55 MB -23 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 21.5 ms 21.8 ms +0.3 ms
CLI --help 62.2 ms 59.8 ms -2.5 ms

@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready at 020bb45. The refactor looks behavior-neutral: the boot and shutdown response now comes from one builder, and the wire shape matches the old one when I compare the code. All 19 checks pass, and there are no conflicts. Nothing else needs to happen before merge.

Not blocking: the key-order test returns early on !response.ok || !response.data, which would skip the check if the builder ever returned an error shape, so an expect(response.ok).toBe(true) first would close that gap. The comments in packages/contracts/src/device.ts still say "fixed object literal", and "the shape built by" would now be more accurate. The pass-through wrapper removal in session.ts is disclosed in the PR body and needs nothing. Take or leave any of these.

I did not run the tests or your mutation checks, so I judged revert-sensitivity from the diff only. I did not run a live boot or shutdown on a device, and I did not run tsc myself. The wrapper removal relies on the green CI for type compatibility.

No open inline threads still apply. Both Cubic threads are fixed at this head, so please resolve them. The first is the comment wording, and the "literal" leftover is trivial: #3557 (comment). The second is the key-order assertion, which is now in place at session-device-status-response.test.ts:32-40: #3557 (comment)

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 13, 2026
@thymikee
thymikee merged commit 65013e3 into main Oct 13, 2026
19 checks passed
@thymikee
thymikee deleted the refactor/daemon-session-handler-cleanup branch October 13, 2026 05:45
@github-actions

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

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