✨ Launch native Agent sessions in independent terminal panes (#731) - #741
Draft
taras wants to merge 4 commits into
Draft
✨ Launch native Agent sessions in independent terminal panes (#731)#741taras wants to merge 4 commits into
taras wants to merge 4 commits into
Conversation
A `<Session.Launch>` written at the root takes the run's one foreground terminal, so native UIs are sequential. Inside a `<Terminal>` that would defeat the point of a grid, where every pane is interactive at the same time. So core installs a native launcher in each paired pane's scope, closed over that pane's claim. `<Session.Launch>` finds it by being written there: it is handed no pane, ordinal, token or mode, and its request, result and retained phases are the ones a root launch would have. What changes is which terminal answers `reserve` and `flush` — the pane's, through its claim, so two panes do not contend and one pane admits one live launch at a time. A pane also flushes what it has rendered before the UI draws over it, which is the root rule in the one place a pane's text goes. Readiness now has a boundary a launch can report. `NativeLauncherHandler.launch` takes the runtime's child-start event as a parameter — not a request member, not a context, not a result — and the foreground launcher reports it from the child's own `spawn` event, before it waits for the exit. The pane launcher listens and trips its claim's latch there and nowhere else: preparation, the reservation, the flush and an allocated PID are not a start, and a child that never ran never reports one. `nativeLaunch()` is unchanged for adapters, which hear nothing about the start. Terminal ownership and Agent-session ownership stay independent. Nothing pane- derived enters the coordinator key, the launch request, the retained record or a diagnostic, and two panes naming one logical session still contend through the existing non-waiting coordinator. No tmux, no new Agent advertisement, and root launch behavior is unchanged. Evidence: SP1–SP5 in the core launch suite (pane lease, concurrency, readiness, a failure before the start, one-live-launch-per-pane), FL8–FL9 in the runtime launcher (the start event, and a child that never starts), and Tier GN over the checked-in journey `TerminalGridNativeLaunch.test.md` through the whole TestAgent stack. Removing the pane launcher fails SP1–SP4 and GN1–GN4; never reporting readiness fails SP1, SP2, SP3 and SP5.
| // The runtime's own start event, and the only thing reported as one. A | ||
| // spawn that fails emits `error` instead, so a child that never ran never | ||
| // reports having started. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // reports having started. |
PR #741: ✨ Launch native Agent sessions in independent terminal panes (#731)12 files, +1508 / -34 Scope🔴 PR has 1542 lines changed. Split into focused PRs. 🟡 1542 lines changed. PRs under 400 receive more thorough review. StructuralOxlint structural signals:
Slop
Oxlint slop signals:
Static AnalysisOxlint: 51 diagnostics across 3 files (15 rules) no-unused-vars (10): packages/core/src/expand.ts, packages/acp/src/provider.ts CorrectnessNo extraneous code patterns detected. |
No production change. The pane-scoped launcher, the runtime spawn callback, the authority boundaries and root-launch behavior are exactly as reviewed. The checked-in journey is now the 2×2 grid TG5 asks for: three native Agent sessions and the host's default shell. All four children report their start before the composite attaches, each waits for its siblings while holding its own pane, and the row reads back the authored row-major positions and forms. Four rows added, all driven by signals this run produced: - GN7: after attachment one native UI exits nonzero while its sibling is live. Only that pane fails; the sibling is observed alive on the far side of the failure and stops only when the reader leaves; the grid ends on the pane that failed, and the close's cancellation is not a second failure. - GN8: the reader leaves with both launches live. Both are cancelled where they stood, neither pane fails, the composite comes down — and a root launch after the grid, naming a session a pane held, proves both leases came back. Which refusal it gets is the proof: not "already holds this run's terminal", not "another owner is using session", but the #517 recovery tombstone a cancelled native UI leaves behind. - GN9: a pane admits its next user only once the last one is wholly done, with the launch and the prompt that follows it going through the real coordinator. - GN10: a grid interrupted with a live pane launch, resumed on the same journal. It rebuilds the composite, starts the native child on the identity the first attempt retained, prepares nothing, and the retained record comes back unchanged — identity, route, binding and phase alike. SP5 now proves the pane stays held through both halves: refused while the child is live, refused again once the child has gone but the lease around it is still unwinding, admitted only after both. A launch that merely returned showed only the first. Two harness repairs. Pane states are read as a set of panes rather than a count of messages — a pane still live when the reader leaves is told twice, once from the outcome close decided and once from its own settlement, and that is display rather than a second settlement. And a generated variant is written to a directory of its own with copies of the scenarios it names, so a killed run leaves nothing in the repository.
| // The runtime's own start event, and the only thing reported as one. A | ||
| // spawn that fails emits `error` instead, so a child that never ran never | ||
| // reports having started. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // reports having started. |
The harness's interrupted branch built a `Result` as an object literal and cast it. Effection has a constructor for exactly that, so it uses it: no cast, and the type is the constructor's rather than an assertion's. Behavior and evidence are unchanged.
| // The runtime's own start event, and the only thing reported as one. A | ||
| // spawn that fails emits `error` instead, so a child that never ran never | ||
| // reports having started. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // reports having started. |
An orderly cancellation that finished proves everything a normal return proves, and must say so. It did not: `ownership.quiesced()` was a statement after `authority.perform()`, and cancellation unwinds past every statement after the operation it cancels. A reader closing a terminal grid therefore left every session its panes had launched carrying a recovery tombstone, and the next owner was told to recover a session nothing was using. The launch now runs in a scope the ownership body owns, and the acknowledgement is that scope's cleanup — reached on every path there is, cancellation included. It brings the launch down deliberately and reads the outcome of doing so, so the two facts it needs are facts rather than inferences: the native child and its cleanup settled, and this provider holds no handle for the session. A teardown that could not prove the child stopped throws out of `destroy()` and is not quiescence — and is still a failure, so it propagates rather than passing quietly. Nothing grid-specific reaches the provider. Reader close is the ordinary launch cancellation path, and this is the ordinary launch cancellation path's rule. The conservative cases keep their tombstone: a detach that failed or a session prepared and never handed over leaves a handle, and a child or provider cleanup that failed leaves the acknowledgement unmade. Cancellation, a released lease, a PID and elapsed time still prove nothing on their own. CX1 asserted the behavior this replaces — that a cancelled launch stays owned — so it now asserts the accepted one. CX2 is new and holds the other half: a cleanup that could not finish withholds quiescence, and the record stays active. GN8 is rebuilt as directed: two pane children held on unresolved operations, signals from each child's own teardown, teardown proven to finish after both, and a root launch afterwards on one of the same logical sessions that acquires ownership and starts — receiving neither session-busy nor session-recovery-required, and reclaiming the root foreground lease as it goes. Broken on purpose and re-run: acknowledging only on a normal return fails CX1 and GN8; acknowledging without proving the cleanup settled fails CX2, and only CX2.
| // the reader closing a terminal grid is one — unwinds past every | ||
| // statement after it, so a decision written down here would never be | ||
| // reached; written as this scope's cleanup, it is reached on every | ||
| // path there is. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // path there is. |
| // prepared and never handed over, leaves one. Either one | ||
| // missing leaves the session owned rather than looking | ||
| // finished, which is what the next owner is told to recover | ||
| // deliberately. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // deliberately. |
| // A `<Session.Launch>` written in this pane finds this launcher simply | ||
| // by being here: it reserves and flushes this pane instead of competing | ||
| // for the run's one foreground lease, and the child it starts is what | ||
| // makes this pane ready. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // makes this pane ready. |
| // The runtime's own start event, and the only thing reported as one. A | ||
| // spawn that fails emits `error` instead, so a child that never ran never | ||
| // reports having started. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // reports having started. |
3 tasks
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes#731. Third implementation Story under Quest #717, stacked on #738.
Base is
agent/issue-730-terminal-execution, notmain, as the Story's verification stack directs. Branched fromb2f826ac002eca28813526e18acbd6861b3f8c89, the accepted head of #738.Why
A
<Session.Launch>at the root takes the run's one foreground-terminal lease, so native UIs are sequential by construction — the second waits for the first to close. Inside a<Terminal>that defeats the point of a grid, where every pane stays interactive at the same time. This Story lets an Implementor, a Planner and a reviewer sit in three panes at once, without weakening either of the two ownerships involved.What changes
Before: a
<Session.Launch>written inside a paired<Terminal>asked for the run's foreground lease, which the grid was already holding, and was refused.After: it uses the pane. Panes launch concurrently; one pane admits one live launch at a time; sequential use of a pane is ordinary composition.
<Terminal.Grid columns={2}> <Terminaltitle="Planner"> <Session.Launch session="planner">You are the repository planner.</Session.Launch> </Terminal> <Terminaltitle="Implementor"> <Session.Launch session="implementor">You are the repository implementor.</Session.Launch> </Terminal> <Terminaltitle="Reviewer"> <Session.Launch session="reviewer">You are the repository reviewer.</Session.Launch> </Terminal> <Terminaltitle="Shell" /> </Terminal.Grid>The launch itself is unchanged. It receives no pane prop, token, identifier or mode; its
AgentLaunchRequest, its result and its retainedagent_session_launchphases are the ones a root launch would have.How it works
packages/core/src/terminal/pane-launcher.tsis the whole mechanism: middleware on the existingNativeLauncherApi, installed in the pane's scope and closed over that pane's claim.reserveandflushare answered here and deliberately not delegated — delegating would ask for the root lease the grid is holding.launchdelegates the exact request onward and only wraps the start report.Readiness needed a boundary a launch could report, so
NativeLauncherHandler.launchnow takes the runtime's child-start event as a parameter — not a request member, not a context, not a return value, which is what the spec requires of the latch. The foreground launcher reports it from the child's ownspawnevent, before it waits for the exit.nativeLaunch()is unchanged for provider adapters, which hear nothing about the start and so cannot fake one.A paired pane also flushes what it has rendered so far before the UI draws over it. That is the root flush rule applied in the one place a pane's text goes:
composite.display(ordinal, …).Review guide
Start with:
packages/test-agent/src/TerminalGridNativeLaunch.test.md— the authored journey: a 2×2 grid of three native Agent sessions and the host's default shell.Then review:
packages/core/src/terminal/pane-launcher.ts— the whole delivered mechanism.packages/runtime/launcher.ts— the start event, and where the foreground launcher reports it.packages/core/src/expand.ts—paneWork(), where the launcher and the pane flush are installed.packages/test-agent/tests/terminal-grid-native-launch.test.ts— Tier GN, andpackages/core/tests/agent-session-launch.test.ts— Tier SP.Look carefully at:
reserve()returning a resource that holdsclaim.admit()for the launch's whole scope — not just the child's lifetime. That is what keeps the pane held while the session lease around the child is still unwinding, and what refuses a second overlapping launch.spawnedwrapper inlaunch. Reporting anywhere else — after preparation, after the reservation, on an allocated PID — would present a pane that never started as one that is running.What must stay true
<Session.Launch>is unchanged — enforced by installing nothing at the root, checked by SL1–SL18 (all still passing).agent_session_launchrecords for the authored pane titles,ordinalandcolumns.session-busy.spawnedparameter, checked by SP3, SP4 and FL9.How to verify it
Tier SP — the pane seam, against a stub provider:
reserve.Tier FL — the foreground launcher, with real children:
spawnevent is reported once before waiting, and never reported for a child that could not start.Tier GN — the whole TestAgent stack, real worker over a real ACP connection, deterministic coordinator:
attach, none having left by then, in the authored row-major positions and forms.session-busy, and nothing attaches.Each claim was broken on purpose and re-run: removing the pane launcher fails SP1–SP4 and GN1–GN4, GN7–GN9; never reporting readiness fails SP1, SP2, SP3 and SP5; releasing the pane as soon as it is taken fails SP5 and only SP5.
Scope
Included
NativeLauncherHandler.launch.Intentionally unchanged
nativeLaunch()'s signature, so no provider adapter changed.New abstractions
usePaneNativeLauncher(claim, flush)exists because a pane's terminal has to answerreserveandflushfor whatever is written in it, and<Session.Launch>must not learn that it is in a pane. Its consumer ispaneWork()inexpand.ts.ControlledLauncherOptions.startexists so a suite can decide whether a launch starts at all — reporting is what a successful start does, throwing without reporting is what a failure before the start does. Its consumers are Tier SP and Tier GN.Risks and limitations
ownership.quiesced()and the next owner of that session receivessession-recovery-required. This is the settled Launch a deterministically prepared Agent session in its native UI #517 rule — "a session prepared and never handed over does not acknowledge, and the durable record stays active" — and GN8 asserts it deliberately, using the two refusals as its discriminator: "another owner is using session" would mean the lease was still held, "was left owned by work that did not finish" means it was released and tombstoned. If the grid contract's "awaits session quiescence" was meant to convert a reader close into a clean release, that is a change to Launch a deterministically prepared Agent session in its native UI #517's ownership semantics and is not made here.closedtwice — once from the outcome close decided, once from its own settlement. Display, not a second settlement; the rows assert the set of panes in a state rather than a message count.main, so the test matrix,lint,jsr,siteand the requiredgreencheck do not run. Local evidence: the four focused suites above, pluspackages/core,packages/runtime,packages/durable-streams(400 passed, 3058 steps) andpackages/test-agent,packages/acp(49 passed, 336 steps),deno task checkanddeno task lintclean.Scope confirmation