Conversation
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 36 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
At 1376a9e, I found two problems that need fixing before this can merge. The Coverage check fails, and I think the failure comes from this diff. cleanupExpiredLeasedSession passes The Coverage job fails in Is one binder module the smaller shape here? It would export The PR body says live device evidence is still pending. On one Android emulator or iOS simulator, with a fresh daemon built from this head, please run these and paste the CLI output. First run Not blocking, take or leave: I read the diff but did not run the mutation controls from the PR body. I also did not confirm a production flow where a leased session is cwd-scoped. I only confirmed that Before merge, please move the three binders into |
d0f7cf8 to
097ab95
Compare
c4bd3ea to
75c680a
Compare
712e7da to
bef2ebb
Compare
17ba57f to
480429a
Compare
|
The earlier consolidation of the teardown imports into session-capture-binding.ts looks good, but one defect and the missing device evidence from the review at 1376a9e still remain at 480429a. request-execution-scope.ts:421 is unchanged. The live device evidence asked for at 1376a9e is still missing, and the PR body still calls it pending. Please run these on one Android emulator or iOS simulator, with a fresh daemon built from this head, and paste the CLI output. First: Not blocking: The cubic-dev-ai thread on the expiry lookup still applies (#3137 (comment)). It is marked fixed in #3139, but line 421 at this head still looks up the public name and returns early. Checks for 480429a are queued and the earlier runs were cancelled, so CI has not reported yet. The Coverage eager-closure failure at 1376a9e came from the session-teardown.ts imports. This commit moves them into session-capture-binding.ts, but I have not run the eager-closure budget test, so please confirm Coverage passes. There are no conflicts. Before merge, the lease-expiry SessionRef fix must be in this PR or the PR body must say the two land together, and the three live runs must be pasted. I could not re-read lease-lifecycle at this head and relied on the 1376a9e finding that |
480429a to
fd483da
Compare
696ea9f to
2652dd2
Compare
Session teardown imported three per-capture binding modules plus their shared binding, which grew its eager closure past the merge-base. The audio, perf and screen-recording bindings now live beside bindSessionCapture.
2652dd2 to
cbf0a6e
Compare
fd483da to
e0a818c
Compare
|
Consolidated into #3135 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. |
|
Summary
Capture finishing and forced cleanup now use a lifetime-bound session binding. The coordinator keeps the captured handle/fence across native awaits; clearing checks that the same lifetime and resource still occupy the slot. A rebuilt record keeps its unrelated changes.
Capture-kit no longer accepts a whole session record or
sessionSlot.replaceprojection. Daemon field owners provide explicit slot updates. Close, shutdown, audio, perf, app-log and record callers use the shared finish operations; shutdown and record-only removal retire captured lifetimes.Depends on #3136. Ref #3116. Thirty-six files, 661 gross changed lines. Recording/journal and remaining session writers migrate next.
Validation
Tested
1376a9e26c: quick checks, Fallow and exact-head affected checks pass, including 277 related files / 1,726 tests. All 73 focused capture/teardown tests pass.Removing clear, bypassing handle comparison or using address-only clearance each fails a held-finish control; restored code passes 19/19. R68 rejects former package writers and allows the daemon owners. Independent read-only review has no findings. CI-owned coverage/provider and live device evidence remain pending.