refactor(apple): put the Apple app states at one source and reuse the contract failure - #2950
Conversation
… contract failure #2929 added `appState` beside the #2682 activation disclosure, and the two state lists ended up pointing the wrong way: the full `XCUIApplication.State` list was spread from the smaller prior-state subset, so the narrower concept owned the names the broader one answers with. The full list is now the single declaration and the prior states derive from it by dropping the one state the runner skips `activate()` in. The Apple binder also restated two things contracts already owns: a resolver type that is the local interactor resolver under another name, and the advertised-without-an-implementation failure the `invalidRuntimeContract` leaf already builds. Both are reused now, and the failure branch finally has a test pinning its typed reason, which is what keeps it outside every closed reason set that licenses answering from the session record. `AppleApplicationState` no longer re-exports through `contracts/app-state-runtime`: the kernel is its declaration site and both consumers read it there.
`invalidRuntimeContract` had no export target: it was a leaf kept out of the `platform-runtime` façade so that façade stays exhaustive over its sources and inside its eager-closure budget. Now that a platform package consumes it, the subpath is its declaration site in the manifest, and the independently committed contracts export snapshot carries it with the one-line regeneration.
|
We've triggered an ultrareview automatically — This cross-cutting refactor changes the Apple app-state source of truth, shifts contract subpath exports, and alters the exported type shape of the prior-state list, so a subtle API or mapping regression could break downstream consumers or the appState command.. I'll post findings when complete. An ultrareview is cubic's deepest review, catching hard-to-find bugs in the most critical PRs. It runs a longer, multi-pass analysis using cubic's most capable review models, and typically takes around 30 minutes. It consumes your team's reviewed-lines allowance at 3× the standard rate. Automated ultrareviews are disabled by default. We triggered this run as part of your trial. Want cubic to do this for every high-risk PR? Enable auto-ultrareview in your settings. |
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Reviewed d76aeb2. I found no defect: the Apple app states now come from one source, the invalid-state path reuses the shared contract failure, and the runner tests confirm the raw-value mapping without a change to the runner wire. All 19 checks pass, and there are no conflicts. Not blocking: the new test checks code, message and |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Follow-up to #2929 that resolves the duplication that PR surfaced, without touching the runner wire.
APPLE_APPLICATION_STATESis now the single declaration of theXCApplication.Statenames, andIOS_TARGET_ACTIVATION_PRIOR_STATESderives from it by droppingrunningForeground. Before, the full list spread from the prior-state subset, so the narrower concept owned the names theappStatecommand answers with. Behavior and order are unchanged; the decoder's raw table and the native SDK pins still verify the mapping.AppStateInteractorResolveris now an alias ofLocalInteractorOperationResolver, and the advertised-without-an-implementation failure is built byinvalidRuntimeContractinstead of a second copy of the code, reason, and hint. That failure branch had no test; it now has one pinning the typed reason, which is what keeps it outside every closed reason set that licenses answering from the session record (ADR 0019 §2).AppleApplicationStatestops re-exporting throughcontracts/app-state-runtime; the kernel is its declaration site and both consumers read it there.Evaluated and skipped from the same review, with reasons:
targetActivation.priorStateas a name instead of an integer. Not safe on this repo's seams:runner-artifact.tsaccepts an operator-supplied.xctestrun(iosXctestrunFile) andAGENT_DEVICE_IOS_RUNNER_DERIVED_PATHoverrides the derived path, and neither spends the runner-source fingerprint that guards cache reuse and lease adoption. A daemon can therefore reach a runner it did not build, and the numeric table plus the native raw-value pins (#2726) are what catch SDK drift on that path.appBundleIdin the Apple input to drop a session check. The two checks are not redundant: one refuses explicit--platformrequests before device resolution, the other decides afterresolveCommandDevicefor selector-targeted requests.9 files, +58/−39. No CLI, MCP, docs, or wire change.
Validation
Tested at d76aeb2.
pnpm check:affected --run: all runnable checks passed (format, lint, typecheck, layering, di-seams, fallow, mcp-metadata, build, package, integration-node, macos-coverage, vitest-related, integration-progress, replay-compat, daemon-wire-compat, affected-selector, and the model/tooling suites).pnpm test:unit: 1403 files / 11374 tests passed.pnpm check:fallow --base origin/main: no issues in the 9 changed files.pnpm check:production-exports: 68 findings, identical to theorigin/mainbaseline (no new unused export).pnpm check:daemon-wire-compat: protocol unchanged vsv0.21.14.{}fails only that test.No device-facing behavior changed, so no device runs apply.