refactor(ios): brand the simctl builders and require the brand where runXcrun runs simctl - #2917
Conversation
buildSimctlArgsForAddress and buildSimctlArgsForDevice now return a ScopedSimctlCommand, minted in core/tool-provider.ts from set-scoped arguments. runXcrun takes that command or an argv whose tool is a named non-simctl tool (devicectl, xcdevice, xctrace), so a literal, const-held, string-typed or copied simctl argv is a type error. The simctl branch unwraps the command to its scoped arguments; the ScopedSimctlArgs double cast is gone. The Apple tool host hands a simctl request's scoped args straight to the simctl provider. The set-scope primitive is private to core/simctl.ts; doctor and inventory use the named mints simctlAvailabilityProbeArgs and simctlListDevicesArgs. Every argv stays byte-identical. Refs #2878
Drop the '--set' literal ban, the global ArrayExpression clause, the set-scope identifier check and the tool-provider argv exemption. R79 now checks the argv at an xcrun invocation (a call with an 'xcrun' argument or an executable: 'xcrun' spec), which must name its tool as a literal other than simctl, and refuses casts to the scope brands outside the two modules that mint them. Refs #2878
…ver it reaches an executor runCmd, runCmdBackground and an executable: 'xcrun' spec take any string argv, so the runXcrun brand does not reach them. R79 again flags every array whose first element names simctl (literal, quasi-only template, or a same-file binding of either) outside core/simctl.ts and core/tool-provider.ts, so a variable-held, aliased or spread simctl argv fails the gate as the inline form does.
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at 064fa5f. The brand looks right: every simctl route I traced goes through the mint points, and the argv output does not change. I read the One design question before this is ready: could Not blocking: the rule in apple-simulator-scope-policy.ts#L62 checks only the first array element, so a variable tool name or xcrun options before the tool could pass. Also, The Coverage failure is the Android |
|
Follow-up to the 064fa5f review, pushed as Can R79 checks only the first element. Partly fixed in
Coverage. Your diagnosis is correct: the Android Validation: see the updated PR body. Local |
|
This is ready at 607e6b8; the brand routing from the earlier review (064fa5f) still holds since this delta doesn't touch it. Not blocking: namesNonSimctlTool only checks the first argv element, so an option-first inline xcrun call such as |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Follow-up to #2898 (addresses its review); refs #2878.
runXcrunexecutes, sorunXcrunrequiresScopedSimctlArgsfor asimctlinvocation instead of a plain string array — theraw-argv gap the refactor(ios): make unscoped simctl argv a type error at the Apple tool port #2898 review flagged at that executor.
R79 apple-simulator-scopelayering rule to the simctl bypassestsccannot reject, and adds a gate refusing a
simctlargv array outside the brand'smint points however it reaches an executor (host port,
simctl.run,runXcrun).--setscoping logic areunchanged from refactor(ios): make unscoped simctl argv a type error at the Apple tool port #2898.
25 files touched, +410/-260.
Validation
Tested at
607e6b8078(review follow-up on top of064fa5ff3d).pnpm check:affected --runat607e6b8078: EXIT 1, all failures 5 s timeouts under hostcontention (other sessions' suites running). Format, lint, typecheck, layering (R79 tests pass,
zero violations), di-seams, fallow, mcp-metadata, build, package, integration-node and
macos-coverage pass. Every failed file passes when rerun alone except
interaction-response-shape.contract.test.ts"press @ref", which times out identically onorigin/main. The remaining light gates pass alone: integration-progress, replay-compat,daemon-wire-compat, affected-selector, gate-manifest(-model), depgraph, mutation-model.
src/tests under--coverage,pnpm check:coverage-changed --base origin/main): 24/24 lines, 6/6 branches.064fa5ff3dCoverage failure was the Androidsnapshot-helper-session"cancels astalled snapshot" 5 s timeout; it passes 3/3 alone on this branch and on
origin/main.pnpm check:production-exports: EXIT 0, 68 unused exports atwarn(unchangedcount), 18 suppressed, 0 stale suppressions; none of the 25 touched files appear.
simctl --set <set> launch <udid> …argv still carries the set correctly, thenfails with
Invalid devicebecause the runner'sxcodebuildset-redirect(
runner-device-set.ts) empties the scoped set during a coldbuild-for-testing.An A/B against
origin/mainreproduces the identical failure, so this is apre-existing runner issue, not introduced by this branch or Enforce simulator-set scope for simctl: device scope for any udid, set scope only for calls that name no device #2878.