Skip to content

Commit a2c19af

Browse files
fix(tui): tell an already-open MCP manager when background startup completes
Optional MCP registration runs on its own goroutine so a slow server cannot delay the first response, which means its result arrives with no user input behind it. Bubble Tea renders only in response to a message, so reporting late failures through a getter was not enough on its own: a manager opened while an optional server was still connecting kept showing the configuration-derived enabled state until unrelated input or a resize happened to redraw it. The startup's completion channel is now surfaced to the model, Init schedules a wait on it, and the resulting message rebuilds the MCP view state. Startup stays non-blocking, tool readiness is untouched, and unconfigured built-in defaults still produce no startup warning. The regression asserts on the rebuilt cache rather than on a render. Rendering calls mcpViewState, which invalidates on demand, so an overlay drawn after the message looks correct even when the handler does nothing: the first version of this test passed with the rebuild deleted, which is the failure mode it exists to catch.
1 parent 20faf82 commit a2c19af

6 files changed

Lines changed: 177 additions & 5 deletions

File tree

‎internal/cli/app.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,9 +1020,13 @@ func runInteractiveTUIWithSetup(stderr io.Writer, deps appDeps, permissionMode a
10201020
// are not known yet and a snapshot cannot carry them. Pulled instead, or
10211021
// they stay rendered from configuration alone: enabled, with no
10221022
// explanation, for a server that never connected.
1023-
MCPLateSkipped: optionalMCPRuntime.Skipped,
1024-
MCPPermissionStore: mcpPermissionStore,
1025-
MCPTokenStore: mcpTokenStore,
1023+
MCPLateSkipped: optionalMCPRuntime.Skipped,
1024+
// And a signal to rebuild when it finishes. Pulling alone leaves an
1025+
// already-open manager showing the configuration-derived state, because a
1026+
// background completion schedules no render of its own.
1027+
MCPStartupCompleted: optionalMCPRuntime.completed(),
1028+
MCPPermissionStore: mcpPermissionStore,
1029+
MCPTokenStore: mcpTokenStore,
10261030
MCPCommand: func(ctx context.Context, args []string) tui.MCPCommandResult {
10271031
if ctx == nil {
10281032
ctx = context.Background()

‎internal/cli/mcp_startup.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,16 @@ func (startup *optionalMCPStartup) closeRuntime() {
151151
// Discarding their failures here left every one of them rendered from
152152
// configuration alone, which reports a server that never connected as enabled --
153153
// the single thing that panel exists to prevent.
154+
// completed exposes the completion signal so an already-open surface can be
155+
// told to rebuild. Nil when there is no background registration, which is what
156+
// the consumer treats as "nothing to wait for".
157+
func (startup *optionalMCPStartup) completed() <-chan struct{} {
158+
if startup == nil {
159+
return nil
160+
}
161+
return startup.done
162+
}
163+
154164
func (startup *optionalMCPStartup) Skipped() []mcp.SkippedServer {
155165
if startup == nil {
156166
return nil

‎internal/tui/command_views.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,3 +774,20 @@ func (m model) skillsText() string {
774774
},
775775
})
776776
}
777+
778+
// mcpStartupCompletedMsg reports that the background MCP registration finished.
779+
type mcpStartupCompletedMsg struct{}
780+
781+
// waitForMCPStartupCompletion turns the completion channel into one message.
782+
//
783+
// Optional MCP registration runs on its own goroutine so a slow server cannot
784+
// delay the first response. That means its result arrives with no user input
785+
// behind it, and Bubble Tea renders only in response to a message, so without
786+
// this the manager an operator already has open keeps showing what the
787+
// configuration said until they happen to type something or resize the terminal.
788+
func waitForMCPStartupCompletion(done <-chan struct{}) tea.Cmd {
789+
return func() tea.Msg {
790+
<-done
791+
return mcpStartupCompletedMsg{}
792+
}
793+
}
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
package tui
2+
3+
import (
4+
"errors"
5+
"strings"
6+
"testing"
7+
8+
tea "charm.land/bubbletea/v2"
9+
10+
"github.com/Gitlawb/zero/internal/config"
11+
mcppkg "github.com/Gitlawb/zero/internal/mcp"
12+
)
13+
14+
// AN ALREADY-OPEN MANAGER HAS TO BE TOLD, NOT ASKED.
15+
//
16+
// Optional MCP registration runs on its own goroutine so a slow server cannot
17+
// delay the first response, which means its result lands with no user input
18+
// behind it. Bubble Tea renders only in response to a message, so a getter that
19+
// reports late failures is not enough on its own: an overlay opened while a
20+
// server was still connecting keeps rendering the configuration-derived enabled
21+
// state until unrelated input or a resize happens to redraw it.
22+
func TestAnOpenManagerRebuildsWhenBackgroundStartupCompletes(t *testing.T) {
23+
cfg := config.MCPConfig{Servers: map[string]config.MCPServerConfig{
24+
"optional": {URL: "https://optional.invalid/mcp"},
25+
}}
26+
var late []mcppkg.SkippedServer
27+
done := make(chan struct{})
28+
29+
m := &model{
30+
width: 120,
31+
mcpConfig: cfg,
32+
mcpStartupConfig: cfg,
33+
mcpManager: &mcpManagerState{},
34+
mcpLateSkipped: func() []mcppkg.SkippedServer { return late },
35+
mcpStartupCompleted: done,
36+
}
37+
38+
// Opened while the background registration is still in flight.
39+
before := m.mcpManagerOverlay(m.width)
40+
if !strings.Contains(before, "optional") {
41+
t.Fatalf("SETUP INVALID: the manager does not list the server:\n%s", before)
42+
}
43+
if strings.Contains(strings.ToLower(before), "failed") {
44+
t.Fatalf("SETUP INVALID: it already reads as failed before the result arrived:\n%s", before)
45+
}
46+
47+
// The background registration finishes and reports the failure. No key, no
48+
// resize, no command: only the completion.
49+
late = []mcppkg.SkippedServer{{Name: "optional", Err: errors.New("optional server refused the connection")}}
50+
close(done)
51+
52+
updated, _ := m.Update(mcpStartupCompletedMsg{})
53+
next, ok := updated.(model)
54+
if !ok {
55+
t.Fatalf("Update returned %T, want a model", updated)
56+
}
57+
58+
// Asserted on the CACHE, not by rendering. Rendering calls mcpViewState,
59+
// which invalidates on demand, so an overlay drawn after the message would
60+
// look right even if the handler did nothing: the test would pass for the
61+
// wrong reason and stop pinning the handler at all. The cache being fresh
62+
// BEFORE anything renders is what says the completion was consumed.
63+
var cached string
64+
for _, server := range next.mcpViewStateCache.Servers {
65+
if server.Name == "optional" {
66+
cached = server.State + "|" + server.Error
67+
}
68+
}
69+
if !strings.Contains(cached, "failed") {
70+
t.Errorf("the completion did not rebuild the view state: %q", cached)
71+
}
72+
if !strings.Contains(cached, "refused the connection") {
73+
t.Errorf("the rebuilt state carries no reason: %q", cached)
74+
}
75+
76+
after := next.mcpManagerOverlay(next.width)
77+
if !strings.Contains(strings.ToLower(after), "failed") {
78+
t.Errorf("the open manager still shows the configured state after the background failure:\n%s", after)
79+
}
80+
if !strings.Contains(after, "refused the connection") {
81+
t.Errorf("the reason never reached the open manager:\n%s", after)
82+
}
83+
}
84+
85+
// And Init actually schedules the wait, or the message above would never be
86+
// produced in a real session.
87+
func TestInitWaitsForBackgroundStartupCompletion(t *testing.T) {
88+
done := make(chan struct{})
89+
m := model{mcpStartupCompleted: done}
90+
close(done)
91+
92+
cmd := m.Init()
93+
if cmd == nil {
94+
t.Fatal("Init scheduled nothing")
95+
}
96+
if !producesMCPStartupCompletion(cmd) {
97+
t.Error("Init never scheduled the wait for background MCP startup, so a completed registration reaches no open surface")
98+
}
99+
}
100+
101+
// producesMCPStartupCompletion runs a batched command tree far enough to see
102+
// whether the completion wait is part of it.
103+
func producesMCPStartupCompletion(cmd tea.Cmd) bool {
104+
if cmd == nil {
105+
return false
106+
}
107+
switch msg := cmd().(type) {
108+
case mcpStartupCompletedMsg:
109+
return true
110+
case tea.BatchMsg:
111+
for _, sub := range msg {
112+
if producesMCPStartupCompletion(sub) {
113+
return true
114+
}
115+
}
116+
}
117+
return false
118+
}

‎internal/tui/model.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@ type model struct {
114114
// against, so the same invalidation the startup snapshot gets can be applied
115115
// to them. mcpLateSkippedCount is what the cached view state was built from,
116116
// so a new arrival invalidates the cache without a config change.
117-
mcpLateSkipped func() []internalmcp.SkippedServer
117+
mcpLateSkipped func() []internalmcp.SkippedServer
118+
// mcpStartupCompleted closes when the background registration finishes, so an
119+
// already-open manager can be told to rebuild rather than waiting for input.
120+
mcpStartupCompleted <-chan struct{}
118121
mcpStartupConfig config.MCPConfig
119122
mcpLateSkippedCount int
120123
mcpPermissionStore *internalmcp.PermissionStore
@@ -1009,6 +1012,7 @@ func newModel(ctx context.Context, options Options) model {
10091012
mcpConfig: options.MCPConfig,
10101013
mcpSkipped: options.MCPSkipped,
10111014
mcpLateSkipped: options.MCPLateSkipped,
1015+
mcpStartupCompleted: options.MCPStartupCompleted,
10121016
mcpStartupConfig: options.MCPConfig,
10131017
mcpSkippedCredentials: mcpCredentialFingerprint(options.MCPTokenStore.SecretValues()),
10141018
mcpPermissionStore: options.MCPPermissionStore,
@@ -1161,6 +1165,9 @@ func (m model) armComposerBlink() (model, tea.Cmd) {
11611165

11621166
func (m model) Init() tea.Cmd {
11631167
cmds := []tea.Cmd{textinput.Blink, composerBlinkCmd(m.composerBlinkSeq)}
1168+
if m.mcpStartupCompleted != nil {
1169+
cmds = append(cmds, waitForMCPStartupCompletion(m.mcpStartupCompleted))
1170+
}
11641171
if m.petAnimation != nil && !m.reducedMotion {
11651172
cmds = append(cmds, petTickCmd(m.petTickSeq, m.petFrameDelay()))
11661173
}
@@ -1494,6 +1501,13 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
14941501
return m, dragEdgeScrollTickCmd(m.edgeScrollSeq)
14951502
case providerWizardOAuthMsg:
14961503
return m.applyProviderWizardOAuth(msg)
1504+
case mcpStartupCompletedMsg:
1505+
// The background registration is done, so the observations it produced are
1506+
// available now. Rebuilding HERE is what makes an already-open manager show
1507+
// them: nothing else schedules a render once Bubble Tea is idle, so a getter
1508+
// that reports late failures is not on its own enough.
1509+
m.refreshMCPViewState()
1510+
return m, nil
14971511
case aimlapiOnboardMsg:
14981512
return m.applyAimlapiOnboard(msg)
14991513
case aimlapiExistingBalanceMsg:

‎internal/tui/options.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,16 @@ type Options struct {
6565
// one cannot delay the first response, which means their failures land after
6666
// this surface exists; MCPSkipped is a snapshot and cannot carry them.
6767
// Optional: nil means every failure was already known.
68-
MCPLateSkipped func() []mcp.SkippedServer
68+
MCPLateSkipped func() []mcp.SkippedServer
69+
// MCPStartupCompleted closes when the background MCP registration has
70+
// finished, so an ALREADY-OPEN manager can be told to rebuild.
71+
//
72+
// A getter that reports late failures is not enough on its own: nothing
73+
// schedules another render, so an overlay opened while an optional server was
74+
// still connecting keeps showing the configuration-derived enabled state until
75+
// unrelated input or a resize happens to redraw it. Optional: nil means there
76+
// is no background registration to wait for.
77+
MCPStartupCompleted <-chan struct{}
6978
MCPPermissionStore *mcp.PermissionStore
7079
MCPTokenStore *mcp.TokenStore
7180
MCPCommand func(context.Context, []string) MCPCommandResult

0 commit comments

Comments
 (0)