Repository navigation
P1015: give each Toad app its own launch inputs and lifetime - #1365
Merged
Merged
Conversation
trissim
marked this pull request as ready for review
October 30, 2026 11:53
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
P1015 app isolation — root-selection checkpoint
5 commits including the main synchronization; 36 files, +115/-96 versus current main. The latest correction replaces consumer-owned root selection with the existing app access owner. This is not complete P1015 delivery.
Pair with agent-comms#3704 (P1026, approved Core head reported as 4cf224558). Never land the Core deletions against Toad main alone. The hidden-channel wait also consumes the public
VisibleReadMarking.read_workerin toad#1368 (published head reported as 3e7dac5); retain that change when integrating. No.markingcompatibility flag or private-worker probe.Before / after: route ownership
Before:
LaunchRoot.startupwas discarded, Toad reread the process singleton, andcapture(source)compared the source to an ambient route. The first version of this PR incorrectly claimed that passing the source removed global dependence; that claim is retracted.The next checkpoint used explicit
RouteSelection.of(app_root)but pasted the selection and source check throughout views, commands, paging and ACP. Three replacement checks drifted in their exceptions or retries. That was relocation, not factoring (IMPL-12; paper1 Theorems 4.6/4.7).After:
LaunchRootand passes it to the app's existingCoordinationAccess.CoordinationAccess.selectionis the one productionRouteSelection.ofsite.selected(source)owns source refusal;current(source)asks Core's declared instance method.confirm(selected)owns held-selection replacement checks; writes retain that sink validation..launch_root. Target discovery no longer resolves the route again through its separatecurrent()call or raises its own replacementValueError.WireSourceretains the access owner, not a second root-selection recipe. Dataclass replacement/restyling retains the same owner.Import correction
CoordinationAccessis imported at wire_history module scope, not added to its TYPE_CHECKING block. The prior guarded import was an avoidable new dependency copy; the reviewer verified the top-level import on the installed runtime. The author did not repeat that runtime check. Current main's NativeContextData consumer migrations are integrated, not replaced.Caller and guard preservation
All required ToadApp factories and helper callers are migrated, including the original seven named app/about sites. WireSource constructors receive real app access. The child-launch contract uses its explicit
launch_rootfixture, notLaunchRoot.current().The hidden-channel no-route-read probe is restored against
RouteSelection.root_is_current, preserving its scroll/tab-return assertions. Its obsolete.markingwait now awaits the public worker lifetime from #1368. No author test execution.Source evidence
git diff --checkpassed.git grep -n 'RouteSelection.capture\|root_is_current' srchas one result:comms_root.pydelegates to Core's retainedRouteSelection.root_is_currentinstance method. It has zerocapturecalls and zero removed free-helper imports. Calling that broad grep zero would be false.RouteSelection.ofhas one production site, in CoordinationAccess; no productionLaunchRoot.currentremains.Still open / acceptance owed by integration
No tests, collection, mypy, local NRA gate, runtime imports, build, install, provider input or live application run was performed by the author. No paired behavior or CI success is claimed. Root owns combined acceptance: two apps on different roots despite process-default changes; foreign-source and replaced-bus refusal; CLI startup/about; write sink validation; hidden-channel/tab-return and child-launch contracts.
The remaining P1015 steps are still open: AppPaths/settings/database instance ownership, process-owned heap freeze, child retirement, then deletion of interpreter-per-test isolation.
OwnProcessTestis deliberately retained until those owners are fixed. Reported 28-minute baseline is not an author measurement; the under-20-minute result and deleted isolation line count remain unmeasured.Assumed