Skip to content

Adopt externally managed zmx sessions#604

Draft
LucasIcarus wants to merge 5 commits into
supabitapp:mainfrom
LucasIcarus:issue-591-adopt-zmx-session
Draft

Adopt externally managed zmx sessions#604
LucasIcarus wants to merge 5 commits into
supabitapp:mainfrom
LucasIcarus:issue-591-adopt-zmx-session

Conversation

@LucasIcarus

@LucasIcarus LucasIcarus commented Jul 8, 2026

Copy link
Copy Markdown

Closes #591

Summary

Adds a supacode tab adopt-zmx <session> command (and a matching …/tab/adopt-zmx deeplink) so an external orchestrator can surface a zmx session it already manages as a Supacode tab. Supacode only attaches — it never creates or kills the external session. Re-running is idempotent: the tab id is derived from the session name, an unchanged sync neither steals focus nor churns the sidebar, and closing or pruning an adopted tab spares the external session (only Supacode-owned sessions are torn down). Adopted tabs are excluded from the layout snapshot, so a restart defers re-adoption to the owner instead of restoring a detached shell.

Type of change

  • Bug fix (the linked issue is a bug report)
  • Feature (the linked issue is a feature request marked ready)
  • Documentation
  • Other (please describe)

How was this tested?

  • Unit tests cover the deeplink parse/build, idempotent re-adoption without focus stealing, snapshot exclusion of adopted tabs, and that tab close / prune / surface close never kill an externally owned session.
  • make check and make test pass locally, and I built, ran, and dogfooded the app.

Dogfooding used a small external orchestrator (a launchd agent) that syncs live zmx sessions from a remote devbox into Supacode. Simplified model:

# launchd agent, polls the devbox every few seconds
NS = uuid.uuid5(uuid.NAMESPACE_URL, "supacode-bridge")
stable_tab_id = lambda host, sid: str(uuid.uuid5(NS, f"{host}/{sid}"))

def sync(cli, host):
    sessions = inventory_live_zmx(host)          # ssh host, cross-checked against `zmx list --short`
    digest = sha256(canonical(sessions))
    if digest == last_applied:                   # unchanged manifest -> no-op (no churn)
        return
    known = set(run(cli, ["worktree", "list"]))
    for s in sessions:                           # one worktree tab per live remote session
        wt = host + s.worktree_path              # remote worktree id
        if wt in known:
            run(cli, ["tab", "adopt-zmx", s.zmx_name,
                      "-w", wt, "--id", stable_tab_id(host, s.id), "--title", s.title])
    for gone in missing_for_n_polls():           # prune stale tabs only after a threshold
        run(cli, ["tab", "close", "-w", gone.wt, "-t", gone.stable_tab_id])
    save(digest)

The stable UUID pins each remote session to one tab across polls; the manifest digest makes an unchanged sync a true no-op. Running this every few seconds against real devbox sessions is what surfaced the focus-steal, sidebar-churn, and external-session-kill cases fixed here.

  • make check passes (format + lint)
  • make test passes
  • I built and ran the app to confirm the change works

AI tool disclosure (optional)

  • Model(s): Claude Opus 4.8 & GPT 5.5
  • Harness / tools: Cursor & Codex

Checklist

  • This pull request is linked to an issue with Closes # above.
  • For a feature, the linked issue is labeled ready.
  • I am the author of this work and accountable for it; no commit is authored or co-authored by an AI agent.
  • I have read the Contributing guide and the Code of Conduct.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Thanks, this now meets the contribution policy. I've cleared the invalid label.

@github-actions github-actions Bot added the invalid Does not meet the contribution policy; closed automatically after a few days if left inactive. label Jul 8, 2026
@LucasIcarus LucasIcarus force-pushed the issue-591-adopt-zmx-session branch from 4f2aa8d to 7a47eea Compare July 8, 2026 03:26
@github-actions github-actions Bot removed the invalid Does not meet the contribution policy; closed automatically after a few days if left inactive. label Jul 8, 2026
@LucasIcarus LucasIcarus marked this pull request as ready for review July 8, 2026 03:26
@LucasIcarus LucasIcarus force-pushed the issue-591-adopt-zmx-session branch from 7a47eea to 583857a Compare July 9, 2026 09:48
@LucasIcarus

Copy link
Copy Markdown
Author

Quirk: an adopted zmx session running a mouse-tracking TUI (Claude Code) can't scroll until the first keystroke

Hit this dogfooding tab adopt-zmx; posting the root cause in case there's a cleaner intended behavior.

Symptom. A tab adopted onto a live zmx session running Claude can't be scrolled (wheel/trackpad) — it looks frozen. Any keystroke, or a window resize, instantly fixes it. Only Claude is affected (codex/gpt don't rely on SGR mouse events).

