Repository navigation
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
The Limrun alert legs duplicate the Android tree-alert driver, and that needs to change before merge (reviewed at 4a1862b). The Is there a reason The two threads on Cubic's confirm-on-ambiguous-capture (alert.ts:173) and duplicate-label skipping (alert-detection.ts:152) should be settled once in that shared driver. Please also validate on a live Limrun run that These open threads still apply: pid boundary comment, CHANGELOG app dialogs, plus six lower-priority test and fixture threads. This docs thread does not apply, because the page describes how the backend acts and gives accurate user guidance; please resolve it. The failing Smoke Tests job runs the Apple XCTest-runner simulator smoke. This diff touches only |
…red driver PR #3699 review: the Limrun iOS alert legs duplicated the Android tree-alert driver (poll, confirm, same-alert, response building) and its label vocabulary. Both backends now reach those steps through one backend-neutral driver, runTreeAlertAction in platform-android (exported via the mechanics facet provider-limrun already depends on), and classify button roles through one English vocabulary that lives in contracts/alert-contract with the alert budgets and typed reasons (contracts owns vocabulary only; the driver loop owns the AppError/timer mechanics R18 keeps out of contracts). Each backend supplies only its own seams: Android wraps its dump reader and keeps its platform-id roles, neutral/Back fallbacks, and tap/Back presses; Limrun supplies the blank-root process-exclusive host reader, the label-strict chooser, and client.tap. The confirm-on-ambiguous-capture and duplicate-label threads settle once in the shared code: the three-way alert|none|undecided observation gates confirmation in the driver, and selectUniqueAlertButtonByRole refuses duplicated roles at the shared choice boundary. The deadline order resolves to read-first: over a positive window both predecessor loops already observed before waiting. The alert-not-found and undecided refusals now carry the same typed details.reason and dispatched: 'no' on both backends.
|
Reviewed a27302c. The earlier findings are fixed except the ones below, and one new regression remains in the shared button choice. The 20 checks pass at this commit, but the Android alert suites pass only because no fixture has a multi-grant permission sheet. I know of no conflicts.
The PR body still lists the final-head live evidence as pending, and the device route has changed since the earlier fail-before run: it now goes through the shared driver, read-first deadline order and the uniqueness boundary. Unit fixtures cannot show that the Limrun tap lands, or that the confirm leg sees I could not read the Limrun captures behind the "no hittable key" claim, so I accepted it on consistency. I did not run tests, and I did not confirm on a live Android 11+ device that both grant buttons appear with Simplicity question: could the shared driver and vocabulary stay, with the choice boundary as a declared role-plus-preference table in On the open Cubic threads: these are fixed at this head and can be resolved: blank-root pid subtree, changelog residual, Android dismiss retry, typed refusal reason, driver reason test, one-tap dismiss test, and fixture comment. The label dedupe thread is fixed too, but the fix went too far into role-level uniqueness, as described above. Two threads do not apply: hittable:true because captures omit the key and only an explicit false vetoes, and limrun.md wording because the page describes backend behavior accurately. Before merge, please restore single-press accept on multi-grant permission sheets with regression tests, then post the live Limrun ATT evidence. |
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 13 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
The earlier gaps from a27302c are fixed at 8d5e9d2: the preference table now lives in the shared alert contract, each backend keeps its own fallback, and the regression tests cover the earlier cases. Live proof at this head is still missing, and the cancellation work added in this round goes beyond what #3349 asks for. The device route changed again in this round: ranked choice, window-guarded observer, signal-gated press, and a separate confirm window. The PR body says final-head live verification is still outstanding, and unit fakes cannot show that the Limrun tap lands once or that confirm sees Is the cancellation and window apparatus needed for #3349? It adds a second feature to the shared driver: request signals through every seam, wrapped errors keyed on the window-expired reason, Not blocking, and you can take or leave these: alertSeamInterrupted is declared to return an error but throws in its canceled branch, and the request signal reaches the backends through two channels ( On the open threads from other reviewers, these still apply: the Limrun capture ignoring the seam signal, same-label buttons on Android, unranked accept filtered before the tie check, the docs on alert-window-expired, the changelog claim about bounded budgets and the AbortSignal.timeout overflow. The adb executor options thread does not apply, because every production caller passes only CI is still pending: all 9 non-passing jobs are queued or in progress, with no failure logs yet. The diff overlaps Lint/Typecheck, Coverage and possibly Android smoke, while the Apple smoke lanes do not overlap it. I did not run the tests, and I did not reproduce the "20 ms wait returns alert at 59 ms" case. Before merge, this needs the live Limrun iOS and Android 11+ runs at the final head, and a decision on moving the cancellation work to its own PR. |
…red driver PR #3699 review: the Limrun iOS alert legs duplicated the Android tree-alert driver (poll, confirm, same-alert, response building) and its label vocabulary. Both backends now reach those steps through one backend-neutral driver, runTreeAlertAction in platform-android (exported via the mechanics facet provider-limrun already depends on), and classify button roles through one English vocabulary that lives in contracts/alert-contract with the alert budgets and typed reasons (contracts owns vocabulary only; the driver loop owns the AppError/timer mechanics R18 keeps out of contracts). Each backend supplies only its own seams: Android wraps its dump reader and keeps its platform-id roles, neutral/Back fallbacks, and tap/Back presses; Limrun supplies the blank-root process-exclusive host reader, the label-strict chooser, and client.tap. The confirm-on-ambiguous-capture and duplicate-label threads settle once in the shared code: the three-way alert|none|undecided observation gates confirmation in the driver, and selectUniqueAlertButtonByRole refuses duplicated roles at the shared choice boundary. The deadline order resolves to read-first: over a positive window both predecessor loops already observed before waiting. The alert-not-found and undecided refusals now carry the same typed details.reason and dispatched: 'no' on both backends.
8d5e9d2 to
482cff1
Compare
|
At 482cff1, the earlier gaps are mostly closed, but two defects remain, and the Android smoke check now fails because of one of them. The earlier a27302c review asked for a The Android smoke step Localized Android permission sheets also regress, at https://github.com/callstack/agent-device/blob/482cff1/packages/contracts/src/alert-contract.ts#L118. Could the PR get smaller? Keep the preference table, but rank by backend-declared keys: label rank for Limrun, permission-id rank for Android. Then drop the per-capture window code ( The open inline threads on same-label buttons in The failing Android smoke step looks related to this change. Only the new Before merge, please stop the per-capture window from refusing the first Android capture, resolve the conflicts, and then show two live runs at the final head. First, the Android smoke |
…red driver PR #3699 review: the Limrun iOS alert legs duplicated the Android tree-alert driver (poll, confirm, same-alert, response building) and its label vocabulary. Both backends now reach those steps through one backend-neutral driver, runTreeAlertAction in platform-android (exported via the mechanics facet provider-limrun already depends on), and classify button roles through one English vocabulary that lives in contracts/alert-contract with the alert budgets and typed reasons (contracts owns vocabulary only; the driver loop owns the AppError/timer mechanics R18 keeps out of contracts). Each backend supplies only its own seams: Android wraps its dump reader and keeps its platform-id roles, neutral/Back fallbacks, and tap/Back presses; Limrun supplies the blank-root process-exclusive host reader, the label-strict chooser, and client.tap. The confirm-on-ambiguous-capture and duplicate-label threads settle once in the shared code: the three-way alert|none|undecided observation gates confirmation in the driver, and selectUniqueAlertButtonByRole refuses duplicated roles at the shared choice boundary. The deadline order resolves to read-first: over a positive window both predecessor loops already observed before waiting. The alert-not-found and undecided refusals now carry the same typed details.reason and dispatched: 'no' on both backends.
482cff1 to
5b1e1c1
Compare
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…red driver PR #3699 review: the Limrun iOS alert legs duplicated the Android tree-alert driver (poll, confirm, same-alert, response building) and its label vocabulary. Both backends now reach those steps through one backend-neutral driver, runTreeAlertAction in platform-android (exported via the mechanics facet provider-limrun already depends on), and classify button roles through one English vocabulary that lives in contracts/alert-contract with the alert budgets and typed reasons (contracts owns vocabulary only; the driver loop owns the AppError/timer mechanics R18 keeps out of contracts). Each backend supplies only its own seams: Android wraps its dump reader and keeps its platform-id roles, neutral/Back fallbacks, and tap/Back presses; Limrun supplies the blank-root process-exclusive host reader, the label-strict chooser, and client.tap. The confirm-on-ambiguous-capture and duplicate-label threads settle once in the shared code: the three-way alert|none|undecided observation gates confirmation in the driver, and selectUniqueAlertButtonByRole refuses duplicated roles at the shared choice boundary. The deadline order resolves to read-first: over a positive window both predecessor loops already observed before waiting. The alert-not-found and undecided refusals now carry the same typed details.reason and dispatched: 'no' on both backends.
2482e24 to
f584e8a
Compare
|
Head Android smoke step. The Live proofs at this head (repo-built CLI / instrumented real client seam, isolated state):
Receipts: direct gate Kept honest, not widened: Limrun SDK 0.64.1 exposes no abort on |
|
Coverage run 38924441176 at Fixed at |
|
Receipts for
No further changes planned on my side for the alert family at this head. |
|
Corrected Android live proof at The earlier proof script ( Corrected run: Results (raw JSON + per-command exits in Limits honestly kept: this Android leg does not instrument tap counts; the Limrun iOS instrumented-seam proof remains banked at the earlier head (PROBE_EXIT 0), whose attribution to that head stands — the newer commits change only the cut-off error path. |
…red driver PR #3699 review: the Limrun iOS alert legs duplicated the Android tree-alert driver (poll, confirm, same-alert, response building) and its label vocabulary. Both backends now reach those steps through one backend-neutral driver, runTreeAlertAction in platform-android (exported via the mechanics facet provider-limrun already depends on), and classify button roles through one English vocabulary that lives in contracts/alert-contract with the alert budgets and typed reasons (contracts owns vocabulary only; the driver loop owns the AppError/timer mechanics R18 keeps out of contracts). Each backend supplies only its own seams: Android wraps its dump reader and keeps its platform-id roles, neutral/Back fallbacks, and tap/Back presses; Limrun supplies the blank-root process-exclusive host reader, the label-strict chooser, and client.tap. The confirm-on-ambiguous-capture and duplicate-label threads settle once in the shared code: the three-way alert|none|undecided observation gates confirmation in the driver, and selectUniqueAlertButtonByRole refuses duplicated roles at the shared choice boundary. The deadline order resolves to read-first: over a positive window both predecessor loops already observed before waiting. The alert-not-found and undecided refusals now carry the same typed details.reason and dispatched: 'no' on both backends.
957726b to
326aa3b
Compare
…red driver PR #3699 review: the Limrun iOS alert legs duplicated the Android tree-alert driver (poll, confirm, same-alert, response building) and its label vocabulary. Both backends now reach those steps through one backend-neutral driver, runTreeAlertAction in platform-android (exported via the mechanics facet provider-limrun already depends on), and classify button roles through one English vocabulary that lives in contracts/alert-contract with the alert budgets and typed reasons (contracts owns vocabulary only; the driver loop owns the AppError/timer mechanics R18 keeps out of contracts). Each backend supplies only its own seams: Android wraps its dump reader and keeps its platform-id roles, neutral/Back fallbacks, and tap/Back presses; Limrun supplies the blank-root process-exclusive host reader, the label-strict chooser, and client.tap. The confirm-on-ambiguous-capture and duplicate-label threads settle once in the shared code: the three-way alert|none|undecided observation gates confirmation in the driver, and selectUniqueAlertButtonByRole refuses duplicated roles at the shared choice boundary. The deadline order resolves to read-first: over a positive window both predecessor loops already observed before waiting. The alert-not-found and undecided refusals now carry the same typed details.reason and dispatched: 'no' on both backends.
326aa3b to
60aec19
Compare
Limrun iOS direct sessions refused all four alert legs with unsupported-provider-mode even though the session's element-tree capture already shows the system alert and its touch seam can press its buttons. Issue #3349. Detect the alert from the capture: a system alert arrives as its own blank-label AXApplication root hosting text and pressable buttons, while the app's own UIAlertController rides the app's named root, so app-owned dialogs stay app UI. Accept and dismiss press exactly the button whose label states the asked-for choice and confirm the alert is gone; an ambiguous choice is reported, never guessed, and dismiss never authorizes tracking. The four legs bind through the same catalog and facts the Android tree-based legs already use.
Review follow-up on the alert legs: a sheet must offer at least two pressable choices, launch-icon buttons never qualify, a capture whose host sheets disagree is refused instead of shadowing one with the other, and an action fires only when exactly one button states its choice. The legs' poll/confirm budgets now spend through an injected sleep seam so their tests drive the clock, a malformed timeout fails as a bounded default instead of an unbound loop, and the grant vocabulary covers the iOS variants (Allow Once / While Using App / All the Time) and Close App. User docs and CHANGELOG updated.
…red driver PR #3699 review: the Limrun iOS alert legs duplicated the Android tree-alert driver (poll, confirm, same-alert, response building) and its label vocabulary. Both backends now reach those steps through one backend-neutral driver, runTreeAlertAction in platform-android (exported via the mechanics facet provider-limrun already depends on), and classify button roles through one English vocabulary that lives in contracts/alert-contract with the alert budgets and typed reasons (contracts owns vocabulary only; the driver loop owns the AppError/timer mechanics R18 keeps out of contracts). Each backend supplies only its own seams: Android wraps its dump reader and keeps its platform-id roles, neutral/Back fallbacks, and tap/Back presses; Limrun supplies the blank-root process-exclusive host reader, the label-strict chooser, and client.tap. The confirm-on-ambiguous-capture and duplicate-label threads settle once in the shared code: the three-way alert|none|undecided observation gates confirmation in the driver, and selectUniqueAlertButtonByRole refuses duplicated roles at the shared choice boundary. The deadline order resolves to read-first: over a positive window both predecessor loops already observed before waiting. The alert-not-found and undecided refusals now carry the same typed details.reason and dispatched: 'no' on both backends.
origin/main migrated the Android alert loops to pollUntilSettled/Deadline and moved timing into fake timers at the test edge. The shared driver adopts the same shape: poll/confirm run on the deadline primitive, so the driver-owned sleep seam is dead weight and goes — Limrun's tests now drive the clock the way the migrated Android tests do.
…y catch-name lint Adds the maintainer's live-validated sequence at the unit seam: accept taps exactly once, and a following get on the same seam reports no alert. Renames catch params to satisfy oxlint's catch-error-name (three findings the affected gate reported).
The unique-role choice boundary refused exactly the sheets the fix is for: Android 11+'s permission_allow_foreground_only_button and permission_allow_one_time_button both classify accept, and iOS's 'Allow Once'/'Allow While Using App' sheet refused the same way. The shared vocabulary is now a per-role preference table: distinct grants resolve by declared rank (a lasting grant before a one-time grant), and the acting legs refuse only when the capture names no single physical button — the same label at two points, or vocabulary-free multiplicity. Android keeps its neutral-button/Back fallback, Limrun keeps none.
… owns Three review findings on the shared tree-alert driver, plus the test that masked a polling loop: - alert get on a tree backend answered undecided exactly like absence (alert: null, message-only difference). The result now carries a typed status (visible/none/undecided) so clients switch on states, not text. - The budgets bounded only the poll sleeps: a capture or press that hung could outrun the leg. Each leg now builds a window (deadline clock plus the joined window/caller signal) that reaches the capture and press seams; the Android legs hand it to the snapshot capture and the adb input sends, which kill their child on abort. A capture that lands once its window closed is refused on a typed alert-window-expired reason and never consumed or pressed; a press cut off mid-flight says dispatched: unknown, one refused before dispatch says no. - Limrun's WebSocket transport exposes no abort on elementTree/tap (checked against the shipped typings), so the consumption rule is what keeps it honest, and the SDK gap is stated rather than papered over. - The awaitAlert first-capture test now asserts the capture count; its same-alert fixture had made it vacuous.
The shared tree-alert driver consumed an answer from a poll that started after its window closed (the loop tests settled before remaining, and an expired-at-start probe rode unguarded), and its first probe paid the Android helper cold start inside the action budget, aborting the very read that leg was about to press on — the executed smoke failure. The first observation is now read-first, unarmed, consumed once, and arms the window behind it; a later probe starting past the deadline never captures at all, and the press rides the armed window. - contracts: a mix of ranked and unranked role candidates refuses instead of letting the one known label win (selectRankedAlertButton- ByRole); MAX_ALERT_WINDOW_MS names the timer ceiling the daemon refuses above (INVALID_ARGS) and the driver defaults above. - android: same-label dedup keys on the physical center, so nested nodes collapse and two distinct OKs reach the shared refusal; the input dispatch option type owns cancellation only, and the tap forward picks fields so allowFailure cannot reach the transport. - docs: alert-window-expired/none/undecided reasons stated exactly per phase, Limrun's non-abortable transport described as post-capture and pre-press guards with the hung-request limit named. Prose at the driver trims to invariants.
…confirmed press The executed Android smoke failure at 5b1e1c1: the session's alert probes ran without the app-backed helper session scope, so each probe cold-started the snapshot helper inside the window that bounds it — the request log shows the post-press confirm capture aborted at 2.3s ("operation was aborted due to timeout") while the canary proves the dialog was in fact dismissed. Alert reads now carry the same helperSessionScope as every other app-backed read. The failure also proved the wording lied: with no confirm capture ever answering, "did not dismiss the visible alert"/"dialog is still visible" accused a dialog that was already gone. Only a re-read naming the same alert earns the still-visible failure; an unconfirmed press now says exactly that, on both the cut-off and the spent-budget exits, with a hint to run alert get before retrying. The same-alert stuck-alert failure keeps its message. Pinned: interactor scoping regression (fail-before against 5b1e1c1) and the three Limrun confirm-phase tests moved to the truthful split; the Android still-visible pin keeps its message via the same-alert path.
Two review findings on the confirm split, one batch: - The spent-budget exits of confirmTreeAlertDismissed omitted details.reason, while commands.md promises every post-press failure branches on alert-window-expired. Both unconfirmed and still-visible failures now carry the typed reason, pinned in the Android and Limrun suites. - The same-alert flag latched on the first matching capture, so a later undecided re-read still reported the dialog as currently visible. Each probe now replaces the verdict with its own observation: the still-visible failure requires the latest capture that answered to name the same alert, and a fresher undecided read exits as an unconfirmed press. A probe whose capture never answered is not a completed observation, so the thrown cutoff keeps saying only that the press went unconfirmed, at any earlier match. Pinned fail-before at the pre-fix alert.ts: same-alert then undecided to budget-end, and same-alert then a hung capture, both expect the unconfirmed message and the typed reason. The helper-scope comment is trimmed to the non-obvious constraint per AGENTS.
…swered sighting The Coverage run at f584e8a (38924441176) failed the maintainer-owned provider-integration scenario 'Android alert accept fails when the dialog stays visible' — a suite the affected gate's related-test selection does not reach, so the local exact-head receipt never ran it. The driver, not the oracle, was wrong: f584's confirm catch downgraded every thrown cutoff to unconfirmed, discarding the captures that had answered before it. On CI the final confirm capture starts inside the 2s window and lands past it (the thrown path); locally it starts after the deadline (the sentinel path), which is why the same commit passed one and failed the other. The dialog had been re-named by answered reads; the error claimed the check never answered. The latest-completed-observation rule from review thread r4296070770 and from commands.md is: the latest capture that ANSWERED carries the claim. A hung probe is not an observation, so the cutoff now reports still-visible when an answered capture named the alert, and unconfirmed only when none did — the 2482e24 smoke case, with no answer at all. The undecided-overrules-match case is unchanged: undecided is an answered observation. Pin flipped to the CI timeline (fail-before at f584's catch); all 128 alert-family tests and the 197-test provider-integration project pass at this working tree.
The independent review of f584e8a returned merge-ready with three follow-up-scale findings, batched here: - The documented `dispatched: 'unknown'` press-cutoff failure had no test. Pinned in the Android suite with the real transport shape: the adb fake hangs the tap until the 2s action window closes and dies with the signal; the failure must disclose unknown, never 'no'. Mutation-checked: flipping the disclosure to 'no' fails the pin. - The `[timeout]` positional is honored by `alert wait` only — the Apple acting legs pass fixed runner budgets and the tree legs run their own lookup/confirm windows; commands.md now says the argument is wait-only per backend instead of leaving the silence to surprise callers. - `void device;` in limrunAlertOperationFacts follows the `_device` convention; the two byte-identical visible-after-press exits collapse into one helper, keeping the two hints that genuinely differ (latest read hung vs budget ended unresolved). No behavior change for any message, reason, or tap.
60aec19 to
be48a28
Compare
|
Thanks for the update. Two issues from the earlier review at 482cff1 are fixed, and two defects remain at be48a28. CI is green on 20 checks, and the earlier Coverage failure on the confirm path is gone since 71087fb. I ran no tests and no devices, so the points below come from reading the code and your 957726b and f584 runs. Fixed since 482cff1: the spent-budget confirm exits, the confirm probe wrapper, the button dedup, the unranked tie check and the window cap are addressed at this head. First, the budget rule is still only half applied in https://github.com/callstack/agent-device/blob/be48a28/packages/platform-android/src/alert.ts#L270. A probe that started inside the window must finish. Only the first observation has that now. Second, accept now refuses where main pressed, in https://github.com/callstack/agent-device/blob/be48a28/packages/contracts/src/alert-contract.ts#L140. The shared tie check rejects any unranked candidate. A localized Android 11+ sheet ("Mientras la app está en uso" and "Solo esta vez") now returns null from Could this be simpler? Both findings come from the per-capture window: The open inline threads still stand for the shared tie check (#3699 (comment), which is the localized-sheet issue above) and for the After the fix, please rerun both proofs at the final head. On the Android emulator, with no alert on screen, Before merge, the tree-alert driver must consume every probe that started inside its window, so a quiet-screen accept or dismiss always answers |
The maintainer review at be48a28 named two defects and a design simplification; this applies all three. Budget rule (whole): the alert budget now bounds only how long a leg polls. pollUntilSettled awaits every probe it starts, the deadline ends the loop, and only the caller's cancellation stops a capture or press in flight. A quiet screen answers alert-not-found with dispatched:no on every backend, never a reason that depends on which capture crossed the deadline; the late positive that used to be refused is consumed and pressed, per the rule the maintainer asked for and this supersedes the earlier late-answer-refusal contract. Grant ranking: the shared choice boundary takes a backend rank first and the label vocabulary only where the backend declares nothing. Android declares the permissioncontroller grant ids (foreground-only, always, one-time), so a Spanish Android 11+ sheet and the Android 10 foreground copy resolve to the lasting grant instead of refusing; equal ranks and candidates no rank reaches keep refusing. Design: the driver moved to capture-kit/alert-tree-driver (Limrun no longer depends on platform-android), treeAlertWindow/arm/start, the delegating clock, the window-closed sentinel, lastCaptureNamesAlert, and the exception paths are gone; net production diff vs be48a28 is -161 lines. Docs and CHANGELOG state the chosen behavior.
There was a problem hiding this comment.
All reported issues were addressed across 15 files (changes from recent commits).
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…st-press seam, rank only on one scale Follow-up to the review at 5cfe029: - platform-android: load the shared tree-alert driver behind the alert entry's function-scoped import (types stay static), so mechanics.ts keeps its merge-base eager closure (177->176, the gate's owning-edge rule). - capture-kit driver: a caller cancel is typed dispatched:'unknown' from the press onward even when a non-abortable transport RESOLVES after the cancel or the cancel lands in the confirm window, and a cancel that rode a straddling lookup probe refuses cleanly (dispatched:'no', no press). - contracts: selectRankedAlertButtonByRole refuses pairs mixing backend id-ranks with label-table positions — different scales are not comparable; same-source conflicts still let the owning backend win. - tests: deterministic started-gated cancel pins (no unreachable mock branches), Limrun no-abort capability pins, backend-rank-override and mixed-scale pins at the contract seam, faithful vendor-id fixture. - docs: correct the CHANGELOG/commands/driver claims about Limrun cancellation (started calls run to their answer; the leg ends at the next seam cancel can reach), scope the quiet-screen guarantee to accept/dismiss, and limit the details.reason promise to driver refusals; fix the stale platform-android module comment.
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…confirm-never-started pin - platform-android/alert.ts: drop the false comparison to how the Limrun provider loads its own alert file; the comment now states only the real constraint — a static driver import would grow the mechanics facade past its merge-base eager-closure budget (ADR-0019). - commands.md: replace 'the next seam cancellation can reach' with what the reader observes: a Limrun request already sent is not stoppable; cancellation lands once that request answers; restarting the instance session clears a stall. Human-rule facts unchanged. - alert.test.ts: the resolves-after-cancel pin also asserts the leg ended before any confirm read started (elementTree exactly once).
…he stall docs exhaustive R6-review P3 follow-ups (report /tmp/3349-r6-review-report-3e1e.md): - capture-kit driver: a capture whose transport died with the caller's cancellation exits through observeTreeAlert, which is always pre-press (acting legs never press before the lookup returns) — it now carries the clean dispatched:'no' like the other pre-press exit instead of omitting the key; both backend pins assert it. Pins fail without the key. - capture-kit driver: correct the seam comment's attribution — the caller learns of its cancellation from its own client transport; the daemon's answer lands once the leg settles on the started probe's answer. - commands.md: the Limrun stall sentence enumerates the SDK's own ~30 s request timeout (verified against @limrun/api@0.64.1 sendRequest) as a route the unanswered request resolves on, instead of reading exhaustive.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…utes that end here R7 P3 / inline r4300817984: 'always a pre-press exit' overstated the catch's reach — confirmTreeAlertGone polls through the same observeTreeAlert, so a cancel mid-confirm-probe reaches it post-press (harmless: the actOnTreeAlert press/confirm catch re-types it to dispatched:'unknown', which owns that invariant). The comment now states where each typed answer is owned instead of asserting a false reachability fact. No behavior change.
|
Final live proofs at the certified head Cert chain for this head (all earlier heads superseded): exact gate Android (emulator-5554, own session
|
|
Reply to the review at 1. Budget rule applied whole. The per-capture window machinery is gone entirely — 2. Id-derived ranks instead of refusal. 3. Simpler shape, mostly as proposed. The driver moved out of 4. Limrun proof rerun at the final head, real CLI/daemon route. Fresh instance at 5. One open item, stated without clearance. The |
Summary
Closes #3349. Across 30 files, Limrun iOS
alert get/wait/accept/dismissuse a shared tree-alert driver. Started captures finish; Android permission IDs rank grants conservatively.agent-device alert accept --session qaselects an unambiguous affirmative action; dismiss refuses unsafe choices.Validation
Published head:
92df16aa70fb3d2812e94dd15e544b57abfffa62; mergeable, zero unresolved review threads.pnpm check:affected --run --head 92df16aa70fb3d2812e94dd15e544b57abfffa62:run-ktnH3cpassed, exit 0,exactCommit: true, unchanged clean checkout; 36 checks passed, 25 skipped, two deduplicated. Current CI: 21 successful checks, two intentional skips; Coverage, Integration and all four platform smoke lanes passed.alert-not-found/dispatched: no; a real location sheet selects While using, grants permissions, then returns none. Limrun CLI/daemon shows ATT visible → Allow → none; a logging loader records one accept tap, separately from the app-trigger tap. Raw responses and transport evidence and review replies are published; own proof daemons stopped and Limrun instances deleted. The shared emulator remains open.4c1c78696provider run passed 201 tests, direct exit 0; this does not explain the earlier five provider failures. Earlier iOS failure cause and PR participation remain unknown. SDK in-flight abort is unavailable; global-reset impact remains unknown. Chief-of-Staff readiness label absent.