Skip to content

fix: preserve session lifetimes through open, replay and shutdown - #3144

Merged
thymikee merged 8 commits into
refactor/session-journal-lifetimesfrom
fix/replay-observation-lifetimes
Oct 4, 2026
Merged

thymikee merged 8 commits into
refactor/session-journal-lifetimesfrom
fix/replay-observation-lifetimes

Conversation

@thymikee

@thymikee thymikee commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Summary

Preserve captured session lifetimes through open, replay, interactions and shutdown. Alert and settings now share the optional action recorder and skip journals after retirement; delete their duplicate recorder. Snapshot/interaction finalizers retain strict readiness checks.

Refuse Apple open success when a recognized URL confirmation remains after failed acceptance, even if the app runs behind it. Help describes this failure and the English-title plus Cancel/Open recognition rule. Consolidates eight original layers; depends on #3140. Related to #3116.

Validation

62d6af6340: pnpm check:affected --run passed 13,443 tests with one skipped, twelve documentation controls and selected tooling. All 52 focused recorder/prompt controls pass. Both alert regressions were red; clipboard and first-read button-shape mutants were red.

iOS smoke at unchanged prompt code reaches cold URL open and subsequent destination reads. Fresh CI is pending. Dedicated local prompt verification captured exactly Cancel/Open but its first native accept timed out and correctly refused success; positive accepted-response proof remains pending. Physical recording-health remains blocked by Xcode signing. User handles merges.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.97 MB 4.98 MB +2.7 kB
Package (unpacked) 4.97 MB 4.98 MB +2.7 kB
Package (download) 1.49 MB 1.49 MB +862 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.2 ms 26.8 ms -1.4 ms
CLI --help 83.6 ms 83.1 ms -0.5 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Re-trigger cubic

@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Thanks for the PR. The code reads correctly at 5c349ba, but it needs one live run before merge. All 14 of 14 checks pass at this head, and I know of no conflicts.

This PR changes the production route for replay divergence capture and for ref publication. Finalization used to refuse any rebuilt record. Now it publishes into the latest record of the same lifetime, see session-replay-command.ts. Refs that were suppressed as stale-capture before can now be published on a live replay. Only mocked-dispatch fixtures cover this route, and the PR body says live provider evidence is still pending. Those tests cannot show that a divergence ref published on a real device works in the next command. Please do one run at 5c349ba on an iOS simulator or an Android device. Run a replay where a step diverges because its selector is absent. The output must show divergence.screen.state: available with refs and no stale-capture or refs-suppressed reason. Then a get @eN or click @eN in the same session, using a ref from that divergence screen, must succeed. A plain snapshot run does not reach this route.

Not blocking, take or leave: signal is now passed to captureSnapshot, but the only caller binds without one, so drop it or add a caller that uses it. The ref ? store.requireCurrent(ref) : undefined line throws before the NO_ACTIVE_SESSION branch for a retired ref, so that branch only covers no session at bind. updateSession, assertSelectorMatches and getRuntimeHints in createReplaySession still resolve by address, which fits the next #3116 slice.

I read the merge-base and 8fc9fe9 code paths but did not run the tests or the planted-regression mutations. I also did not check whether every open or replacement path could carry the old snapshot, generation, runtime revision and refFrame together in a same-entry set. The lineage check needs at least one of these to change. Before merge, the live run above must show published divergence refs that a follow-up @eN command can use.

@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Completed the requested live control at exact head 5c349ba931 on the owned Android emulator (emulator-5580), after pnpm build and pnpm build:android.

The helper control reported androidSnapshot.backend: "android-helper", helperVersion: "0.21.18", helperTransport: "persistent-session".

Ran a native script containing click "label=ownership_3116_absent" against the active Settings session. It failed as intended after three selector polls. The actual replay response contained:

{
  "code": "REPLAY_DIVERGENCE",
  "reason": "selector_not_found",
  "screen": {
    "state": "available",
    "refsGeneration": 206361,
    "refs": [{"ref": "e11", "role": "text", "label": "Network & internet"}]
  }
}

Without another snapshot, the separate command get text @e11 --platform android --state-dir /private/tmp/3116-live-replay --json succeeded on that same implicit session and returned Network & internet. This proves publication through the divergence route and subsequent ref authority, rather than ordinary snapshot behavior. The run used only the owned target and isolated daemon state; cleanup followed.

