Skip to content

refactor(daemon): thread the platform-services port through request execution - #3403

Merged
thymikee merged 2 commits into
mainfrom
pr-a-platform-services
Oct 11, 2026
Merged

thymikee merged 2 commits into
mainfrom
pr-a-platform-services

Conversation

@thymikee

@thymikee thymikee commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Summary

Threads the daemon's host-facing asks that bind no device runtime — local readiness, boot-time and runner-session observation, open-target classification — across the daemon boundary as one required DaemonPlatformServices dependency (src/daemon/platform-services.ts), composed by the root in src/platform-runtime-daemon-services.ts and passed root → RequestRouterDeps → request scope → handlers/consumers. Replaces the daemon's value imports of platform-runtime-apple-resources, -open-target, -device-ready, and -device-boot. No fail-closed placeholder exists: an un-composed process cannot build the router (AGENTS.md forbids the fallback). Every member is an already-published neutral contract type; device execution never enters the port (ADR 0019 §11). Tests use the shared fixture (__tests__/platform-services-fixture.ts).

192 files, mostly mechanical one-field threading; gross diff ~1.6k lines exceeds the 1k budget — flagged here per the plan; the split point (claim-recovery composition) is stack PR #2.

Validation

Commits 6b73f3fb5 + 60f5e9d78. Local at tip: pnpm build, check:quick, check:layering (R76 records the single remaining edge), check:di-seams, check:affected --run all pass; eager-closure and test-size-ratchet gates pass; test:integration:provider 213/213; test:concurrency-torture 4/4. pnpm depgraph: the only daemon → platform-runtime value edges left are daemon-runtime.ts (composition) and the pre-existing classified direct-ios-selector.ts edge (unchanged; no raw-selector escape ported).

Live device evidence (this stack, iPhone 18 Pro sim + ReactNative_API_35 emulator): iOS open(Safari URL) + snapshot -i + selector click (text=refresh) OK; Android open + readiness path OK with androidSnapshot.backend=android-helper, helperVersion=0.21.25-dev matching package.json.

View guided diff Turn on auto-fix

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.16 MB 5.16 MB +5.3 kB
Package (unpacked) 5.16 MB 5.16 MB +5.3 kB
Package (download) 1.55 MB 1.55 MB +817 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.8 ms 26.2 ms +0.4 ms
CLI --help 82.0 ms 82.5 ms +0.4 ms

@thymikee

Copy link
Copy Markdown
Member Author

The code in 60f5e9d looks right to me. Required live evidence (iOS open, snapshot -i and selector click; Android open and readiness) is in the PR body as your report. I did not inspect those artifacts, and I did not run typecheck, check:layering or the test suites. All 18 checks pass at this commit, and I know of no conflicts. The claim-reconciliation boot observation and the recording-health refresh were not run live. They are 1:1 member swaps, so I read the diff for them. I did not measure startup cost of the new dynamic import in startDaemonRuntime, and the Size Report shows no material change.

Not blocking, and you can take or leave these: four tests (request-recording-health.test.ts:12, request-router-recording-health.test.ts:7, session-device-resolution.test.ts:17, ios-app-session-hint.test.ts:7) still vi.mock platform-runtime-apple-resources.ts, while ADR 0019 section 11 says tests pass the shared fixture instead, so either pass daemonPlatformServicesFixture({ appleSessionObservation: stub }) or soften that ADR sentence. The ADR line "every member is an already-published contract type" is also wrong for the three open-target signatures declared inline in platform-services.ts. The new JSDoc for platformServices in request-execution-scope.ts sits above dispatchLedger's doc instead of the platformServices field, and platform-runtime-daemon-services.ts:12 cites "ADR 0019 section 1" where it means section 11.

Most of the +493 is one more parallel field beside inspectFacts, bindDevice and bindExactDevice in about 40 handler param types. RuntimeAdmissionBindings already names that tuple, so could handlers take one bindings: RuntimeAdmissionBindings from the locked scope instead of re-forwarding each member? Then the next request-scoped capability would add one field in one place. I found no smaller owner for the port itself. If you think that migration belongs in a separate change, a short note on why is enough.

The PR is still a draft. Mark it ready for review, optionally after the two notes above.

