Skip to content

feat: persist native runtime pane bindings - #16

Merged
shuv1337 merged 4 commits into
masterfrom
native-attachments
Oct 9, 2026
Merged

shuv1337 merged 4 commits into
masterfrom
native-attachments

Conversation

@shuv1337

@shuv1337 shuv1337 commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Native Shuvcode Sessions need an exact display attachment that survives Herdr restart without taking over execution. Add advertised JSON pane.bind_runtime, pane.get_runtime, pane.report_runtime, and pane.unbind_runtime operations with immutable attachment identity, applied/current acknowledgments, monotonic status sequences, and expiring observations. JSON ping also reports the actual peer session name so ShuvBro can verify its configured non-default destination before acting.

Persist credential-free attach argv and binding-owner tombstones separately from frozen generation-1 codecs. Restore the exact native view, retain scrollback when attachment restoration fails, preserve ownership through pane moves, and reserve closed owners' public IDs across restart. Bound panes retain native status authority; the real client keeps Done after acknowledgement and restart while existing Seen filtering and completion-notification semantics remain intact. Shuvcode launch and distribution detection preserve the Shuvcode executable.

Companion integration: Shuvcode #440 supplies exact runtime attachment and presentation; ShuvBro #62 supplies orchestration policy and pane placement. The contract and joint evidence describe the boundary.

Validation

  • Final source 63835aa2: 3,830 Rust tests passed, 8 intentional skips; formatting, Linux Clippy, maintenance, hotpath, assets and API schema checks passed. Named/default ping and omitted-field compatibility are covered; frozen generation-1 fixtures are unchanged.
  • The actual client Done regressions failed before the presentation fix and pass after it. A private real-client check shows Done before acknowledgement, after acknowledgement, and after Herdr restart, preserving home/Session/pane/binding identities with model requests 4 → 4.
  • Six release render profiles on the completed presentation path passed. At 1/15 panes, median server render time was 293/289 microseconds for background workspaces and 293/360 microseconds for active panes; client composition was 214/226 and 222/217 microseconds respectively. The subsequent ping identity field adds no rendering work. These are cardinality measurements, not a before/after benchmark.
  • Private joint lab with compiled Linux Shuvcode, actual Herdr PTYs and a deterministic local Responses provider: one lead/two workers, blocked/working/completed projection, display close while work continues, exact restore without extra model calls, positive-settlement cleanup retaining neighbors and parent focus, independent runtime/manager/Herdr restart, explicit reopen, and observation expiry.
  • Peer-capable binary SHA-256 aa98c90952d6b10ce14e9c030d2846d8fa24f6ddb57bcaa0187041d6be22b8f2: ShuvBro refused a mismatched session before native initialization. Continuous-watch cleanup/reopen confirmed exact foreground argv and both closed pane absences, preserved both generations' neighbors and parent focus, and retained native settlement with model requests 4 → 4.
  • An additional actual restart check closed the highest bound pane w5:p1 and its workspace. After restart with only w1–w4 surviving, the next workspace was w6, confirming closed-owner identity reservation.

Initial Windows CI exposed Unix-only fixture paths; they now use platform-native absolute paths with strict production validation retained. All seven GitHub checks at 63835aa2 passed, including Ubuntu, macOS, Windows and ConPTY. Local Windows cross lint remains unverified because this environment lacks the configured Windows SDK libc file. The joint workflow qualification is local Linux with new managed homes; two-host operation and production load remain unqualified.

shuv1337 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

🔍 Automated Cursor review is underway. Please hold off on merging until the review comment lands here (usually 10–20 minutes).

@cursor

cursor Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review complete:

Generation-1 method shapes are left intact (the four methods are new hashes, not edits of the frozen fixture), and the session fields are optional with serde defaults. The problems below are behavioral.

High

Restore will execute a caller-chosen absolute command, including when agent resume is off. src/runtime_attachment.rs:47 and src/persist/restore.rs:816.
RuntimeAttachment::validate only checks that some absolute prefix is followed by supervisor attach plus the eight identity flags. These prefixes all pass: any absolute executable, bun plus any absolute entrypoint, and bun --no-env-file --preload plus any absolute preload script. pane_restore_startup then turns that argv into an AgentResumePlan even when resume_agents_on_restore is false, and it does not record reserved_agent_session, so a failed spawn cannot roll the dedupe key back. The comment says arbitrary commands are not persisted; the check does not enforce that. A JSON client can plant a command that runs on the next session load and ignores the resume opt-out. If validation fails, the same function returns initial_history_ansi: None without falling through, so a one-flag argv change also drops scrollback and will not resume the saved session.
Suggested fix: pin the prefix to a known shuvcode basename (and, if bun is required, a known entrypoint with no free --preload path). Launch it only from that allowlist. On validation failure, keep pane history and surface a restore error instead of a blank shell.