@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Thanks for the live run at 5c349ba. It covers the route I asked about: the replay divergence returned screen.state: available with refs, and get text @e11 then worked in the same session with no new snapshot. The code verdict stays clean, and there are no conflicts, so this is ready for human review.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 3, 2026
@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch 2 times, most recently from 10ff582 to f36994c Compare October 3, 2026 15:16
@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch 2 times, most recently from 974356e to b08a3f6 Compare October 3, 2026 16:56
@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch from b08a3f6 to c0cb32f Compare October 3, 2026 17:49
@thymikee
thymikee removed this pull request from stack #3146 October 3, 2026 19:38
@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch from c0cb32f to 0e24d48 Compare October 3, 2026 19:39
@thymikee
thymikee added this pull request to stack #3187 October 3, 2026 19:45
@thymikee
thymikee removed this pull request from stack #3187 October 3, 2026 20:59
@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 3, 2026
@thymikee thymikee changed the title fix: bind replay capture and publication to one lifetime fix: preserve session lifetimes through open, replay and shutdown Oct 3, 2026
@thymikee
thymikee changed the base branch from fix/request-session-health to refactor/session-journal-lifetimes October 3, 2026 20:59
@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch from 0e24d48 to 7f2e379 Compare October 3, 2026 21:00
@thymikee
thymikee added this pull request to stack #3196 October 3, 2026 21:03
@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch from 7857480 to 57c2f32 Compare October 4, 2026 07:00
@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch from 57c2f32 to a6b53be Compare October 4, 2026 07:28
@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

The earlier concern is addressed: the lifetime changes through open, replay and shutdown now look correct at 57c2f32, but one gap in the iOS launch-prompt route still needs live proof. Commit 75ea97f changes how open --launch-url settles on iOS. isLaunchConfirmation now also requires the runner items to be exactly ['Cancel','Open'], and this gates the first accept path as well as the new still-present check. Only mocked interactor ports cover this. Nothing at this head proves the real runner reports the alert buttons in that shape. If it does not, every launch prompt becomes alert-unrecognized and open returns green behind the prompt again, which is the failure the iOS smoke run hit before. Could you run one live check on an iOS simulator at 57c2f32? Run open <app> --launch-url <custom-scheme URL> with a URL that raises the 'Open in “…”?' prompt. The output must show data.launchConfirmation: "accepted", followed by a successful read of the app. A green iOS Smoke Tests job at this head that reaches the same open-URL step also counts. A plain open without the prompt does not reach this route. Not blocking: the open help in app.ts and commands.md still say launch_confirmation_unanswered happens only when the app is provably not running after a second hand-off, while open-policy.ts now also fails when a recognized prompt stays visible after a failed accept, so you may want both to describe every condition and the recognition rule, or leave them. I did not run any tests here, and CI is still pending, with no failure to attribute yet. A red iOS Smoke Tests job at this head is likely related to this change, because it runs open with a launch URL through the route above. Before merge, we need the live iOS proof or a green iOS smoke run at 57c2f32.

The head moved to a6b53be after this review. The new commit comes from the base branch (it joins daemon dispatches before shutdown retires session admission), so this verdict still describes 57c2f32. I will check the new head separately.

@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch from a6b53be to d652aef Compare October 4, 2026 08:14
@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

The requested live prompt evidence remains pending; the mocked controls do not establish the real runner button shape. I am tracking the current iOS smoke job and will only count it if it reaches the URL open and subsequent app read. Historical failed runs do not count as passing proof.

The help and command documentation now describe both launch_confirmation_unanswered outcomes: the app still not running after the second handoff, and a recognized prompt remaining visible after failed acceptance even when the app runs behind it. They also state the English title plus Cancel/Open button recognition rule. The documentation commit is 3d1ce8da5d; both dependent branches are rebased and pushed. Final-head local gates and fresh GitHub/native checks remain separately tracked.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 90 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/daemon/request-generic-dispatch.ts
Comment thread src/daemon/selector-recording.ts Outdated
Comment thread src/daemon/handlers/snapshot-settings.ts Outdated
Comment thread src/daemon/snapshot-session.ts Outdated
Comment thread src/daemon/handlers/snapshot-alert.ts Outdated
Comment thread src/daemon/interaction/internal/interaction-runtime.ts
Comment thread packages/platform-apple/src/launch-confirmation.ts Outdated
@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-3144/

