fix: project doctor output platform to the public leaf after the Platform collapse#1004
Merged
Conversation
…apse
The Platform collapse projected most `doctor` output but missed the device
inventory breakdown: `deviceInventoryEvidence` keyed `byPlatform` by the raw
internal `device.platform`, so an Apple session emitted `byPlatform: { apple: … }`
in the doctor response — a machine-facing `apple` leak (approach b keeps output
leaf `ios`/`macos`). Key it by `publicPlatformString(device)` instead (now
`Map<PublicPlatform, …>`, so an `apple` key is compile-impossible), which also
splits Apple devices into ios vs macos in the breakdown.
Also project two doctor command SUGGESTION strings (`apps --platform …`,
`close --platform …`) to the leaf — `--platform apple` was valid but broader than
the session's actual device.
The doctor descriptor routing (command-descriptor `daemon` block) and its lack of
a platform-specific `supports` gate are already correct; toolchain checks already
accept `apple`. Verified: tsc, oxlint, oxfmt, layering, fallow, provider-integration
82/82; full unit green except the known android fillAndroid contention flake
(passes 92/92 in isolation).
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
Member
Author
|
Review pass: this fixes the remaining doctor-facing platform leak I found after #1002. The emitted inventory breakdown now keys by PublicPlatform via publicPlatformString(device), and the doctor command suggestions now use the public leaf as well. I spot-checked the remaining raw device.platform/session.device.platform references in the doctor handlers: they are internal selector/toolchain/session comparisons, not emitted machine output. Checks are green (21/21). This looks ready for human maintainer judgment. |
|
This was referenced Jul 1, 2026
thymikee
added a commit
that referenced
this pull request
Jul 1, 2026
…e platform Adds a provider-integration guard (apple-platform-output-guard.test.ts) that stands up a fake-provider daemon for BOTH a macOS Apple session and an iOS-simulator Apple session, drives EVERY public command off PUBLIC_COMMANDS, and deep-scans each serialized response for the internal 'apple' platform token (any string VALUE or object KEY that exactly equals 'apple'). The guard is catalog-driven: a partition test fails if a new public command is neither in DRIVEN_COMMANDS nor SKIPPED_COMMANDS, so a new command can't silently escape the check. All 50 public commands are driven; the skip-set is empty. Caught + fixed a leak PR #1004 misses: doctor's data.platform (session-doctor.ts) echoed the raw internal device.platform ('apple') when doctor ran against a bound session with no --platform flag. Now projected via publicPlatformString(device). doctor's byPlatform.apple KEY leak stays tracked to PR #1004 via a narrow, documented allowlist entry (not duplicated here).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the Platform collapse (#1002) — you were right to double-check
doctor(added recently in #883, concurrent with the refactor). It was mostly converted but had one real output leak the collapse's guard didn't cover.The gap (fixed)
session-doctor-device.tsdeviceInventoryEvidencebuilt thebyPlatformbreakdown keyed by the raw internaldevice.platform, so an Apple session'sdoctordevice-inventory check emittedbyPlatform: { apple: {...} }— a machine-facingappleleak, contrary to approach (b) (output stays leafios/macos).publicPlatformString(device)→Map<PublicPlatform, …>, so anapplekey is compile-impossible, and Apple devices split intoiosvsmacosin the breakdown (more useful).doctorcommand-suggestion strings (apps --platform …,close --platform …) to the leaf —--platform applewas valid but broader than the session's actual device.What was already correct (verified, not changed)
doctoris in the command catalog + command-descriptor registry; it routes via the descriptordaemonblock (route:'session',sessionKind:'inventory') — nodaemon-command-registryentry needed.supports/capability gate (doctor is platform-agnostic) → correctly absent from the ApplesupportsByDefaultmap.appendToolchainChecksalready handles'apple'(sosession-doctor.tstoolchain routing works);:111platform is an internal device-inventory selector, not output.Verification
tsc · oxlint --deny-warnings · oxfmt --check · layering guard · fallow (no new findings) · provider-integration 82/82; full unit green except the known android
fillAndroidcontention flake (passes 92/92 in isolation). The fix is type-guaranteed (leaf keys) andpublicPlatformStringis already parity-tested.