Managed Done is not what the shell shows, and the post-restore Done path never becomes a completion. src/app/api/runtime_attachment.rs:161 and src/client/shell/endpoint_agent_state.rs:89.
Done.detect() is Idle, so a report only takes the special completion path when the previous detect state is already Idle. After cold restore, handoff, or TTL expiry, terminal.state is Unknown. The next Done report is Unknown → Idle, is_completion_transition is false, and last_agent_completion_seq stays empty. The shell treats Idle and Done as the same value and keeps Done only while completions[pane] == state_change_seq and that sequence is not yet acknowledged (projected_status). With no completion sequence, seen() is true and the shell rewrites Done to Idle immediately. When the special case does run (Idle → Done), it sets last_agent_state_change_seq and last_agent_completion_seq to the same number, so the next surface acknowledgement clears Done again. Server pane.agent_status and the server agent-view test stay Done; the client shell does not. That same Idle → Done branch also never calls record_or_deliver_agent_notification, so there is no finished toast or sound unless the pane was previously Working or Blocked.
Suggested fix: record a completion sequence for every transition into runtime Done, including from Unknown, and teach the shell projection to keep an explicit managed Done instead of collapsing it with acknowledged idle. Deliver the existing completion notification from that edge without overwriting the pane's real seen flag.

Medium

Unbind leaves a tombstone that does not follow a later move, and unbind does not make the binding replaceable. src/app/api/runtime_attachment.rs:214 and src/app/api/panes.rs:1278.
pane.move rewrites binding.pane_id and the owner map only while terminal.runtime_binding is still set. pane.unbind_runtime clears the terminal binding and returns applied: true, but it never removes or retargets runtime_binding_owners. Unbind, then move, then bind the same binding_id and attachment to the pane's new public id hits runtime_binding_conflict, because the tombstone still names the old id. The conflict text says to unbind the current binding first; after unbind, a different attachment on that same binding_id still conflicts. There is no test for unbind-then-move.
Suggested fix: on every public-id change, retarget owner entries whose pane_id was the previous id, including tombstones. Either drop the owner on unbind, or define a real replace operation that keeps the sequence but accepts a new attachment.

Live handoff throws away a fresh observation. src/runtime_attachment.rs:191 and src/persist/restore.rs:692.
restore_runtime_binding always sets state to Unknown, clears the label, and sets runtime_observed_at to None. Cold restore needs that, because the attach process is new. Handoff imports the existing PTY and skips the attach plan (was_imported), but it still calls restore_runtime_binding. Status drops to Unknown across every server handoff until something reports again. A reporter that only sends transitions, not heartbeats, never recovers.
Suggested fix: for an imported pane, keep the last runtime state and start a new TTL from restore time. Force Unknown only when the attach command is actually relaunched.

Low

agent start --kind shuvcode is only fixed for that exact string. src/app/agents.rs:197 and src/terminal/state.rs:2026.
parse_agent_label lowercases and maps shuvcode to OpenCode, but the executable override compares params.kind == "shuvcode". Shuvcode still launches opencode. begin_managed_agent then stores agent_label(OpenCode), which is "opencode", as the name owner, so the managed identity is opencode until process detection rewrites the display label. The new branch also has no Windows .cmd suffix.
Suggested fix: choose the executable from the normalized distribution label, and store that same label on the managed-agent owner.

I did not find a generation-1 codec break. Windows cross-checks and a real two-host attach are still untested; Path::is_absolute and the POSIX shell quoting in deferred resume are both host-local, so those paths need a Windows run before this is treated as done.

@shuv1337

shuv1337 commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

Reviewed and approved via Plannotator at 63835aa2 (no changes requested).

Re-verified the Cursor findings against the current head (Cursor reviewed before 2fcdfc36/63835aa2 landed):

  • Fixed: validation-failure scrollback/reservation rollback, client-shell managed Done projection (managed_statuses, optional + serde default), unbind tombstone retargeting on pane.move, --kind distribution-label normalization.
  • Still open, non-blocking: RuntimeAttachment::validate accepts any absolute executable prefix; live handoff resets bound status to Unknown until the next report; runtime_binding_owners tombstones are never pruned (persisted growth + permanent workspace-ID reservation); Unknown -> Done reports produce no completion notification.

No generation-1 codec changes; frozen fixtures untouched.

@shuv1337

Copy link
Copy Markdown
Owner Author

Follow-ups filed for the non-blocking items: #17 (executable allowlist), #18 (handoff resets status to Unknown), #19 (owner tombstones never pruned), #20 (Unknown -> Done completion notification).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant