Skip to content

fix: bind shutdown hints and touch probes to session lifetimes - #3170

Closed
thymikee wants to merge 47 commits into
chore/session-state-write-ownershipfrom
fix/session-shutdown-review
Closed

thymikee wants to merge 47 commits into
chore/session-state-write-ownershipfrom
fix/session-shutdown-review

Conversation

@thymikee

@thymikee thymikee commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Summary

Shutdown now captures runtime hints from the session's stored address before any cleanup await. The owning teardown helper supplies those values to application cleanup and refuses a replacement lifetime's hints. This covers implicit, cwd-scoped sessions whose public name differs from their address.

Optional point-probe liveness checks now live inside the probe helper. A retired session cannot start a probe or emit a late failure warning; a live failure still reports diagnostics.

Follows #3154; addresses its shutdown and probe review findings for #3116. Five files changed, 108 additions and 37 deletions.

Validation

Tested 07a82a682374f970abb80edac24330ac582881a7:

  • Four regression cases failed before the fixes; 28 focused tests pass afterward.
  • Three planted mutations fail: public-name hint lookup, successor hint adoption, and retired-probe warnings.
  • pnpm check:quick and pnpm check:affected --base chore/session-state-write-ownership --run pass; 962 tests across 171 files.
  • Independent review found no actionable findings.

Full pnpm check passes: 12,241 unit tests, tooling, packaging, leak checks and smoke controls. The implicit-session iOS shutdown control passes: correct scoped hints, unchanged daemon lifetime, both native preferences removed, and owned claim/runner cleanup complete. GitHub checks remain authoritative for CI. Adversarial tests establish the lifetime contract without claiming every replacement schedule is reachable through ordinary locked requests.

@thymikee thymikee changed the title fix: centralize scoped shutdown hints and touch probe liveness fix: bind shutdown hints and touch probes to session lifetimes Oct 3, 2026

@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 5 files

Re-trigger cubic

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.96 MB 4.96 MB -21 B
Package (unpacked) 4.96 MB 4.96 MB -21 B
Package (download) 1.49 MB 1.49 MB -3 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.6 ms 26.8 ms +0.1 ms
CLI --help 82.0 ms 82.2 ms +0.2 ms

@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

This PR is ready at 07a82a6. The code is correct, and the focused-test and mutation evidence in the PR body covers the new behavior. No conflicts.

Not blocking, and you can take or leave these. First, the touch probe in interaction-touch-reference-frame.ts checks the session lifetime only on entry and in the catch, so after the awaits at lines 38 and 52 it can still call setTouchReferenceFrame on a recording from a retired session; checking sessionStore.resolveCurrent(ref) again before each write would give the probe one rule, that every side effect needs the captured lifetime to still be current. Second, the PR summary calls scoped-address hint capture for implicit sessions a fix, but the base already read hints from ref.address, so the real change in daemon-runtime.ts is that the helper now owns the capture and refuses a retired lifetime's hints.

The checks are red, and I think the failures are unrelated to this diff. The TS2883 errors are in app-log-session-resource.ts, screen-recording-session-binding.ts and session-capture-binding.ts, which this PR does not touch. They come from DurableCaptureSlotClearResult, which exists in the base stack (#3155/#3154) but not on main, and the build continued past them. The iOS smoke failure is wait text Automation lab in smoke:automation-input, stopped at runner-start readiness, and this diff does not run during a wait step.

Before merge, please rerun the iOS smoke job. The TS2883 errors need a fix on the #3155 base branch.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 3, 2026
@thymikee
thymikee added this pull request to stack #3146 October 3, 2026 13:46
@thymikee

thymikee commented Oct 3, 2026 •

Copy link
Copy Markdown
Member Author

The first iOS Smoke attempt at 07a82a6823 stopped during deep-link navigation. open reported success, but the waits never verified “Automation lab”; later observation failed when the app was absent and runner startup exhausted its budget. The failure screenshot still shows the system Open in “Agent Device Tester”? prompt.

This establishes where the run stalled. It does not prove why the prompt remained or identify a regression in either ownership change. The artifact records no interaction commands, and its cleanup close does not establish that the daemon shutdown-hints path ran.

Failed attempt. The unchanged-head rerun passed at 07a82a6823. No timeout or retry policy was changed. The exact-head iOS check is green; that result does not establish the first failure's cause.

Deep-link confirmation prompt on the failed iOS Smoke run

@thymikee
thymikee force-pushed the chore/session-state-write-ownership branch from 54e813a to 1be1932 Compare October 3, 2026 14:41
@thymikee
thymikee force-pushed the fix/session-shutdown-review branch from 07a82a6 to d3d0c57 Compare October 3, 2026 14:42
@thymikee
thymikee force-pushed the chore/session-state-write-ownership branch from 1be1932 to 99e8d0c Compare October 3, 2026 15:16
@thymikee
thymikee force-pushed the fix/session-shutdown-review branch from d3d0c57 to 7e46a5a Compare October 3, 2026 15:16
@thymikee
thymikee force-pushed the fix/session-shutdown-review branch from 7e46a5a to e4ea909 Compare October 3, 2026 16:41
@thymikee
thymikee force-pushed the chore/session-state-write-ownership branch from 801a4ba to 61ef999 Compare October 3, 2026 16:56
@thymikee
thymikee force-pushed the fix/session-shutdown-review branch 2 times, most recently from 408192c to 94807d3 Compare October 3, 2026 17:49
@thymikee
thymikee force-pushed the chore/session-state-write-ownership branch from 61ef999 to 624321d Compare October 3, 2026 17:49
@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

The delayed probe finding is addressed in #3174 at 09bd6bb6d2: one private rememberFrame function checks both the captured lifetime and exact recording resource before either awaited probe writes. A rebuild of the same lifetime/resource stays valid. Eight owning controls and two concrete mutations distinguish stale publication from a harmless early return; Android visual proof reaches the actual size-probe route.

Your shutdown-summary correction is right. The base already looked up hints using ref.address; this layer makes the teardown helper own capture and refuse a retired lifetime's hints. It does not introduce that scoped lookup.

The parent fixes declaration generation with an inline clear-result union. The follow-up removes the redundant type export that became invalid during rebasing. Exact-head affected checks now pass, including declaration generation and 971 related tests.

The unchanged-head iOS rerun passed at the original 07a82a6823, as recorded in the earlier evidence. Current-head native CI remains separate and pending.

@thymikee
thymikee removed this pull request from stack #3146 October 3, 2026 19:38
@thymikee
thymikee force-pushed the chore/session-state-write-ownership branch from 624321d to a3e7316 Compare October 3, 2026 19:39
@thymikee
thymikee force-pushed the fix/session-shutdown-review branch from 94807d3 to 111f50f 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 force-pushed the chore/session-state-write-ownership branch from a3e7316 to 17b90d1 Compare October 3, 2026 21:00
@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Consolidated into #3144 as part of reducing #3116 to seven PRs. The composition preserves the complete pre-consolidation source tree, including tests and later review corrections. This PR is superseded; its review discussion and native evidence remain available. Outstanding findings transfer to the owning keeper in the implementation record.

@thymikee thymikee closed this Oct 3, 2026
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