Built to branch gh-pages at 2026-10-04 10:10 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@thymikee
thymikee force-pushed the fix/replay-observation-lifetimes branch from 1551ae2 to 8039c90 Compare October 4, 2026 08:50
@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Thanks for the update. The delta at 8039c90 fixes the selector recorder (r4176770576) and the launch-confirmation diagnostic (r4176770608), but one optional recorder still throws when a session lifetime ends mid-request.

The two guarded recorders, session-action-recorder.ts:120 and selector-recording.ts:140, now return when resolveCurrent(ref) is undefined. The third one, recordIfSession in snapshot-session.ts, still calls sessionStore.recordAction(ref), which calls requireCurrent. Alert accept/dismiss (snapshot-alert.ts:150) and the settings handlers (snapshot-settings.ts:255 and :300) use it. If the ref retires during bound.execute, a completed alert action or settings read comes back as session_lifetime_ended. The base code did not throw here, so this PR adds the throw. A client retry would then accept or dismiss the alert a second time. This also contradicts the PR body, which says optional recorders skip ended lifetimes centrally, and the three resolved threads below were closed on the claim that these callers inherit the fix. Could we make this one rule with one owner: every optional recorder returns without writing when resolveCurrent(ref) is undefined, and only the session-close-script and open-execution writers stay strict? The simplest shape may be to delete this recordIfSession and route alert and settings through recordSessionAction. Please add one alert or settings handler test that retires inside execute and expects ok: true with empty journals.

Not blocking, and you can take or leave these: d652aef adds the guard to recordSessionAction but no test covers it, so a handler-level control (for example a clipboard handler that retires during execute, expecting ok: true and empty retired and successor journals) would pin it, and isLaunchConfirmation in launch-confirmation.ts:135 now needs exactly ['Cancel','Open'] on the first read, but only the post-failure re-read has a test, so one answerLaunchConfirmation case with a matching title and other buttons expecting alert-unrecognized and no accept would cover it.

On the open threads, r4176770587, r4176770590 and r4176770595 still apply even though they are marked resolved (all three point at the unguarded recordIfSession above): #3144 (comment), #3144 (comment), #3144 (comment). These do not apply and can stay resolved: #3144 (comment) and #3144 (comment) (the finalizer keeps requireCurrent on purpose because it owns readiness), and #3144 (comment) and #3144 (comment) (both fixed at this head).

I did not run local tests, and I did not trace the claim that existing-session locks keep retirement out of a request through request-binding.ts and daemon-session-idle-expiry.ts. The red-without-fix claims rest on reading the pre-fix code. I did not check whether the open timeout covers two extra 10s alert reads on the failed-accept route.

Smoke Tests was still running when I checked, so CI is pending. I know of no conflicts.

Before merge, please fix and test recordIfSession. Please also post a live run on a booted iOS Simulator at 8039c90: agent-device open <session app> --launch-url <scheme the session app owns> --platform ios --json, started from a state where the URL raises the "Open in App?" prompt before the app is observable. The output should show data.launchConfirmation == "accepted", plus an alert get --json taken before the accept with items exactly ["Cancel","Open"]. That shows the new button check still matches the real SpringBoard prompt.

@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

The remaining optional recorder was real: my earlier replies overstated coverage. It is now fixed at 62d6af6340: alert/settings call recordSessionAction, and the duplicate snapshot-session.recordIfSession is deleted. Both alert handler controls retire the admitted lifetime during native execution and prove success, one dispatch, and empty retired/successor journals. Both were red before the fix. The clipboard guard and first-read button-shape controls also reject planted mutants. All 52 focused controls pass.

The exact-head affected gate passed 13,443 tests with one skipped, twelve documentation controls and selected tooling. #3152/#3155 are rebased and pushed with their own passing gates. Fresh CI remains pending.

Native verification at this head captured the real SpringBoard title and exactly ["Cancel","Open"] before acceptance. Two controlled attempts, including a fresh visible simulator, returned launch_confirmation_unanswered; the post-failure screen shows the destination running behind the same visible prompt. The new guard correctly refuses false success. This is not positive launchConfirmation: accepted proof, and I am not claiming readiness. These attempts pre-raised the prompt with simctl and then the CLI sent its URL, so they include a pending handoff plus a second dispatch; no attribution to a new native regression is established. The single-dispatch accepted-response requirement remains open.