I confirmed it's not on supacode / libghostty side: ghostty keeps mouse_event enabled the whole time and correctly emits the SGR wheel events to the pty — they're dropped inside zmx.

Why. supacode attaches as a non-leader client. In Daemon.handleInput, a non-leader's bytes only reach the pty when util.isUserInput() is true, and that helper intentionally excludes mouse events:

// util.zig — isUserInput()
// mouse events: CSI M / CSI < — EXCLUDE these
// only intentional keyboard input should trigger leader switch
if (csi.final == 'M' or csi.final == '<') return false;
// main.zig — Daemon.handleInput()
if (leader == client) { queuePtyInput(payload); return; }  // leader: forward all
if (isUserInput(payload)) {                                // non-leader: only "user input"
    try setLeader(client);                                 //   → promotes to leader
    queuePtyInput(payload);                                //   → and forwards
}
// a non-leader's mouse: neither promotes nor forwards → silently dropped

So the wheel events are dropped. A keystroke passes isUserInput, which promotes supacode to leader, after which everything (incl. the wheel) flows — hence "press a key to unfreeze".

Both intents behind this look right to me: leader = resize authority, and ambient mouse motion/focus shouldn't steal the leader from whoever's typing. The quirk is that isUserInput gates both "promote leader" and "forward to pty", so excluding mouse from the leader switch also drops it entirely — even though mouse input has no resize-style single-authority requirement.

@LucasIcarus

Copy link
Copy Markdown
Author

@sbertix This PR was ready for review now, 2-days local dogfooding works fine with it's local build version and my local link-bridge launchd scripts.

But indeed a quirk "bug" located and mentioned above.

Thks for your efforts. Any following up and revision suggestions are welcome.

@sbertix

sbertix commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

The main pain points aren’t really the issues or suggestions themselves (and there are several, since we’re here, e.g., the supa- namespace collision kills the adopted session, leading-dash session names are parsed as options, cross-worktree moves destroy the source tab before validating the destination, cross-host tab-ID collision on the default ID, failed adopts are acknowledged as success, "Session not found" reports success, missing tests, inaccurate docstrings, etc.). But those can be fixed easily.

I held back from giving more structured feedback before because I’m worried about the direction. The core ownedZmxSessionIDs refactor is sound and well tested, but the primitive itself worries me more the more I think about it (again, the implementation itself is fine).

@LucasIcarus

LucasIcarus commented Jul 13, 2026

Copy link
Copy Markdown
Author

The main pain points aren’t really the issues or suggestions themselves (and there are several, since we’re here, e.g., the supa- namespace collision kills the adopted session, leading-dash session names are parsed as options, cross-worktree moves destroy the source tab before validating the destination, cross-host tab-ID collision on the default ID, failed adopts are acknowledged as success, "Session not found" reports success, missing tests, inaccurate docstrings, etc.). But those can be fixed easily.

I held back from giving more structured feedback before because I’m worried about the direction. The core ownedZmxSessionIDs refactor is sound and well tested, but the primitive itself worries me more the more I think about it (again, the implementation itself is fine).

@sbertix Thanks for the detailed read — and agreed the worthy worries. I only use supacode to serve my use case and didn't hold the critical big picture, happy to keep it out of core until/unless the shape feels right. No pressure either way.

And if I understand the situation correctly, the scroll quirk above plus the failure modes you listed are symptoms of the same thing: the primitive has supacode display and act on a session it doesn't own, so it inherits a large, leaky surface (lifecycle, kill-safety, ID identity, even the multiplexer's input model) it can't fully control.

If there is any appetite, I'd rather find the smallest contract everyone would be comfortable owning than push the current surface. A maximally narrowed version in my mind might be:

  • attach / display / detach only — never create, kill, or prune an external session (supacode owns nothing on the other side);
  • require an explicit stable id — no default-derived id, so no cross-host collision;
  • drop cross-worktree move entirely (the most destructive path);
  • strict session-name validation (reject leading-dash / the supa- namespace).

But still, I don't know supacode's full workflow surface or codebase well enough to judgee, and don't obtain rich variety of workflow variants to dogfooding with supacode. So I'll gladly defer to whichever direction you prefer — including shelving this.

@sbertix

sbertix commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

I'm worried about the opposite, tbh: that the contract is not "big enough" to actually treat attached sessions as fully fledged Supacode ones. We're introducing a second-class surface, without indication, without further escalation possibilities.
I think it'd probably make more sense to double down the other way: once you attach a session, it's 1:1 a Supacode session. But I'm not sure yet either 😅

@LucasIcarus

Copy link
Copy Markdown
Author

Understood. I'll keep running it locally. Whenever the shape feels right to you, I'm happy to reshape or redo (or some other PR) this to match.

@sbertix sbertix marked this pull request as draft July 14, 2026 10:01
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.

Adopt externally managed remote terminal sessions into Supacode

2 participants