@thymikee
thymikee force-pushed the pr-a-platform-services branch from 60f5e9d to 029223a Compare October 11, 2026 07:01
@thymikee
thymikee marked this pull request as ready for review October 11, 2026 07:02
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto latest main (#3393/#3373/#3385/#3390/#3351/#3400, clean) and addressed all nits at 029223a:

  • Test mocks: all four tests now pass daemonPlatformServicesFixture with an appleSessionObservation override through the port instead of vi.mock-ing platform-runtime-apple-resources.ts (ADR 0019 §11). The three port-backed tests override only observeRunnerSession; the hint test overrides resolveSoleForegroundApp.
  • ADR 0019 §11: reworded — the two observation members are published contract types; readiness and the three open-target members are inline signatures taking/returning only neutral types. The port JSDoc made the same claim and was aligned.
  • request-execution-scope.ts: the platformServices JSDoc moved onto its own field, above dispatchLedger's doc again.
  • platform-runtime-daemon-services.ts: citation corrected to section 11.
  • bindings: RuntimeAdmissionBindings: agreed it's the right end state — it would collapse the parallel field beside inspectFacts/bindDevice/bindExactDevice in ~40 handler param types to one field in one place. Deliberately not in this PR: it's a mechanical migration across every handler signature and would materially grow a port-introduction PR. Filed as a follow-up.
  • chore(gates): daemon layer manifest + R81 daemon-layer rule (no moves) #3391 (daemon-layer manifest, R81) is still open, so no manifest entry to add; check:layering is green.

Validation at 029223a: check:quick, check:layering, check:di-seams, test-file-size ratchet, eager-closure budgets, and check:affected --run all pass. test:integration:provider has one failure (settle-observation: press --settle, 5s timeout) that reproduces identically on plain origin/main — pre-existing, not from this branch.

@thymikee

Copy link
Copy Markdown
Member Author

Addressed the notes:

  • The four tests now stub observation through daemonPlatformServicesFixture({ appleSessionObservation: stub }) instead of vi.mock-ing platform-runtime-apple-resources.ts — the ADR sentence and the tests now agree, no softening needed.
  • ADR §11 corrected: only the observation members are contracts-package types; the three open-target members are declared at the port over neutral inputs, and the sentence says that.
  • The misplaced platformServices JSDoc in request-execution-scope.ts moved onto its field; platform-runtime-daemon-services.ts now cites section 11.

On RuntimeAdmissionBindings: agreed that's the right end state, and it's deliberately not here. That tuple is owned by runtime-fact admission (request-runtime-binding.ts shapes), and folding the port into it would either make admission own a non-admission capability or force a new merged type across ~190 call sites in this PR — pushing the diff well past the budget on top of the port's own mechanical threading. Happy to take it as a follow-up (bindings object from the locked scope, one field per new request-scoped capability).

Rebased onto main; full gate rerun at the new head: build, check:quick, check:layering, check:di-seams, check:affected --run, eager-closure + size-ratchet, provider 213, torture 4/4, plus the four re-pointed test files. Marking ready for review.

@thymikee
thymikee force-pushed the pr-a-platform-services branch from 029223a to e53be1d Compare October 11, 2026 07:02
@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready at e53be1d. The earlier ADR wording is now fixed, and the author's answer on the bindings question is fine as a filed follow-up.

Not blocking: the JSDoc at https://github.com/callstack/agent-device/blob/e53be1d/src/daemon/platform-services.ts#L15 still says every member is an already-published neutral contract, but the three open-target members and ensureLocalDeviceReady are declared inline in the port. Rewording it to match the corrected ADR sentence would fix it: observation members are contracts types, and readiness and open-target are port-declared signatures over neutral types. You can take this or leave it.

Could handlers take one bindings object from the locked scope instead of forwarding platformServices next to inspectFacts, bindDevice and bindExactDevice? You answered that runtime-fact admission owns that tuple and filed a follow-up. That is fine, and this delta adds no new seam.

The Android Smoke Tests job failed because live-assertions.ts:109 expected helperTransport persistent-session and got instrumentation. This PR does not touch packages/platform-android, and the same assertion failed on the unrelated branch run 38062260602. I did not look into why the helper session fell back in either run, so this rests on no diff overlap and the matching failure elsewhere. The same run shows Android open, bootstrap and inventory passing, and a snapshot from android-helper with 268 nodes. I did not inspect the iOS device artifacts in the PR body, and I did not run typecheck, layering, the vitest suites or provider integration.

Please rerun the Android Smoke Tests job. The other checks were still queued or in progress when I looked, so they need to finish before merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 11, 2026
…xecution

Compose the daemon’s host-facing asks (local readiness, boot and runner-session observation, open-target classification) as one required DaemonPlatformServices dependency at the process root and thread it root -> RequestRouterDeps -> request scope -> handlers/consumers. Replaces the daemon value imports of platform-runtime-apple-resources/-open-target/-device-ready/-device-boot. No fail-closed placeholder: an un-composed process cannot build the router.
R76 inventory records the one daemon -> platform-runtime value edge this port keeps (the root composition factory in daemon-runtime), re-points the planted-violation fixtures at it, and the root-module zone list gains the new composition module.
@thymikee
thymikee force-pushed the pr-a-platform-services branch from e53be1d to 5e85b14 Compare October 11, 2026 08:21
@thymikee

Copy link
Copy Markdown
Member Author

Took the JSDoc nit: src/daemon/platform-services.ts now says the two observation members are published contracts types, and that readiness and open-target classification are signatures declared on the port over neutral types. This matches ADR 0019 §11. It's folded into the refactor commit (head 5e85b14, comment-only, typecheck clean). #3404 is restacked onto it with a tree identical to the reviewed cda2166 apart from that comment (head 7584bf2). The Android smoke failure (helperTransport instrumentation vs persistent-session) also passed on #3404's run at cda2166, which contains this PR's code. The new push re-runs it.

@thymikee
thymikee added this pull request to stack #3410 October 11, 2026 08:26
@thymikee
thymikee merged commit 4135c0b into main Oct 11, 2026
19 checks passed
@thymikee
thymikee deleted the pr-a-platform-services branch October 11, 2026 08:27
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-11 08:28 UTC

thymikee added a commit that referenced this pull request Oct 11, 2026
#3391's manifest was cut before #3403 and #3404 landed, so
src/daemon/platform-services.ts (imported by daemon-core files, so core)
and src/daemon/device/claim-recovery-gateway.ts (imported only from
daemon-server, beside its device-claim siblings in resources) were the
only files R81 found unowned. This PR re-ran check:layering after
rebasing over them and assigns both.

Co-authored-by: Copilot <2235562194+Copilot[bot]@users.noreply.github.com>
thymikee added a commit that referenced this pull request Oct 11, 2026
…fest (#3412)

#3391 added the daemon layer manifest while #3403 and #3404 were in flight,
and each merged green on its own: src/daemon/platform-services.ts and
src/daemon/device/claim-recovery-gateway.ts reached main in no layer, so R81
fails on main and every PR's Repo Guards. platform-services.ts fits only
daemon-core; the claim-recovery port fits any layer and goes in
daemon-resources beside the device claims, matching the assignment #3395
already carries.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
thymikee added a commit that referenced this pull request Oct 11, 2026
…n-contracts (#3395)

* refactor(move): extract the daemon-contracts zone into packages/daemon-contracts

The ten files of the daemon-contracts zone (the nine root modules declared by
ROOT_MODULE_ZONES plus src/daemon-contracts/daemon-diagnostics-scope.ts) move
behind a workspace-package boundary with narrow per-file subpath exports. The
shared daemon/client placement from #2559 and the #3288 zone declaration
become physical: R11 and the package manifest hold the boundary where the
per-file rows held it before. Consumers switch from relative specifiers to
@agent-device/daemon-contracts/<file>; the tests move with their sources.
Behavior is unchanged.

* chore(gates): retarget daemon-contracts path-keyed enforcement at the package

ROOT_MODULE_ZONES sheds its daemon-contracts rows (the package derives the
zone), the R78 guidance names the package as the shared-contract home, the
wire-compat manifest/ledger/mutation keys move with request-progress-protocol,
the mutation-lane exclusion pins the moved test path, and ADR 0033 records the
extraction with its rationale.

* test(daemon-contracts): own the daemon-registration fixture and export it as a subpath

publishDaemonRegistration was duplicated verbatim between
packages/daemon-contracts/src/daemon-registration.fixtures.ts and
src/__tests__/test-utils/device-claim-store.ts. Keep the package as the
single owner: export the fixture via ./daemon-registration-fixtures the
way capture-kit exports ./recording-artifact-fixtures, delete the root
copy, and point the six root tests at the subpath.

Co-authored-by: Copilot <2235562194+Copilot[bot]@users.noreply.github.com>

* docs(layering): name packages/daemon-contracts/ in the root-zone migration note

The example repeated src/command-runtime/ after the daemon-contracts
zone moved out of src/daemon-contracts/; name the package instead.

Co-authored-by: Copilot <2235562194+Copilot[bot]@users.noreply.github.com>

* fix(layering): place the two port files R81 found unlayered

#3391's manifest was cut before #3403 and #3404 landed, so
src/daemon/platform-services.ts (imported by daemon-core files, so core)
and src/daemon/device/claim-recovery-gateway.ts (imported only from
daemon-server, beside its device-claim siblings in resources) were the
only files R81 found unowned. This PR re-ran check:layering after
rebasing over them and assigns both.

Co-authored-by: Copilot <2235562194+Copilot[bot]@users.noreply.github.com>

---------

Co-authored-by: Copilot <2235562194+Copilot[bot]@users.noreply.github.com>
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