Screenshots show the fresh attempt before and after. JSON files retain the raw alert and refusal response. The isolated session/daemon/device are being cleaned up. Physical recording-health remains blocked by signing.

Observed alert (before acceptance):

{
  "success": true,
  "data": {
    "message": "Open in \u201cAgent Device Tester\u201d?",
    "items": [
      "Cancel",
      "Open"
    ]
  }
}

Open result:

{
  "success": false,
  "error": {
    "code": "COMMAND_FAILED",
    "details": {
      "reason": "launch_confirmation_unanswered",
      "appBundleId": "com.callstack.agentdevicelab",
      "dispatched": "unknown"
    }
  }
}

SpringBoard prompt before acceptance

Destination behind the still-visible prompt after refusal

@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Thanks for the update. The alert and clipboard recording fixes at 62d6af6 look right, but the live iOS evidence is still missing.

The alert, clipboard and settings handlers now record through the guarded recordSessionAction, and recordIfSession is gone. The alert test covers a session retiring during accept and dismiss. I ran no local tests, so I did not confirm that the new alert and clipboard tests fail without the fix. The settings handlers share the same helper but have no retire-during-execute test of their own.

The live run requested earlier has not happened yet (launch-confirmation.ts#L135). Your two native attempts raised the prompt with simctl first, then had the CLI dispatch the URL a second time. That is a different route with two hand-offs, and both runs returned launch_confirmation_unanswered with the app running behind a visible prompt. No run has reached the single-dispatch path, where the first read matches Cancel and Open, the accept succeeds, and open reports success. If accept fails on the real route too, every iOS open --launch-url that raises the prompt will fail with launch_confirmation_unanswered, where it used to succeed. Please run exactly one agent-device open <app> --launch-url <scheme the app owns> --platform ios --json on a booted simulator at this head, with the app installed and no prompt raised beforehand. Please capture data.launchConfirmation == "accepted", then run a snapshot or alert get that succeeds and shows no prompt. I could not tell whether your earlier result comes from the double hand-off setup or from a real accept failure. A green iOS Smoke Tests job at 62d6af6 that reaches its open-URL step also counts.

Of the earlier inline threads, none of the P1 or P2 threads still apply. These are done or intentional, so please resolve them: snapshot-settings, recordIfSession, snapshot-alert and selector-recording are fixed at this head. request-generic-dispatch and interaction-runtime are intentional, because the finalizer keeps requireCurrent on purpose and those files are not in this delta. The launch-confirmation P3 was fixed at 8039c90 and is unchanged.

Smoke Tests is still running. It exercises iOS open with a launch URL through launch-confirmation.ts and open-policy.ts, which this PR changes, so a red result on that step would likely be related. No conflicts are known. Merge waits on the single-dispatch run above or a green Smoke Tests job at 62d6af6.

@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

The iOS Smoke Tests job at 62d6af6 is green, and it covers the open-URL route I asked about. Its live fixture scenario runs open <app> --relaunch --launch-url <deep link> and then checks that the app received the deep-link event, so open --launch-url succeeds through the changed launch-confirmation.ts path (job log). That settles the missing evidence from my last comment. All checks pass and there are no conflicts, so this is ready for human review. Please still resolve the earlier inline threads I listed as fixed or intentional.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 4, 2026
@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Verified the accepted alternative at exact head 62d6af63405b67fc8cb7cbc5724df63c760808c0: iOS Smoke Tests run 37194388310 completed successfully. Artifact ios-artifacts (11301510447) records open --relaunch --launch-url agent-device-test-app:///automation?..., destination wait, and reads of automation-event-name and automation-event-payload, all with status 0. The later snapshot, interaction, and close steps also complete.

This verifies the ordinary single-dispatch route required by your latest review. The artifact does not separately retain the data.launchConfirmation response field; its step-history accepted flag is the scenario acceptance flag, not that response field. I am retaining that limit and the earlier double-handoff refusal screenshots at their original attribution. No additional simulator retry or relaxed acceptance guard is needed. The listed inline threads are already resolved. Physical recording-health verification remains blocked by signing.

@thymikee
thymikee merged commit b443126 into main Oct 4, 2026
21 checks passed
@thymikee
thymikee deleted the fix/replay-observation-lifetimes branch October 4, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant