test: surface-wide guard that no command response emits the internal apple platform#1005
Merged
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
…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).
dfb8c85 to
eda2965
Compare
|
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
A surface-wide guard enforcing the Platform-collapse non-breaking contract: NO public command response may emit the internal
appleplatform token on the wire (approach (b) keeps machine output leafios/macos). Three leaks were previously found one-at-a-time (open/perf response, perf-memory support, doctor byPlatform), proving per-site guards are insufficient — so this replaces them with one catalog-driven net.New file:
test/integration/provider-scenarios/apple-platform-output-guard.test.ts(runs in theprovider-integrationvitest project).What the guard drives
platform:'apple',appleOs:'macos') and an iOS-simulator Apple session (platform:'apple', iOS family) — no real hardware, permissive runner/tool/recording provider seams so every command reaches a scannable response.'apple'(case-sensitive exact match — zero false positives oncom.apple.Preferencesbundle ids or theApple TVdevice name).UNSUPPORTED-on-the-wrong-Apple-OS errors (e.g.shutdown/pushon macOS), which are scanned too.ios/macos) in at least one response, so a regression that swapped the leaf back toappletrips the leak scan AND drops the leaf.Auto-covers new commands via the catalog
DRIVEN_COMMANDSandSKIPPED_COMMANDSare keyed byPUBLIC_COMMANDSvalues. A partition test fails if any catalog command is in neither set (or in both), so a newly added command can't silently escape the guard — the author must categorize it.test/replay/batch) just returns a fast, still-scanned error response.Leak caught + fixed
doctordata.platform—src/daemon/handlers/session-doctor.ts:132echoed the raw internalscope.device?.platform('apple') whendoctorran against a bound session with no--platformflag. Fixed by projecting the resolved device throughpublicPlatformString(device); the raw inventory selector fallback (a user-typed--platformvalue) is preserved, so the existingdoctor --platform appletest stays green.This is a leak PR #1004 does not fix — #1004 touches
session-doctor-app/device/options.tsand explicitly leavessession-doctor.tsalone (it reasoned the:111value is "an internal selector, not output"), but it is output viadata.platform. This is exactly the class of miss a surface-wide guard exists to catch. Verified with teeth: reverting the fix makes the guard fail on both worlds withdoctor: value at $.result.data.platform; independently revertingappstate's projection fails withappstate: value at $.result.data.platform.Tracked (not duplicated)
doctorbyPlatform.appleKEY — fixed by PR fix: project doctor output platform to the public leaf after the Platform collapse #1004 (open, not yet on main). Tolerated via one narrow, documented allowlist entry (doctor+kind:'key'+/\.byPlatform\.apple$/), removable once fix: project doctor output platform to the public leaf after the Platform collapse #1004 lands. Not re-fixed here.Out-of-scan observations (documented, not enforced)
The scan is deliberately scoped to exact
applevalue/key positions (the shapes of all three known leaks). Two comma-list token positions surfaced during development and are out of scope, noted here for follow-up:error.data.supportedOn="apple, android"onUNSUPPORTEDerrors — derived from the capability-family bucket name, which was alreadyapplebefore the Platform collapse (commit cd1551b), so it is not a collapse regression and not a device-platform projection.doctorchecks[].command="agent-device apps --platform apple …"— theapps/closesuggestion strings, which PR fix: project doctor output platform to the public leaf after the Platform collapse #1004 already projects to the leaf.Verification (full CI check set, on the final tree)
tsc -p tsconfig.json— cleanoxlint . --deny-warnings— clean (exit 0)oxfmt --write src test+--check— all files correctly formattedlayering/check.ts— OK (679 source files)rslib build— succeedsvitest run --project provider-integration— 85 passed (82 + 3 new)vitest run --project unit— 2963 passedvitest run --coverage— 3048 passed; Statements 84.84% (≥78), Lines 87% (≥80)fallow audit --base origin/main— ✓ no issuesThe android
fillAndroidtimeout test is a known CPU-contention flake — it flaked during one heavy coverage run and passes 92/92 in isolation; a subsequent coverage run was fully green.Human-review only — do not merge.