From c66d451a748a02122a8c4353d5a4259de988cde6 Mon Sep 17 00:00:00 2001 From: tdwd Date: Wed, 9 Sep 2026 13:05:11 +0200 Subject: [PATCH] codex: put gated actions in the approval pane, and hide what codex cannot do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes that finish the modes. Approvals. codex raised a gating request and cathode auto-refused it, so ask and plan could not run a tool at all. The request now goes to the same pane claude uses: the engine emits the shared pendingApprovalMsg, so the y/n bar, the diff card, the question picker and the build-mode short-circuit are reused, and the only codex-specific part is turning a decision back into a JSON-RPC reply. All four modes work now, verified live — ask asks, and the approved file is actually written. Two things had to move off the reader goroutine. It must keep draining while the pane is up, or the item events that draw the very card being approved never arrive. And approvals are admitted one at a time: the UI holds a single pending approval, so a second would overwrite the first and leave codex waiting on a reply that can no longer be given. claude cannot hit that because its approvals are pulled one at a time; codex pushes, so the engine serialises them. Capabilities. /sysprompt is delivered as a claude output style and /mcp reads claude's init line, so both did nothing on codex while still being offered. slashCmd now declares the backends it works on, and the palette, the help text and the dispatcher all ask the same question. Selecting one anyway is handled, not forwarded — falling through would send the literal "/sysprompt" to the agent as a prompt. --- README.md | 10 +-- agentname_test.go | 56 +++++++++++++++ codexapproval.go | 117 ++++++++++++++++++++++++------- codexengine.go | 40 +++++++---- codexengine_test.go | 163 ++++++++++++++++++++++++++++++++++++++++++++ codexlive_test.go | 65 ++++++++++-------- codexreader.go | 13 +++- commandlist.go | 7 ++ commands.go | 46 +++++++++++-- help.go | 3 + model.go | 7 ++ 11 files changed, 444 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index c41abe0..a812399 100644 --- a/README.md +++ b/README.md @@ -105,14 +105,14 @@ Codex needs `codex login` completed, the same way claude needs `claude login`. The codex backend is newer and narrower than the claude one: -- `build` and `bypass` work fully. Tools run, and codex asks for nothing. -- `ask` and `plan` refuse gated actions rather than granting them, because the - approval pane is not wired to codex yet. +- All four modes work. `build` and `bypass` run tools without asking; `ask` and + `plan` raise the same approval pane claude uses, and your answer becomes the + decision codex is waiting on. - File changes render as real diff cards, in both the unified and side-by-side styles. Other tool calls render as plain cards rather than the typed ones claude gets. -- Session replay, `/compact`, `/sysprompt` and the slash-command palette are - claude-only so far. +- `/sysprompt` and `/mcp` are claude-only and are hidden on codex rather than + offered and inert. Session replay and `/compact` are claude-only too. - `@path` inserts a path but does not inject the file. Only claude expands an `@` mention into file contents; codex reads the file itself with a tool. diff --git a/agentname_test.go b/agentname_test.go index c8f51c5..dff49ae 100644 --- a/agentname_test.go +++ b/agentname_test.go @@ -99,3 +99,59 @@ func TestSplashDialsTheRunningBackend(t *testing.T) { t.Errorf("codex splash still names claude:\n%s", out) } } + +// A command that depends on one backend's features must not be offered on the +// other. Offered and selected, /sysprompt would restart the session and change +// nothing, and an unhandled slash line is forwarded to the agent as a prompt. +func TestClaudeOnlyCommandsAreHiddenOnCodex(t *testing.T) { + claudeOnly := map[string]bool{} + for _, c := range slashCommands() { + if !c.availableOn(backendCodex) { + claudeOnly[c.name] = true + } + } + for _, name := range []string{"sysprompt", "mcp"} { + if !claudeOnly[name] { + t.Errorf("/%s depends on claude and must not be offered on codex", name) + } + if !mustFind(t, name).availableOn(backendClaude) { + t.Errorf("/%s must still work on claude", name) + } + } + + for _, it := range slashItems(backendCodex) { + if claudeOnly[it.id] { + t.Errorf("the codex palette still lists /%s", it.id) + } + } + if len(slashItems(backendClaude)) <= len(slashItems(backendCodex)) { + t.Error("claude should offer strictly more commands than codex right now") + } +} + +// Selecting one anyway is handled here, not forwarded. Forwarding would send +// the literal "/sysprompt" to the agent as a prompt. +func TestUnavailableCommandIsHandledNotForwarded(t *testing.T) { + m, _ := newTestModel(t, "") + m.backend = backendCodex + + _, _, handled := runSlash(&m, "/sysprompt on") + if !handled { + t.Fatal("an unavailable command must be handled, or it reaches the agent as a prompt") + } + last := m.entries[len(m.entries)-1] + if !strings.Contains(last.text, "not available") || !strings.Contains(last.text, "codex") { + t.Errorf("entry = %q, want it to say the command is unavailable on this backend", last.text) + } +} + +func mustFind(t *testing.T, name string) slashCmd { + t.Helper() + for _, c := range slashCommands() { + if c.name == name { + return c + } + } + t.Fatalf("no /%s command", name) + return slashCmd{} +} diff --git a/codexapproval.go b/codexapproval.go index 4470a4a..c83e070 100644 --- a/codexapproval.go +++ b/codexapproval.go @@ -3,17 +3,23 @@ package main +import ( + "encoding/json" + "strings" +) + // ---- answering the requests codex makes of us ---- // // The app-server sends requests in the other direction, and every one of them // blocks until answered. An unanswered request is not a dropped message: the -// turn stops there and the session looks frozen with no error anywhere. So this -// file's rule is that every server request gets a reply, always. +// turn stops there and the session looks frozen with no error anywhere. So the +// rule here is that every server request gets a reply, always — either the +// user's decision, or a refusal, but never silence. // -// The approval pane is not wired to codex yet. Until it is, a gated action is -// refused rather than granted, because the alternative is a backend that -// silently runs whatever it likes in the mode whose entire purpose is asking -// first. +// A gated action goes to the same pane claude's approvals use. That reuse is +// the point: the y/n bar, the diff card, the AskUserQuestion picker and the +// build-mode short-circuit are all shared, and the only codex-specific part is +// turning a decision back into a JSON-RPC response. // codexApprovalMethods are the server requests that gate an action, and so can // be answered with a decision. Anything not on this list is answered with a @@ -27,7 +33,8 @@ var codexApprovalMethods = map[string]bool{ "execCommandApproval": true, } -// codexRefusal is the decision sent for a gated action. +// codexRefusal is the decision sent when the user declines, and when a request +// has to be refused without asking. // // "cancel" and not "decline", and deliberately not read from the request's // availableDecisions: a file-change approval offers only ["accept"], so there is @@ -35,7 +42,19 @@ var codexApprovalMethods = map[string]bool{ // cancel is accepted anyway and ends the turn cleanly with TurnAborted, rather // than being rejected as an unknown variant. That makes it the one refusal that // works for every request shape. -const codexRefusal = "cancel" +const ( + codexRefusal = "cancel" + codexApproval = "accept" +) + +// codexApprovalParams is the part of a gating request this needs. The command +// approval carries the command; the file-change approval carries ids and +// nothing else, which is why the card is drawn from the earlier item/started +// and paired by itemId (toolcard.go). +type codexApprovalParams struct { + ItemID string `json:"itemId"` + Command string `json:"command"` +} // answerServerRequest replies to a request from the app-server. It never // declines to answer: see the file comment for what silence costs. @@ -43,25 +62,75 @@ func (e *codexEngine) answerServerRequest(f codexFrame) { if f.ID == nil { return } - if codexApprovalMethods[f.Method] { - _ = e.write(map[string]any{ - "jsonrpc": "2.0", - "id": *f.ID, - "result": map[string]any{"decision": codexRefusal}, - }) - e.emitError("refused " + f.Method + " — approvals are not wired to this backend yet") + if !codexApprovalMethods[f.Method] { + // Not an approval. Answer with the JSON-RPC "method not found" code, + // which is a well-defined way to say "this client cannot do that" and + // leaves the server to decide what happens next. + e.replyError(*f.ID, -32601, "cathode does not implement "+f.Method) + e.emitError("unhandled request " + f.Method) return } - // Not an approval. Answer with the JSON-RPC "method not found" code, which - // is a well-defined way to say "this client cannot do that" and leaves the - // server to decide what happens next. + e.askUser(*f.ID, f) +} + +// askUser puts a gated action in front of the user and answers with what they +// choose. +// +// All of it runs off the reader goroutine, for two separate reasons. The reader +// must keep draining while the pane is up, or the item events that draw the +// very card being approved never arrive. And approvals are admitted one at a +// time: the UI holds a single pending approval, so a second would overwrite the +// first and leave codex waiting on a reply that can no longer be given. +func (e *codexEngine) askUser(id int64, f codexFrame) { + var p codexApprovalParams + _ = json.Unmarshal(f.Params, &p) + + go func() { + e.approvalSlot <- struct{}{} + defer func() { <-e.approvalSlot }() + + reply := make(chan approvalReply, 1) + e.emitMsg(pendingApprovalMsg{req: approvalReq{ + toolName: codexApprovalLabel(f.Method, p), + toolUseID: p.ItemID, + input: f.Params, + reply: reply, + }}) + + decision := codexRefusal + if (<-reply).allow { + decision = codexApproval + } + e.replyResult(id, map[string]any{"decision": decision}) + }() +} + +// codexApprovalLabel names the action on the approval bar. The command itself +// is far more use than the method name, so prefer it where the request carries +// one; the rest fall back to the item kind. +func codexApprovalLabel(method string, p codexApprovalParams) string { + if c := strings.TrimSpace(p.Command); c != "" { + return c + } + switch method { + case "item/fileChange/requestApproval": + return "file change" + case "item/permissions/requestApproval": + return "permission" + } + return method +} + +// replyResult and replyError are the two shapes of answer, kept apart so a +// caller cannot half-fill one. +func (e *codexEngine) replyResult(id int64, result any) { + _ = e.write(map[string]any{"jsonrpc": "2.0", "id": id, "result": result}) +} + +func (e *codexEngine) replyError(id int64, code int, msg string) { _ = e.write(map[string]any{ "jsonrpc": "2.0", - "id": *f.ID, - "error": map[string]any{ - "code": -32601, - "message": "cathode does not implement " + f.Method, - }, + "id": id, + "error": map[string]any{"code": code, "message": msg}, }) - e.emitError("unhandled request " + f.Method) } diff --git a/codexengine.go b/codexengine.go index bd039cf..6314be9 100644 --- a/codexengine.go +++ b/codexengine.go @@ -35,6 +35,15 @@ type codexEngine struct { wmu sync.Mutex // serialises writes to stdin pending *codexPending + // approvalSlot admits one approval to the pane at a time. + // + // The UI holds exactly one pending approval (update.go assigns m.pending), + // so a second arriving before the first is answered would overwrite it and + // the first request would never be replied to — and codex waits on a reply + // forever. claude cannot hit this: its approvals are pulled one at a time by + // waitApproval. codex pushes, so the serialising has to happen here. + approvalSlot chan struct{} + mu sync.Mutex // guards everything below cwd string // working root the thread runs in resumeID string // thread to resume on Initialize, or "" @@ -42,12 +51,13 @@ type codexEngine struct { turnID string // live turn, learned from turn/started; interrupt needs it mode string // cathode mode, applied at the next turn/start model string - // sink is where a non-reply frame goes. Pipe sets it; until then frames - // are held in backlog. A function rather than the *tea.Program itself - // keeps the emit path independent of Bubble Tea, which is what lets a test - // collect frames directly. - sink func(codexFrame) - backlog []codexFrame // frames that arrived before Pipe registered a sink + // sink is where anything bound for the UI goes. Pipe sets it; until then + // messages are held in backlog. It takes a tea.Msg rather than a codexFrame + // because an approval request carries a reply channel, which cannot be + // expressed as JSON — and reusing pendingApprovalMsg is what lets codex + // share the whole approval pane rather than growing a second one. + sink func(tea.Msg) + backlog []tea.Msg // messages that arrived before Pipe registered a sink } // codexEngineConfig is what main resolved for a codex session. @@ -90,11 +100,12 @@ func newCodexEngine(cfg codexEngineConfig) (*codexEngine, error) { } e := &codexEngine{ cmd: cmd, stdin: stdin, stdout: stdout, - pending: newCodexPending(), - mode: cfg.Mode, - model: cfg.Model, - cwd: cfg.Cwd, - resumeID: cfg.ResumeID, + pending: newCodexPending(), + approvalSlot: make(chan struct{}, 1), + mode: cfg.Mode, + model: cfg.Model, + cwd: cfg.Cwd, + resumeID: cfg.ResumeID, } go e.read() return e, nil @@ -103,14 +114,13 @@ func newCodexEngine(cfg codexEngineConfig) (*codexEngine, error) { // Pipe registers the program and flushes anything the handshake produced. // Unlike claudeEngine.Pipe this does not block: reading started at construction. func (e *codexEngine) Pipe(p *tea.Program) { - send := func(f codexFrame) { p.Send(codexMsg{frame: f}) } e.mu.Lock() - e.sink = send + e.sink = p.Send held := e.backlog e.backlog = nil e.mu.Unlock() - for _, f := range held { - send(f) + for _, msg := range held { + p.Send(msg) } } diff --git a/codexengine_test.go b/codexengine_test.go index 01ba16e..d5d25ea 100644 --- a/codexengine_test.go +++ b/codexengine_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" "time" + + tea "github.com/charmbracelet/bubbletea" ) // fakeAppServer writes a stand-in for `codex app-server` and points @@ -52,6 +54,23 @@ while IFS= read -r line; do done ` +// sinkTo points the engine's UI channel at test channels. Frames and other +// messages are separated because an approval request carries a reply channel +// and so cannot be a frame. Pass nil for other to ignore them. +func sinkTo(e *codexEngine, frames chan codexFrame, other chan tea.Msg) { + e.mu.Lock() + defer e.mu.Unlock() + e.sink = func(msg tea.Msg) { + if cm, ok := msg.(codexMsg); ok { + frames <- cm.frame + return + } + if other != nil { + other <- msg + } + } +} + func TestCodexInitializeOpensAThread(t *testing.T) { fakeAppServer(t, echoServer) @@ -281,3 +300,147 @@ func TestCodexUpdateRendersAsADiffCard(t *testing.T) { t.Errorf("the card should show the change, got:\n%s", out) } } + +// A gated action reaches the shared approval pane, and the user's answer +// becomes the JSON-RPC decision codex is waiting for. +// +// The reply must go out on its own goroutine. The reader has to keep draining +// while the pane is up, or the item events that draw the very card being +// approved never arrive — a deadlock that looks like a frozen approval bar. +func TestCodexApprovalRoundTrip(t *testing.T) { + for _, c := range []struct { + name string + allow bool + want string + }{ + {"allow", true, codexApproval}, + {"deny", false, codexRefusal}, + } { + t.Run(c.name, func(t *testing.T) { + // A stand-in that raises one approval, then echoes whatever we send + // back so the test can read the decision off the wire. + fakeAppServer(t, ` +while IFS= read -r line; do + case "$line" in + *'"initialize"'*) printf '{"method":"item/commandExecution/requestApproval","id":0,"params":{"itemId":"exec-1","command":"rm -rf /tmp/x"}}\n' ;; + *'"decision"'*) printf '{"method":"cathode/test/echo","params":%s}\n' "$line" ;; + esac +done +`) + e, err := newCodexEngine(codexEngineConfig{Mode: "ask"}) + if err != nil { + t.Fatal(err) + } + defer e.Close() + + frames := make(chan codexFrame, 16) + other := make(chan tea.Msg, 16) + sinkTo(e, frames, other) + + go func() { _, _ = e.call("initialize", map[string]any{}) }() + + var req approvalReq + select { + case msg := <-other: + pa, ok := msg.(pendingApprovalMsg) + if !ok { + t.Fatalf("got %T, want the shared pendingApprovalMsg", msg) + } + req = pa.req + case <-time.After(5 * time.Second): + t.Fatal("the approval never reached the UI") + } + + if req.toolName != "rm -rf /tmp/x" { + t.Errorf("bar label = %q, want the command itself", req.toolName) + } + if req.toolUseID != "exec-1" { + t.Errorf("toolUseID = %q, want the itemId that pairs it with the card", req.toolUseID) + } + + req.reply <- approvalReply{allow: c.allow} + + select { + case f := <-frames: + if !strings.Contains(string(f.Params), `"decision":"`+c.want+`"`) { + t.Errorf("sent %s, want decision %q", f.Params, c.want) + } + case <-time.After(5 * time.Second): + t.Fatalf("no decision was sent; codex would wait forever") + } + }) + } +} + +// Two gated actions in flight must both be answered. +// +// The UI holds one pending approval, so an eager second push would overwrite +// the first and its request would never be replied to — and codex waits on a +// reply forever. claude cannot hit this because its approvals are pulled one at +// a time; codex pushes, so the engine serialises them. +func TestCodexApprovalsAreAnsweredOneAtATime(t *testing.T) { + fakeAppServer(t, ` +while IFS= read -r line; do + case "$line" in + *'"initialize"'*) + printf '{"method":"item/commandExecution/requestApproval","id":0,"params":{"itemId":"a","command":"first"}}\n' + printf '{"method":"item/commandExecution/requestApproval","id":1,"params":{"itemId":"b","command":"second"}}\n' ;; + *'"decision"'*) printf '{"method":"cathode/test/echo","params":%s}\n' "$line" ;; + esac +done +`) + e, err := newCodexEngine(codexEngineConfig{Mode: "ask"}) + if err != nil { + t.Fatal(err) + } + defer e.Close() + + frames := make(chan codexFrame, 16) + other := make(chan tea.Msg, 16) + sinkTo(e, frames, other) + go func() { _, _ = e.call("initialize", map[string]any{}) }() + + // Only one may be offered before it is answered. + first := awaitApproval(t, other) + select { + case msg := <-other: + if _, ok := msg.(pendingApprovalMsg); ok { + t.Fatal("a second approval was offered while the first was unanswered") + } + case <-time.After(300 * time.Millisecond): + } + + first.reply <- approvalReply{allow: true} + second := awaitApproval(t, other) + second.reply <- approvalReply{allow: false} + + // Both decisions must reach the wire, or one turn hangs. + seen := map[string]bool{} + deadline := time.After(5 * time.Second) + for len(seen) < 2 { + select { + case f := <-frames: + for _, d := range []string{codexApproval, codexRefusal} { + if strings.Contains(string(f.Params), `"decision":"`+d+`"`) { + seen[d] = true + } + } + case <-deadline: + t.Fatalf("only %d of 2 decisions were sent: %v", len(seen), seen) + } + } +} + +func awaitApproval(t *testing.T, ch chan tea.Msg) approvalReq { + t.Helper() + for { + select { + case msg := <-ch: + if pa, ok := msg.(pendingApprovalMsg); ok { + return pa.req + } + case <-time.After(5 * time.Second): + t.Fatal("no approval reached the UI") + } + } +} diff --git a/codexlive_test.go b/codexlive_test.go index ff20d12..76c20bd 100644 --- a/codexlive_test.go +++ b/codexlive_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" "time" + + tea "github.com/charmbracelet/bubbletea" ) // A live check against the real `codex` CLI, off by default because it spends a @@ -32,9 +34,7 @@ func TestCodexLiveRoundTrip(t *testing.T) { defer e.Close() frames := make(chan codexFrame, 256) - e.mu.Lock() - e.sink = func(f codexFrame) { frames <- f } - e.mu.Unlock() + sinkTo(e, frames, nil) if err := e.Initialize(); err != nil { t.Fatalf("Initialize: %v", err) @@ -76,39 +76,38 @@ func TestCodexLiveRoundTrip(t *testing.T) { } } -// What each mode actually does with a gated action, against the real CLI. +// What each mode does with a gated action, against the real CLI. // // The property that matters is that the turn always ENDS. codex blocks until a // server request is answered, so the failure guarded against is not a wrong // answer, it is no answer — which presents as a frozen UI with nothing in the // log. // -// The two rows also pin the current limit of this backend. In build mode codex -// asks for nothing and the action runs, so the backend is usable today. In ask -// mode every gated action is refused, because the approval pane is not wired to -// codex yet and granting silently would defeat the mode. +// build asks for nothing and the action runs. ask raises a real approval, which +// this answers the way the pane would. func TestCodexLiveGatedActionsAlwaysEndTheTurn(t *testing.T) { if os.Getenv("CATHODE_CODEX_LIVE") == "" { t.Skip("set CATHODE_CODEX_LIVE=1 to run against the real codex CLI") } for _, c := range []struct { - mode string - wantRefusal bool + mode string + wantPrompt bool + allow bool }{ - {"build", false}, - {"ask", true}, + {"build", false, false}, + {"ask", true, true}, } { t.Run(c.mode, func(t *testing.T) { - e, err := newCodexEngine(codexEngineConfig{Mode: c.mode, Cwd: t.TempDir()}) + dir := t.TempDir() + e, err := newCodexEngine(codexEngineConfig{Mode: c.mode, Cwd: dir}) if err != nil { t.Fatalf("spawn: %v", err) } defer e.Close() frames := make(chan codexFrame, 256) - e.mu.Lock() - e.sink = func(f codexFrame) { frames <- f } - e.mu.Unlock() + other := make(chan tea.Msg, 16) + sinkTo(e, frames, other) if err := e.Initialize(); err != nil { t.Fatalf("Initialize: %v", err) @@ -117,21 +116,31 @@ func TestCodexLiveGatedActionsAlwaysEndTheTurn(t *testing.T) { t.Fatalf("Send: %v", err) } - var refused bool + var asked bool deadline := time.After(3 * time.Minute) for { select { + case msg := <-other: + pa, ok := msg.(pendingApprovalMsg) + if !ok { + continue + } + asked = true + t.Logf("approval asked: %s", pa.req.toolName) + pa.req.reply <- approvalReply{allow: c.allow} case f := <-frames: - if f.Method == codexErrorMethod { - t.Logf("notice: %s", f.Params) - refused = true + if f.Method != "turn/completed" && f.Method != "turn/failed" { + continue } - if f.Method == "turn/completed" || f.Method == "turn/failed" { - if refused != c.wantRefusal { - t.Errorf("%s mode: refused=%v, want %v", c.mode, refused, c.wantRefusal) + if asked != c.wantPrompt { + t.Errorf("%s mode: asked=%v, want %v", c.mode, asked, c.wantPrompt) + } + if c.allow { + if _, err := os.Stat(filepath.Join(dir, "probe.txt")); err != nil { + t.Errorf("approved, but the file was not written: %v", err) } - return } + return case <-deadline: t.Fatal("the turn never ended — a server request went unanswered") } @@ -162,9 +171,7 @@ func TestCodexLiveEditRendersAsADiffCard(t *testing.T) { defer e.Close() frames := make(chan codexFrame, 256) - e.mu.Lock() - e.sink = func(f codexFrame) { frames <- f } - e.mu.Unlock() + sinkTo(e, frames, nil) if err := e.Initialize(); err != nil { t.Fatalf("Initialize: %v", err) @@ -214,9 +221,7 @@ func TestCodexLiveModelListReachesThePicker(t *testing.T) { defer e.Close() frames := make(chan codexFrame, 64) - e.mu.Lock() - e.sink = func(f codexFrame) { frames <- f } - e.mu.Unlock() + sinkTo(e, frames, nil) if err := e.Initialize(); err != nil { t.Fatalf("Initialize: %v", err) diff --git a/codexreader.go b/codexreader.go index 24d3917..552379a 100644 --- a/codexreader.go +++ b/codexreader.go @@ -7,6 +7,8 @@ import ( "bufio" "encoding/json" "strings" + + tea "github.com/charmbracelet/bubbletea" ) // The stdout side of the codex connection: one goroutine consuming frames and @@ -72,14 +74,19 @@ func (e *codexEngine) noteTurn(f codexFrame) { // emit forwards one frame to the sink, or holds it until one is registered. // The send happens outside the lock: a slow consumer must not block the reader, // which would stall every later frame behind it. -func (e *codexEngine) emit(f codexFrame) { +func (e *codexEngine) emit(f codexFrame) { e.emitMsg(codexMsg{frame: f}) } + +// emitMsg forwards any message to the sink, or holds it until one is +// registered. The send happens outside the lock: a slow consumer must not block +// the reader, which would stall every later message behind it. +func (e *codexEngine) emitMsg(msg tea.Msg) { e.mu.Lock() sink := e.sink if sink == nil { - e.backlog = append(e.backlog, f) + e.backlog = append(e.backlog, msg) e.mu.Unlock() return } e.mu.Unlock() - sink(f) + sink(msg) } diff --git a/commandlist.go b/commandlist.go index 0c3e0a1..bd07990 100644 --- a/commandlist.go +++ b/commandlist.go @@ -60,6 +60,9 @@ func slashCommands() []slashCmd { { name: "mcp", desc: "manage MCP servers — status, reconnect/enable/disable", + // The server list comes from claude's system/init line, and the + // subcommands are forwarded to claude's own /mcp. + only: []string{backendClaude}, exec: func(m *model, arg string) (model, tea.Cmd) { return m.mcpCommand(arg) }, @@ -121,6 +124,10 @@ func slashCommands() []slashCmd { { name: "sysprompt", desc: "toggle your standing instructions as claude's response style (on|off)", + // Delivered as a claude output style (outputstyle.go). codex has no + // equivalent, so the toggle would restart the session and change + // nothing. + only: []string{backendClaude}, exec: func(m *model, arg string) (model, tea.Cmd) { switch id := strings.TrimSpace(strings.ToLower(arg)); id { case sysPromptOn, sysPromptOff: diff --git a/commands.go b/commands.go index 5c635f8..58a4aaf 100644 --- a/commands.go +++ b/commands.go @@ -17,9 +17,31 @@ import ( type slashCmd struct { name string desc string + // only lists the backends this command works on. Empty means every one. + // + // A command that depends on something one backend has — claude's output + // styles, its MCP server list — is not merely useless elsewhere: offered + // and selected, it either does nothing or gets forwarded to the agent as a + // prompt, which is worse than not being there. Declaring it here keeps the + // palette, the help text and the dispatcher agreeing, because all three ask + // this one question. + only []string exec func(m *model, arg string) (model, tea.Cmd) } +// availableOn reports whether this command works on the given backend. +func (c slashCmd) availableOn(backend string) bool { + if len(c.only) == 0 { + return true + } + for _, b := range c.only { + if b == sessionBackend(backend) { + return true + } + } + return false +} + // runSlash dispatches "/name [arg]" against our in-process command table. // Returns handled=true when a command ran; handled=false when the line isn't a // slash command OR isn't one of ours — in the latter case the caller forwards @@ -34,19 +56,31 @@ func runSlash(m *model, line string) (model, tea.Cmd, bool) { name, arg, _ := strings.Cut(rest, " ") name = strings.ToLower(name) for _, c := range slashCommands() { - if c.name == name { - nm, cmd := c.exec(m, arg) - return nm, cmd, true + if c.name != name { + continue + } + if !c.availableOn(m.backend) { + // Handled, not forwarded. Falling through would send "/sysprompt" + // to the agent as a prompt, which reads as the command silently + // doing something odd rather than not existing here. + m.add(entInfo, "/"+c.name+" is not available on the "+agentName(m.backend)+" backend") + return *m, nil, true } + nm, cmd := c.exec(m, arg) + return nm, cmd, true } return *m, nil, false } -// slashItems projects the slash command table into picker rows. -func slashItems() []pickerItem { +// slashItems projects the slash command table into picker rows, omitting the +// commands this backend cannot run. +func slashItems(backend string) []pickerItem { cmds := slashCommands() items := make([]pickerItem, 0, len(cmds)) for _, c := range cmds { + if !c.availableOn(backend) { + continue + } items = append(items, pickerItem{id: c.name, title: "/" + c.name, subtitle: c.desc}) } sort.SliceStable(items, func(a, b int) bool { return items[a].title < items[b].title }) @@ -58,7 +92,7 @@ func slashItems() []pickerItem { // plugin commands), deduped by name with ours winning — ours run locally, the // rest are forwarded to claude on select. func (m *model) paletteItems() []pickerItem { - items := slashItems() + items := slashItems(m.backend) seen := make(map[string]bool, len(items)) for _, it := range items { seen[it.id] = true diff --git a/help.go b/help.go index 7a4ff02..1d0d28c 100644 --- a/help.go +++ b/help.go @@ -62,6 +62,9 @@ func helpText(backend string) string { b.WriteString(" ctrl+c clear the prompt · interrupt the turn · again to quit\n") b.WriteString("commands:\n") for _, c := range cmds { + if !c.availableOn(backend) { + continue + } b.WriteString(fmt.Sprintf(" /%-10s %s\n", c.name, c.desc)) } b.WriteString(" any other /command is forwarded to " + agentName(backend) + " (custom & plugin commands)\n") diff --git a/model.go b/model.go index 3347596..0e17c5f 100644 --- a/model.go +++ b/model.go @@ -351,5 +351,12 @@ type pendingApprovalMsg struct{ req approvalReq } // waitApproval blocks on the next permission request. Re-issued after each // decision so the next one is picked up. func waitApproval(a *Approvals) tea.Cmd { + // nil on codex, which raises approvals as requests on its own connection + // rather than through a queue this can block on. Returning nil keeps the + // shared approval keys (keys.go) working on both backends: they re-arm the + // waiter after every decision, and on codex there is nothing to re-arm. + if a == nil { + return nil + } return func() tea.Msg { return pendingApprovalMsg{req: <-a.pending} } }