From 94bc6bdccf4fe734d058abb6b2410e49f9619508 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Mon, 3 Aug 2026 11:27:47 +0000 Subject: [PATCH] fix(copilot): pin session id for exact resume, synthesize focus events, free the left-arrow key - dispatch now passes --session-id (uam ids are UUIDv4) so --resume= matches by Copilot's primary session id instead of depending on name matching; --name still seeds the same value for display and as the fallback match for sessions dispatched by older uam versions - the session host now synthesizes ?1004 focus events: focus-in when a controller attaches (or when the provider enables the mode with a controller already attached, which is how every resume starts) and focus-out when the last controller detaches; without a terminal attached, providers that dim or lock their input box while unfocused never heard either event - provider terminal policy gains a BackDetach field; copilot disables the bare-left-arrow quick detach because it binds left arrow to its own pane navigation at an empty composer; profiles and session overrides still take precedence --- internal/adapter/copilot/copilot.go | 16 ++- internal/adapter/copilot/copilot_test.go | 3 + internal/adapter/terminal_policy.go | 19 ++++ .../agents/provider_terminal_policy_test.go | 8 +- internal/app/profile_resolver.go | 5 +- internal/app/profile_resolver_test.go | 41 +++++++ internal/session/host.go | 48 ++++++++ internal/session/host_attach.go | 13 +++ internal/session/host_focus_events_test.go | 104 ++++++++++++++++++ internal/session/todo11_fake_provider_test.go | 7 +- .../todo8_terminal_policy_manual_test.go | 7 +- internal/vterm/private_csi_test.go | 15 +++ internal/vterm/vterm.go | 9 ++ 13 files changed, 283 insertions(+), 12 deletions(-) create mode 100644 internal/session/host_focus_events_test.go diff --git a/internal/adapter/copilot/copilot.go b/internal/adapter/copilot/copilot.go index dc3de1d..05d94b9 100644 --- a/internal/adapter/copilot/copilot.go +++ b/internal/adapter/copilot/copilot.go @@ -6,16 +6,22 @@ import ( func New(backend adapter.Backend) adapter.AgentAdapter { agent := adapter.NewAgent("copilot", "GitHub Copilot", []adapter.CommandCandidate{{Display: "copilot", Args: []string{"copilot"}}}, []string{"--yolo"}, backend) - agent.Terminal = adapter.ProviderTerminalPolicy{Identity: adapter.ProviderCopilot, OuterScreen: adapter.OuterScreenUAM, KeyProtocol: adapter.KeyProtocolNative} - // copilot supports exact-session resume natively: --name seeds the new - // session's name with the uam id at dispatch, and --resume matches it - // exactly (case-insensitive) on resume. + // Copilot binds left arrow to its own pane navigation at an empty + // composer, so the attach client's quick detach would steal the key. + agent.Terminal = adapter.ProviderTerminalPolicy{Identity: adapter.ProviderCopilot, OuterScreen: adapter.OuterScreenUAM, KeyProtocol: adapter.KeyProtocolNative, BackDetach: adapter.BackDetachDisabled} + // copilot supports exact-session resume natively. The uam id is a UUID, + // so --session-id pins the new Copilot session's primary id to it at + // dispatch and --resume then matches by session id — the most stable + // lookup the CLI offers. --name seeds the same value as the session name + // for display and as a fallback match: sessions dispatched by older uam + // versions carry the uam id only as a name, and --resume falls back to + // exact (case-insensitive) name matching for them. agent.SessionArgs = func(req adapter.ResumeRequest, activity string) []string { if req.ID == "" { return nil } if activity == "dispatched" { - return []string{"--name", req.ID} + return []string{"--session-id", req.ID, "--name", req.ID} } return []string{"--resume=" + req.ID} } diff --git a/internal/adapter/copilot/copilot_test.go b/internal/adapter/copilot/copilot_test.go index 086b7f7..54ed7e5 100644 --- a/internal/adapter/copilot/copilot_test.go +++ b/internal/adapter/copilot/copilot_test.go @@ -58,6 +58,9 @@ func TestDispatchSeedsCopilotSessionIDForFutureResume(t *testing.T) { t.Fatalf("Dispatch: %v", err) } argv := be.CommandLog() + if !strings.Contains(argv, "--session-id "+sess.ID) { + t.Fatalf("copilot dispatch should pin the provider session id to the UAM id: %s", argv) + } if !strings.Contains(argv, "--name "+sess.ID) { t.Fatalf("copilot dispatch should name the provider session with the UAM id: %s", argv) } diff --git a/internal/adapter/terminal_policy.go b/internal/adapter/terminal_policy.go index 9284519..60b515a 100644 --- a/internal/adapter/terminal_policy.go +++ b/internal/adapter/terminal_policy.go @@ -24,10 +24,26 @@ type KeyProtocolPolicy string const KeyProtocolNative KeyProtocolPolicy = "native" +// BackDetachPolicy is a provider's default for the attach client's quick +// detach (a bare left arrow while the input box is empty detaches). The +// gesture assumes left arrow is a no-op at an empty prompt; providers that +// bind it to their own UI (pane or tab navigation) disable the default. +// Profiles and session overrides still take precedence either way. +type BackDetachPolicy string + +const ( + // BackDetachDefault leaves the quick detach enabled (the zero value). + BackDetachDefault BackDetachPolicy = "" + // BackDetachDisabled turns the quick detach off unless a profile or + // override explicitly enables it. + BackDetachDisabled BackDetachPolicy = "disabled" +) + type ProviderTerminalPolicy struct { Identity ProviderIdentity OuterScreen OuterScreenPolicy KeyProtocol KeyProtocolPolicy + BackDetach BackDetachPolicy } type TerminalPolicyAdapter interface { @@ -44,6 +60,9 @@ func (p ProviderTerminalPolicy) Validate() error { if p.KeyProtocol != KeyProtocolNative { return fmt.Errorf("invalid provider key-protocol policy %q", p.KeyProtocol) } + if p.BackDetach != BackDetachDefault && p.BackDetach != BackDetachDisabled { + return fmt.Errorf("invalid provider back-detach policy %q", p.BackDetach) + } return nil } diff --git a/internal/agents/provider_terminal_policy_test.go b/internal/agents/provider_terminal_policy_test.go index 4d66867..22c5f86 100644 --- a/internal/agents/provider_terminal_policy_test.go +++ b/internal/agents/provider_terminal_policy_test.go @@ -13,9 +13,11 @@ import ( func TestProviderTerminalPolicyMatrix(t *testing.T) { want := map[string]adapter.ProviderTerminalPolicy{ - "claude": {Identity: adapter.ProviderClaude, OuterScreen: adapter.OuterScreenUAM, KeyProtocol: adapter.KeyProtocolNative}, - "codex": {Identity: adapter.ProviderCodex, OuterScreen: adapter.OuterScreenPrimary, KeyProtocol: adapter.KeyProtocolNative}, - "copilot": {Identity: adapter.ProviderCopilot, OuterScreen: adapter.OuterScreenUAM, KeyProtocol: adapter.KeyProtocolNative}, + "claude": {Identity: adapter.ProviderClaude, OuterScreen: adapter.OuterScreenUAM, KeyProtocol: adapter.KeyProtocolNative}, + "codex": {Identity: adapter.ProviderCodex, OuterScreen: adapter.OuterScreenPrimary, KeyProtocol: adapter.KeyProtocolNative}, + // copilot binds left arrow to pane navigation at an empty composer, + // so the quick-detach gesture is disabled by default for it. + "copilot": {Identity: adapter.ProviderCopilot, OuterScreen: adapter.OuterScreenUAM, KeyProtocol: adapter.KeyProtocolNative, BackDetach: adapter.BackDetachDisabled}, "hermes": {Identity: adapter.ProviderHermes, OuterScreen: adapter.OuterScreenUAM, KeyProtocol: adapter.KeyProtocolNative}, // omp sets no mouse modes and never enters ?1049 — verified on a PTY — // so like codex it owns the primary screen and its scrollback. diff --git a/internal/app/profile_resolver.go b/internal/app/profile_resolver.go index 6dfc7c1..e0e0546 100644 --- a/internal/app/profile_resolver.go +++ b/internal/app/profile_resolver.go @@ -15,7 +15,10 @@ func ResolveProfilePolicy(input ResolutionInput) (EffectivePolicy, error) { scrollbackLines: defaultScrollbackLines, term: fixedTERM, }, attachment: attachmentDefaults{ - mouse: store.MousePolicyAuto, controlPrefix: defaultControlPrefix, backDetach: true, + mouse: store.MousePolicyAuto, controlPrefix: defaultControlPrefix, + // The provider's terminal policy supplies the quick-detach + // default; profiles and session overrides below still win. + backDetach: input.ProviderPolicy.BackDetach != adapter.BackDetachDisabled, }, } if policy.launch.provider == "" { diff --git a/internal/app/profile_resolver_test.go b/internal/app/profile_resolver_test.go index eb8110d..206623c 100644 --- a/internal/app/profile_resolver_test.go +++ b/internal/app/profile_resolver_test.go @@ -104,3 +104,44 @@ func allClientCapabilities() ClientCapabilities { } func pointer[T any](value T) *T { return &value } + +func TestProviderBackDetachPolicySetsTheDefault(t *testing.T) { + cfg := store.DefaultConfig() + record := store.SessionRecord{Agent: "copilot"} + policy := adapter.ProviderTerminalPolicy{ + Identity: adapter.ProviderCopilot, OuterScreen: adapter.OuterScreenUAM, + KeyProtocol: adapter.KeyProtocolNative, BackDetach: adapter.BackDetachDisabled, + } + effective, err := ResolveProfilePolicy(ResolutionInput{Config: cfg, Session: record, ProviderPolicy: policy}) + if err != nil { + t.Fatal(err) + } + attachment, err := effective.NewAttachment(ClientTemporaryOverride{}, allClientCapabilities()) + if err != nil { + t.Fatal(err) + } + if attachment.BackDetach() { + t.Fatal("provider back-detach disabled policy should default the quick detach off") + } +} + +func TestProfileBackDetachOverridesProviderPolicy(t *testing.T) { + cfg := store.DefaultConfig() + cfg.Profiles["gesture"] = store.Profile{BackDetach: pointer(true)} + record := store.SessionRecord{Agent: "copilot", Profile: "gesture"} + policy := adapter.ProviderTerminalPolicy{ + Identity: adapter.ProviderCopilot, OuterScreen: adapter.OuterScreenUAM, + KeyProtocol: adapter.KeyProtocolNative, BackDetach: adapter.BackDetachDisabled, + } + effective, err := ResolveProfilePolicy(ResolutionInput{Config: cfg, Session: record, ProviderPolicy: policy}) + if err != nil { + t.Fatal(err) + } + attachment, err := effective.NewAttachment(ClientTemporaryOverride{}, allClientCapabilities()) + if err != nil { + t.Fatal(err) + } + if !attachment.BackDetach() { + t.Fatal("an explicit profile back-detach must win over the provider default") + } +} diff --git a/internal/session/host.go b/internal/session/host.go index 03c49b0..e515921 100644 --- a/internal/session/host.go +++ b/internal/session/host.go @@ -128,6 +128,11 @@ type host struct { label string state State registry *clientRegistry + // providerFocused records whether the host has told the agent its + // terminal is focused (synthetic ?1004 focus events, guarded by mu). Two + // paths can observe the same attach — the attach initializer and the PTY + // pump seeing ?1004h turn on — and the flag keeps them from both firing. + providerFocused bool child *exec.Cmd // exited is closed once the agent process has been reaped; the kill @@ -245,6 +250,24 @@ func runHost(dir string, spec hostLaunchSpec, ready *os.File) error { // pumpPTY copies agent output into the emulator and to every attached client // until the PTY reaches EOF (agent exit). +// focusIn and focusOut are the xterm focus-tracking events an application +// with ?1004 enabled expects from its terminal. The host synthesizes them at +// controller attach/detach boundaries (and on late ?1004 enablement) because +// the real terminal's focus events can only reach the agent while a client is +// attached — see initializeAttachClient and removeClient. +var focusIn = []byte("\x1b[I") +var focusOut = []byte("\x1b[O") + +// writeFocusEvent injects a synthetic focus event into the agent's input. +// Like applyPTYSize it is deliberately unserialised against controller input: +// the event is a complete three-byte sequence, and callers may hold neither +// mutex (pumpPTY) or only controlMu (attach/drop paths). +func (h *host) writeFocusEvent(event []byte) { + if _, err := h.ptmx.Write(event); err != nil { + log.Debug("write synthetic focus event failed", "session", h.name, "error", err) + } +} + func (h *host) pumpPTY() { buf := make([]byte, 32*1024) for { @@ -254,8 +277,23 @@ func (h *host) pumpPTY() { copy(data, buf[:n]) h.mu.Lock() _, _ = h.term.Write(data) + if !h.term.FocusReporting() { + // The mode is off (or was just turned off): a later re-enable + // deserves a fresh focus-in. + h.providerFocused = false + } + // An agent that enables ?1004 after a controller attached (every + // resume works this way: the client attaches while the replacement + // process is still starting) missed the attach-time focus-in. + focusGained := h.term.FocusReporting() && h.registry.controller != nil && !h.providerFocused + if focusGained { + h.providerFocused = true + } clients := h.registry.readyClients() h.mu.Unlock() + if focusGained { + h.writeFocusEvent(focusIn) + } for _, client := range clients { h.enqueueClient(client, serverMessage{kind: serverFramePTY, payload: data}) } @@ -593,7 +631,17 @@ func (h *host) removeClient(client *attachClient, reason string) { promotedReplay = h.term.Redraw() } } + // The counterpart of the attach-time synthetic focus-in: with no + // controller left there is no terminal whose focus the agent could hold. + focusLost := registered && wasController && h.registry.controller == nil && + h.term != nil && h.term.FocusReporting() && h.providerFocused + if focusLost { + h.providerFocused = false + } h.mu.Unlock() + if focusLost { + h.writeFocusEvent(focusOut) + } if promotedSize.valid() { h.applyPTYSize(promotedSize) } diff --git a/internal/session/host_attach.go b/internal/session/host_attach.go index bb81f65..dd273e0 100644 --- a/internal/session/host_attach.go +++ b/internal/session/host_attach.go @@ -76,7 +76,20 @@ func (h *host) initializeAttachClient(client *attachClient, registration clientR } client.out <- serverMessage{kind: serverFramePTY, payload: append([]byte(titleSequence(label)), h.term.Redraw()...)} client.ready = true + focusGained := controls && h.term.FocusReporting() && !h.providerFocused + if focusGained { + h.providerFocused = true + } h.mu.Unlock() + if focusGained { + // The agent asked for focus events (?1004) but runs under a detached + // host, so no terminal ever tells it a window gained focus. A + // controller attaching is that event; without it agents that dim or + // lock their input box while unfocused stay that way. Client + // terminals that implement ?1004 may also send their own focus-in + // once the replay enables the mode — a duplicate is harmless. + h.writeFocusEvent(focusIn) + } if !controls || !registration.size.valid() { return } diff --git a/internal/session/host_focus_events_test.go b/internal/session/host_focus_events_test.go new file mode 100644 index 0000000..9e0a629 --- /dev/null +++ b/internal/session/host_focus_events_test.go @@ -0,0 +1,104 @@ +package session + +import ( + "bufio" + "context" + "net" + "strings" + "testing" +) + +// attachControllerConn dials the host socket and completes a v1 controller +// attach, returning the open connection. The caller owns closing it. +func attachControllerConn(t *testing.T, c *Client, name string) net.Conn { + t.Helper() + conn, err := net.Dial("unix", SocketPath(c.Dir, name)) + if err != nil { + t.Fatal(err) + } + if err := writeJSONLine(conn, request{Op: opAttach, Cols: 120, Rows: 40}); err != nil { + t.Fatal(err) + } + var resp response + if err := readJSONLine(bufio.NewReader(conn), &resp); err != nil || !resp.OK { + t.Fatalf("attach resp: %+v %v", resp, err) + } + return conn +} + +// A provider with focus reporting (?1004) enabled runs under a detached host, +// so no terminal ever tells it about focus changes. The host must synthesize +// focus-in when a controller attaches and focus-out when the last controller +// detaches. The fake agent enables ?1004 before echoing stdin via cat -v, so +// the synthesized events become visible in the capture as ^[[I / ^[[O once a +// newline flushes the canonical input buffer. +func TestHostSynthesizesFocusEventsAtAttachBoundaries(t *testing.T) { + c := newTestClient(t) + ctx := context.Background() + name := "uam-fake-56565656" + done := startInProcessHost(t, c, name, `printf '\033[?1004h'; echo armed; cat -v`) + defer func() { + _ = c.Kill(ctx, name) + <-done + }() + waitFor(t, "focus reporting armed", func() bool { + out, err := c.Capture(ctx, name, 50) + return err == nil && strings.Contains(out, "armed") + }) + + conn := attachControllerConn(t, c, name) + defer func() { _ = conn.Close() }() + // The synthetic focus-in has no newline; a carriage return flushes the + // agent's canonical input buffer so cat -v echoes what preceded it. Sent + // inside the poll because the attach response can race the host's + // initialization of the new client. + waitFor(t, "synthetic focus-in", func() bool { + _ = writeFrame(conn, frameStdin, []byte("\r")) + out, _ := c.Capture(ctx, name, 50) + return strings.Contains(out, "^[[I") + }) + + if err := writeFrame(conn, frameDetach, nil); err != nil { + t.Fatal(err) + } + waitFor(t, "synthetic focus-out", func() bool { + // Flushes out-of-band; rejected with SessionBusy until the host has + // actually dropped the controller, which the poll absorbs. + _ = c.SendLine(ctx, name, "") + captured, err := c.Capture(ctx, name, 50) + return err == nil && strings.Contains(captured, "^[[O") + }) +} + +// The resume path attaches the controller while the replacement provider +// process is still starting: ?1004 comes on only after the attach. The host +// must synthesize the missed focus-in the moment the mode turns on. +func TestHostSynthesizesFocusInWhenModeArmsAfterAttach(t *testing.T) { + c := newTestClient(t) + ctx := context.Background() + name := "uam-fake-78787878" + done := startInProcessHost(t, c, name, `echo waiting; read go; printf '\033[?1004h'; echo armed; cat -v`) + defer func() { + _ = c.Kill(ctx, name) + <-done + }() + waitFor(t, "agent waiting", func() bool { + out, err := c.Capture(ctx, name, 50) + return err == nil && strings.Contains(out, "waiting") + }) + + conn := attachControllerConn(t, c, name) + defer func() { _ = conn.Close() }() + if err := writeFrame(conn, frameStdin, []byte("go\r")); err != nil { + t.Fatal(err) + } + waitFor(t, "focus reporting armed after attach", func() bool { + out, _ := c.Capture(ctx, name, 50) + return strings.Contains(out, "armed") + }) + waitFor(t, "late synthetic focus-in", func() bool { + _ = writeFrame(conn, frameStdin, []byte("\r")) + out, _ := c.Capture(ctx, name, 50) + return strings.Contains(out, "^[[I") + }) +} diff --git a/internal/session/todo11_fake_provider_test.go b/internal/session/todo11_fake_provider_test.go index f0078ab..380ad7d 100644 --- a/internal/session/todo11_fake_provider_test.go +++ b/internal/session/todo11_fake_provider_test.go @@ -122,7 +122,12 @@ func TestTodo11FakeProviderProcess(t *testing.T) { t.Fatal(err) } pending = pending[:0] - if bytes.Equal(record, todo11ProviderExit) { + // The exit marker may carry a leading synthetic focus-in: the + // fixture provider enables ?1004, so the host injects \x1b[I + // when a controller attaches (and \x1b[O when the last one + // detaches), and those bytes merge into whatever record the + // next delimiter closes. + if bytes.HasSuffix(record, todo11ProviderExit) { return } continue diff --git a/internal/session/todo8_terminal_policy_manual_test.go b/internal/session/todo8_terminal_policy_manual_test.go index bf8eaad..f0bc9d1 100644 --- a/internal/session/todo8_terminal_policy_manual_test.go +++ b/internal/session/todo8_terminal_policy_manual_test.go @@ -52,9 +52,12 @@ func TestTodo8TerminalPolicyRealPTYFixture(t *testing.T) { if _, err := first.ptmx.Write(payload); err != nil { t.Fatal(err) } + // The provider enabled ?1004, so the host synthesized a focus-in when the + // first controller attached; everything after it must be byte-exact. + expectedInput := append(append([]byte{}, focusIn...), payload...) waitFor(t, "byte-exact provider input", func() bool { got, err := os.ReadFile(providerInputPath) - return err == nil && bytes.Equal(got, payload) + return err == nil && bytes.Equal(got, expectedInput) }) providerInput, err := os.ReadFile(providerInputPath) if err != nil { @@ -85,7 +88,7 @@ func TestTodo8TerminalPolicyRealPTYFixture(t *testing.T) { SharedProviderMarker: bytes.Contains(firstLiveOutput, []byte("TASK8-READY")) && bytes.Contains(secondLiveOutput, []byte("TASK8-READY")), FirstPreservedMouse: containsEnabledDECMode(firstLiveOutput, "1000") && containsEnabledDECMode(firstLiveOutput, "1006"), SecondFilteredMouse: !containsEnabledDECMode(secondLiveOutput, "1000") && !containsEnabledDECMode(secondLiveOutput, "1006"), - ProviderInputExact: bytes.Equal(providerInput, payload), + ProviderInputExact: bytes.Equal(providerInput, expectedInput), RuntimeClean: true, } if !assertion.SharedProviderMarker || !assertion.FirstPreservedMouse || !assertion.SecondFilteredMouse || !assertion.ProviderInputExact { diff --git a/internal/vterm/private_csi_test.go b/internal/vterm/private_csi_test.go index 5e91d2a..8944f87 100644 --- a/internal/vterm/private_csi_test.go +++ b/internal/vterm/private_csi_test.go @@ -118,3 +118,18 @@ func TestRedrawReplaysRegionSavedCursorAndPen(t *testing.T) { t.Fatalf("redraw = %q, want the live SGR pen restored last", out) } } + +func TestFocusReportingTracksMode1004(t *testing.T) { + term := New(80, 24, 100) + if term.FocusReporting() { + t.Fatal("focus reporting should be off by default") + } + _, _ = term.Write([]byte("\x1b[?1004h")) + if !term.FocusReporting() { + t.Fatal("focus reporting should be on after ?1004h") + } + _, _ = term.Write([]byte("\x1b[?1004l")) + if term.FocusReporting() { + t.Fatal("focus reporting should be off after ?1004l") + } +} diff --git a/internal/vterm/vterm.go b/internal/vterm/vterm.go index b67c48d..91475e6 100644 --- a/internal/vterm/vterm.go +++ b/internal/vterm/vterm.go @@ -735,6 +735,15 @@ func (t *Terminal) Capture(maxLines int) string { return strings.Join(joined, "\n") + "\n" } +// FocusReporting reports whether the agent has focus reporting (DEC private +// mode 1004) enabled. The session host uses it to synthesize focus events at +// attach and detach boundaries: a real terminal tells the application when +// its window gains or loses focus, and an agent running under a detached host +// would otherwise never hear either. +func (t *Terminal) FocusReporting() bool { + return t.privModes[1004] +} + // Redraw returns an ANSI byte sequence that repaints the current screen on a // fresh terminal: reset attributes, clear, draw every row with the SGR state // each cell was written with, and park the cursor. The session host sends it