Skip to content

fix: project doctor output platform to the public leaf after the Platform collapse#1004

Merged
thymikee merged 1 commit into
mainfrom
fix-doctor-platform-leak
Jul 1, 2026
Merged

fix: project doctor output platform to the public leaf after the Platform collapse#1004
thymikee merged 1 commit into
mainfrom
fix-doctor-platform-leak

Conversation

@thymikee

@thymikee thymikee commented Jul 1, 2026

Copy link
Copy Markdown
Member

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.ts deviceInventoryEvidence built the byPlatform breakdown keyed by the raw internal device.platform, so an Apple session's doctor device-inventory check emitted byPlatform: { apple: {...} } — a machine-facing apple leak, contrary to approach (b) (output stays leaf ios/macos).

  • Now keyed by publicPlatformString(device)Map<PublicPlatform, …>, so an apple key is compile-impossible, and Apple devices split into ios vs macos in the breakdown (more useful).
  • Also projected two doctor command-suggestion strings (apps --platform …, close --platform …) to the leaf — --platform apple was valid but broader than the session's actual device.

What was already correct (verified, not changed)

  • doctor is in the command catalog + command-descriptor registry; it routes via the descriptor daemon block (route:'session', sessionKind:'inventory') — no daemon-command-registry entry needed.
  • No platform-specific supports/capability gate (doctor is platform-agnostic) → correctly absent from the Apple supportsByDefault map.
  • appendToolchainChecks already handles 'apple' (so session-doctor.ts toolchain routing works); :111 platform 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 fillAndroid contention flake (passes 92/92 in isolation). The fix is type-guaranteed (leaf keys) and publicPlatformString is already parity-tested.

…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).
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.5 MB 1.5 MB -20 B
JS gzip 477.9 kB 477.9 kB 0 B
npm tarball 578.7 kB 578.7 kB -1 B
npm unpacked 2.0 MB 2.0 MB -20 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.2 ms 27.1 ms -1.0 ms
CLI --help 49.2 ms 50.3 ms +1.1 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/session.js -20 B 0 B

@thymikee

thymikee commented Jul 1, 2026

Copy link
Copy Markdown
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.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 1, 2026
@thymikee thymikee merged commit fa76a91 into main Jul 1, 2026
21 checks passed
@thymikee thymikee deleted the fix-doctor-platform-leak branch July 1, 2026 18:47
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-01 18:47 UTC

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