Repository navigation
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
At 8821640 the code looks right, but the iOS off-screen rescue has not been exercised live, so I can't call it ready yet. CI is green: 18 checks, none failing. The PR rewires the call behind the iOS off-screen rescue: a new Apple binding, a new conditional fact, a new executor projection, and lease context is now forwarded (interaction-runtime.ts#L113). The PR body says the querySelector request never fired live. The live run only used the fused tapElementSelector route, which never reaches throwIfOffscreenInteractionTarget. If the binding or wiring is broken, the #1542 rescue disappears on iOS and nothing shows it, because the probe swallows every error and returns null, so a failed call looks like a genuine refusal. Could you run one live test on an iOS simulator (and a physical device if you have one, since the fact also covers kind=device)? Use a selector or @ref
Not blocking, so take or leave: no test pins the new wiring, since deleting I did not run the tests or mutation checks; the claim that those mutations stay green comes from reading the changed tests and the existing daemon-route off-screen tests. I also did not confirm that the BrowserStack and AWS owners declare querySelector unavailable, though isLocalIosRunnerSession still gates them, so parity holds either way. Before merge, I need the live iOS run above showing the |
…ntime Move the last leaked platform-mechanics edge into the runtime contract: the off-screen rescue's live read (#1542) is now the CONDITIONAL `querySelector` operation on the request-bound Apple runtime, executed on the same binding the command was admitted for (ADR 0019 §2/§9). direct-ios-selector.ts keeps only the verdict projection; the daemon no longer builds an Apple runner request. - contracts: QuerySelectorInput/Result + fact cell on SelectorObservationRuntimeOperations; conditional declaration on the four captured touch uses and the four gesture tier uses (parity side: an owner without the read refuses on the capture's verdict unchanged, exactly as every non-Apple owner always did). - platform-apple: bindAppleQuerySelectorRuntime answers the read on the runner, projecting the node to rect+hittability; runner-demand classifies the operation as runner-host. - other owners and the unavailable hub: declare the cell unavailable beside findText. - delete src/daemon/apple-runner-options.ts (its last consumer retired).
…end to end
- replace the local readRect with @agent-device/kernel/record parseRect
- build the runner-call options with { ...input.execution, signal } like
bindAppleFindTextRuntime instead of a field-by-field copy
- forward runtime.operations.querySelector directly; delete the no-op
selectOffscreenTargetQuery wrappers
- inline queryDirectIosSelector into the probe and drop the unused signal
- let the CONDITIONAL fact be the whole rescue gate: the fact already
refuses provider-owned and macOS leaves, so the isLocalIosRunnerSession
check added nothing
- daemon-route tests pin both sides of the conditional (live-rect rescue,
zero-query refusal) and the Apple leaf table pins fact-and-binding
parity; assert AbortError on the aborted query
…options manifest entry
8821640 to
33ee013
Compare
|
Rebased onto Live evidence (iPhone 17 sim, iOS 27.0, fixture app Form screen; raw snapshot marks
Points (all in
Mutations (each run, then reverted): delete Gates serial on the pushed head: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 31 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/daemon/__tests__/interaction-get-runtime-fixture.ts">
<violation number="1" location="src/daemon/__tests__/interaction-get-runtime-fixture.ts:99">
P3: This comment names `querySelectorFixture`, but the binding installs `mockQuerySelector`; correct the identifier so maintainers can find the implementation.</violation>
</file>
<file name="src/daemon/offscreen-target-probe.ts">
<violation number="1" location="src/daemon/offscreen-target-probe.ts:29">
P3: The rescue read is a separate live runner request but carries no AbortSignal, so an aborted interaction request cannot cancel it and the runner query keeps running (interaction-guarantees describes `querySelector` as "a SEPARATE runner request"). `QuerySelectorInput.signal` is part of the contract, and the signal is already available right above this call site: the backend wrapper in `createInteractionBackend` receives `_context.signal` and drops it. Thread `signal` from the backend context through `confirmOffscreenTargetVisible` into the `querySelector` input.</violation>
</file>
<file name="src/daemon/__tests__/offscreen-target-probe.test.ts">
<violation number="1" location="src/daemon/__tests__/offscreen-target-probe.test.ts:37">
P2: The identifier path is no longer asserted: this mock returns a rect regardless of its input, so querying with the wrong selector still passes. Capture the query input and assert `{ key: 'id', value: 'shipping-pickup' }`.</violation>
</file>
<file name="packages/platform-apple/src/__tests__/runtime-selector-query.test.ts">
<violation number="1" location="packages/platform-apple/src/__tests__/runtime-selector-query.test.ts:32">
P2: The PR relies on forwarding `runnerLeaseContext` for leased-runner routing, but neither the input nor this expectation includes it. Add a lease context and assert it reaches `queryAppleRunnerSelector`, or this test will pass if that field is dropped.</violation>
<violation number="2" location="packages/platform-apple/src/__tests__/runtime-selector-query.test.ts:60">
P2: `bind(controller.signal)` sets the binding signal, while this query omits `QuerySelectorInput.signal`; the test never exercises caller-signal cancellation or its composition. Add a separate call with an aborted signal on the query input while retaining binding-abort coverage.</violation>
</file>
<file name="packages/provider-limrun/src/facts-runtime.ts">
<violation number="1" location="packages/provider-limrun/src/facts-runtime.ts:177">
P3: This exposes a custom-snapshot hint for the unrelated live selector read, leaving the reason `querySelector` is unavailable unclear. Give this capability a selector-specific hint.</violation>
</file>
<file name="packages/platform-web/src/runtime.ts">
<violation number="1" location="packages/platform-web/src/runtime.ts:369">
P3: This marks a web-wide unavailable operation as an unsupported device kind and gives it an unrelated `open` hint. Use a selector-specific `unsupported-platform-leaf` fact so consumers inspecting the refusal do not receive misleading guidance.</violation>
</file>
<file name="src/daemon/interaction/internal/__tests__/interaction-touch-runtime.test.ts">
<violation number="1" location="src/daemon/interaction/internal/__tests__/interaction-touch-runtime.test.ts:271">
P2: This route test cannot catch the leased-runner regression described by the PR: its `contextFromFlags` fixture supplies no runner lease, and the query assertion checks only the selector. Give the test a lease context and assert `execution.runnerLeaseContext` reaches the query input.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| ); | ||
| test('confirmIosOffscreenTargetVisible: returns the live rect when the owner confirms hittable + inside the viewport', async () => { | ||
| const rect = await probeWith(found({ x: 126, y: 136, width: 75, height: 38 }, true), { | ||
| identifier: 'shipping-pickup', |
There was a problem hiding this comment.
P2: The identifier path is no longer asserted: this mock returns a rect regardless of its input, so querying with the wrong selector still passes. Capture the query input and assert { key: 'id', value: 'shipping-pickup' }.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon/__tests__/offscreen-target-probe.test.ts, line 37:
<comment>The identifier path is no longer asserted: this mock returns a rect regardless of its input, so querying with the wrong selector still passes. Capture the query input and assert `{ key: 'id', value: 'shipping-pickup' }`.</comment>
<file context>
@@ -1,132 +1,129 @@
- );
+test('confirmIosOffscreenTargetVisible: returns the live rect when the owner confirms hittable + inside the viewport', async () => {
+ const rect = await probeWith(found({ x: 126, y: 136, width: 75, height: 38 }, true), {
+ identifier: 'shipping-pickup',
+ });
</file context>
| await bind().querySelector({ | ||
| selector: { key: 'id', value: 'submit-order' }, | ||
| options: { appBundleId: APP }, | ||
| execution: { |
There was a problem hiding this comment.
P2: The PR relies on forwarding runnerLeaseContext for leased-runner routing, but neither the input nor this expectation includes it. Add a lease context and assert it reaches queryAppleRunnerSelector, or this test will pass if that field is dropped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/platform-apple/src/__tests__/runtime-selector-query.test.ts, line 32:
<comment>The PR relies on forwarding `runnerLeaseContext` for leased-runner routing, but neither the input nor this expectation includes it. Add a lease context and assert it reaches `queryAppleRunnerSelector`, or this test will pass if that field is dropped.</comment>
<file context>
@@ -0,0 +1,91 @@
+ await bind().querySelector({
+ selector: { key: 'id', value: 'submit-order' },
+ options: { appBundleId: APP },
+ execution: {
+ requestId: 'req-9',
+ logPath: '/tmp/runner.log',
</file context>
| controller.abort(); | ||
|
|
||
| await expect( | ||
| bind(controller.signal).querySelector({ selector: { key: 'label', value: 'Go' } }), |
There was a problem hiding this comment.
P2: bind(controller.signal) sets the binding signal, while this query omits QuerySelectorInput.signal; the test never exercises caller-signal cancellation or its composition. Add a separate call with an aborted signal on the query input while retaining binding-abort coverage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/platform-apple/src/__tests__/runtime-selector-query.test.ts, line 60:
<comment>`bind(controller.signal)` sets the binding signal, while this query omits `QuerySelectorInput.signal`; the test never exercises caller-signal cancellation or its composition. Add a separate call with an aborted signal on the query input while retaining binding-abort coverage.</comment>
<file context>
@@ -0,0 +1,91 @@
+ controller.abort();
+
+ await expect(
+ bind(controller.signal).querySelector({ selector: { key: 'label', value: 'Go' } }),
+ ).rejects.toMatchObject({ name: 'AbortError' });
+ expect(queryAppleRunnerSelector).not.toHaveBeenCalled();
</file context>
| }, | ||
| sessionName: RESCUE_ROUTE_SESSION, | ||
| sessionStore, | ||
| contextFromFlags, |
There was a problem hiding this comment.
P2: This route test cannot catch the leased-runner regression described by the PR: its contextFromFlags fixture supplies no runner lease, and the query assertion checks only the selector. Give the test a lease context and assert execution.runnerLeaseContext reaches the query input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon/interaction/internal/__tests__/interaction-touch-runtime.test.ts, line 271:
<comment>This route test cannot catch the leased-runner regression described by the PR: its `contextFromFlags` fixture supplies no runner lease, and the query assertion checks only the selector. Give the test a lease context and assert `execution.runnerLeaseContext` reaches the query input.</comment>
<file context>
@@ -215,6 +217,99 @@ test('press @ref fails closed when stored ref bounds are invalid (ADR 0014)', as
+ },
+ sessionName: RESCUE_ROUTE_SESSION,
+ sessionStore,
+ contextFromFlags,
+ ...getRuntimeBindings(),
+ });
</file context>
|
|
||
| /** | ||
| * The CONDITIONAL live selector read (#1542 rescue). Off by default so only the tests that pin | ||
| * the rescue's wiring flip it; when advertised, the binding answers with `querySelectorFixture`. |
There was a problem hiding this comment.
P3: This comment names querySelectorFixture, but the binding installs mockQuerySelector; correct the identifier so maintainers can find the implementation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon/__tests__/interaction-get-runtime-fixture.ts, line 99:
<comment>This comment names `querySelectorFixture`, but the binding installs `mockQuerySelector`; correct the identifier so maintainers can find the implementation.</comment>
<file context>
@@ -90,6 +94,14 @@ const mockFillRef = vi.fn(async (input: FillRefInput) => ({
+/**
+ * The CONDITIONAL live selector read (#1542 rescue). Off by default so only the tests that pin
+ * the rescue's wiring flip it; when advertised, the binding answers with `querySelectorFixture`.
+ */
+export const mockQuerySelector = vi.fn(
</file context>
| * the rescue's wiring flip it; when advertised, the binding answers with `querySelectorFixture`. | |
| * the rescue's wiring flip it; when advertised, the binding answers with `mockQuerySelector`. |
| let nodeReading: Awaited<ReturnType<typeof querySelector>>['node']; | ||
| try { | ||
| result = await queryDirectIosSelector(session, selector, requestOptions); | ||
| ({ found, node: nodeReading } = await querySelector({ |
There was a problem hiding this comment.
P3: The rescue read is a separate live runner request but carries no AbortSignal, so an aborted interaction request cannot cancel it and the runner query keeps running (interaction-guarantees describes querySelector as "a SEPARATE runner request"). QuerySelectorInput.signal is part of the contract, and the signal is already available right above this call site: the backend wrapper in createInteractionBackend receives _context.signal and drops it. Thread signal from the backend context through confirmOffscreenTargetVisible into the querySelector input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon/offscreen-target-probe.ts, line 29:
<comment>The rescue read is a separate live runner request but carries no AbortSignal, so an aborted interaction request cannot cancel it and the runner query keeps running (interaction-guarantees describes `querySelector` as "a SEPARATE runner request"). `QuerySelectorInput.signal` is part of the contract, and the signal is already available right above this call site: the backend wrapper in `createInteractionBackend` receives `_context.signal` and drops it. Thread `signal` from the backend context through `confirmOffscreenTargetVisible` into the `querySelector` input.</comment>
<file context>
@@ -1,25 +1,42 @@
+ let nodeReading: Awaited<ReturnType<typeof querySelector>>['node'];
try {
- result = await queryDirectIosSelector(session, selector, requestOptions);
+ ({ found, node: nodeReading } = await querySelector({
+ selector,
+ ...(session.appBundleId === undefined
</file context>
| // ownership answer covers the live selector read: no rescue hook on a provider device. | ||
| ...selectorObservationRuntimeOperationFacts({ | ||
| findText: customSnapshotUnavailable, | ||
| querySelector: customSnapshotUnavailable, |
There was a problem hiding this comment.
P3: This exposes a custom-snapshot hint for the unrelated live selector read, leaving the reason querySelector is unavailable unclear. Give this capability a selector-specific hint.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-limrun/src/facts-runtime.ts, line 177:
<comment>This exposes a custom-snapshot hint for the unrelated live selector read, leaving the reason `querySelector` is unavailable unclear. Give this capability a selector-specific hint.</comment>
<file context>
@@ -170,9 +170,11 @@ export function limrunAppLogFacts(
+ // ownership answer covers the live selector read: no rescue hook on a provider device.
...selectorObservationRuntimeOperationFacts({
findText: customSnapshotUnavailable,
+ querySelector: customSnapshotUnavailable,
}),
...viewportRuntimeOperationFacts({ setViewport: viewportUnavailable }),
</file context>
| querySelector: customSnapshotUnavailable, | |
| querySelector: unavailableFact( | |
| 'unsupported-provider-mode', | |
| 'Limrun-owned devices do not expose a live selector read.', | |
| ), |
| // selector read either: the off-screen guard refuses on the capture's verdict unchanged. | ||
| ...selectorObservationRuntimeOperationFacts({ | ||
| findText: openTargetKindUnavailable, | ||
| querySelector: openTargetKindUnavailable, |
There was a problem hiding this comment.
P3: This marks a web-wide unavailable operation as an unsupported device kind and gives it an unrelated open hint. Use a selector-specific unsupported-platform-leaf fact so consumers inspecting the refusal do not receive misleading guidance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/platform-web/src/runtime.ts, line 369:
<comment>This marks a web-wide unavailable operation as an unsupported device kind and gives it an unrelated `open` hint. Use a selector-specific `unsupported-platform-leaf` fact so consumers inspecting the refusal do not receive misleading guidance.</comment>
<file context>
@@ -362,9 +362,11 @@ function webRuntimeFacts(
+ // selector read either: the off-screen guard refuses on the capture's verdict unchanged.
...selectorObservationRuntimeOperationFacts({
findText: openTargetKindUnavailable,
+ querySelector: openTargetKindUnavailable,
}),
...screenshotRuntimeOperationFacts({ capture: browserDevice }),
</file context>
| querySelector: openTargetKindUnavailable, | |
| querySelector: unavailableFact( | |
| 'unsupported-platform-leaf', | |
| 'Live selector reads are not supported on web targets.', | |
| ), |
|
The earlier findings on parseRect reuse and live iOS querySelector evidence are fixed at 33ee013, and I found no new code problems. CI is green: 19 checks, 0 failing. I did not run the tests or the mutation checks. The claim that the capturedTapUse-conditional mutation turns the rescue test red comes from the author and from reading the code. The live evidence is the author's quoted log lines. They show the rescue read firing and the genuine refusal standing, but not a rescue that taps at the live rect. The physical-device kind ran in tests only. Not blocking, and you can take or leave these: the delta comment at interaction-runtime.ts:87 says macOS and watchOS owners do not advertise the read, but watchOS does advertise querySelector (runtime.ts:294) and only never holds the hook because captured uses are refused there. Dropping "and watchOS kinds" and the sentence about the retired isLocalIosRunnerSession gate would fix it, and the readOffscreenTargetRescue doc block (lines 98-107) narrates that retired gate too. The header comment at interaction-touch-runtime.test.ts:220 says dropping ...selectorQueryOperations from the Apple runtime breaks one of these route tests. These tests bind the fixture owner, so only runtime.test.ts catches that mutation. The comment could name just the two mutations the route tests catch. On the Cubic threads, the P2 on test names at runtime-selector-query.test.ts:55-60 and the P3 on the fixture doc at interaction-get-runtime-fixture.ts:99 still apply, and the P2 should be settled before merge (rename the tests or reply on the thread); the P3 is optional. These five do not apply, so you can resolve them. The id path is pinned in the route test, the direct-ios-selector unit test and the probe test (thread). Lease forwarding is the spread at runtime-selector-query.ts:37, and a lease assertion would only test object spread (thread). Daemon-side lease forwarding is owned by runtimeExecutionFromContext, which this PR does not change (thread). The Apple binding already composes request.scope.signal, so an aborted request aborts the rescue read (thread). The two threads on the unavailable-fact hint never reach the user, because the guard refuses with offscreen_ref (iOS thread, web thread). Apart from that P2 thread, nothing stops the merge on the code side. |
This branch has not been deployed
1 participant
Summary
Removes the last daemon → platform-mechanics value import (R76
leaked-platform-mechanics):src/daemon/direct-ios-selector.tscalledqueryAppleRuntimeSelectorfrom rootsrc/platform-runtime-apple-resources.tsdirectly. The Apple runtime now exposes aquerySelectoroperation (SelectorQueryRuntimeOperationsin@agent-device/contracts); the daemon obtains it through the request-boundBindDeviceRuntimeand the iOS off-screen rescue (offscreen-target-probe.ts) consumes the bound operation only. The rescue's runner call now forwards the full execution projection includingrunnerLeaseContext(the logical-lease identity the retired edge never sent), so a leased-runner host routes the query to the right lease.querySelectoris declared conditional on the captured touch/long-press/hover/fill and gesture-tier uses: owners that don't advertise it refuse exactly as pre-rescue, so Android/web/HarmonyOS keep the verdict path unchanged. Apple is the only owner binding it (iOS-family, simulator + device). The CONDITIONAL fact is the rescue's whole gate: provider-owned and macOS leaves declare the read unavailable in their facts. 31 files.Validation
Head
33ee013ed. Green:check:quick,check:layering,check:di-seams, eager-closure budgets, test-file-size-ratchet,check:affected --run.Live (iPhone 17 sim, iOS 27.0, fixture app Form screen): raw snapshot marked
shipping-pickup@E81 off-screen (y=1060 > 874);press @e81(requestIdca8d7890dd8c87c3) produced a runnerquerySelectorcommand (runner-46eb14c5…, XCUITest predicateidentifier ==[c] "shipping-pickup"in runner.log) and the response wasCOMMAND_FAILEDwithdetails.reason: offscreen_ref— the rescue read fired and the genuine refusal stood. Same-shape Android leg (emulator-5554, android-helper 0.21.26-dev):press @e108refusedoffscreen_refwith zero extra queries in the request log. Daemon-route tests pin both sides of the conditional (live-rect rescue, zero-query refusal) plus the Apple fact/binding table; three wiring mutations each killed a test. Physical iPhone unavailable (locked), so the kind=device half ran in tests only. Cleanup verified: sessions closed, sim/emulator down,device status --staleclean.