Skip to content

Commit 1c107f7

Browse files
intel352claude
andcommitted
fix: resolve all outstanding review issues
- Fix critical streaming bug: chat.go now chains tea.Cmd to drain all events from the channel, not just the first one - Handle all ChatEvent types in TUI (ToolCallStart, ToolCallResult, Permission, AgentSpawned, AgentMessage) with ToolCallListModel - Wire cancel function for Escape key to abort in-flight requests - Wrap AddProvider in transaction (clear-defaults + insert atomic) - Wire team CLI commands to daemon via gRPC (start, status) - Add StartTeam/GetTeamStatus to gRPC client - Fix release.yml go-version: 1.22 → 1.26 to match go.mod - Remove dead code in skills.go (unused seen map) - Fix hooks_test.go HOME env to avoid test pollution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f927b14 commit 1c107f7

7 files changed

Lines changed: 198 additions & 29 deletions

File tree

.github/workflows/release.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
fetch-depth: 0
1414
- uses: actions/setup-go@v5
1515
with:
16-
go-version: "1.22"
16+
go-version: "1.26"
1717
- uses: goreleaser/goreleaser-action@v6
1818
with:
1919
version: "~> v2"

cmd/ratchet/cmd_team.go

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,66 @@
11
package main
22

33
import (
4+
"context"
45
"fmt"
6+
"os"
7+
8+
"github.com/GoCodeAlone/ratchet-cli/internal/client"
9+
pb "github.com/GoCodeAlone/ratchet-cli/internal/proto"
510
)
611

712
func handleTeam(args []string) {
813
if len(args) == 0 {
9-
fmt.Println("Usage: ratchet team <start|status>")
14+
fmt.Println("Usage: ratchet team <start|status> [args...]")
1015
return
1116
}
17+
18+
c, err := client.EnsureDaemon()
19+
if err != nil {
20+
fmt.Fprintf(os.Stderr, "error: %v\n", err)
21+
os.Exit(1)
22+
}
23+
defer c.Close()
24+
1225
switch args[0] {
1326
case "start":
14-
fmt.Println("team start: not yet implemented")
27+
if len(args) < 2 {
28+
fmt.Println("Usage: ratchet team start \"task description\"")
29+
return
30+
}
31+
task := args[1]
32+
stream, err := c.StartTeam(context.Background(), &pb.StartTeamReq{Task: task})
33+
if err != nil {
34+
fmt.Fprintf(os.Stderr, "error: %v\n", err)
35+
os.Exit(1)
36+
}
37+
for event := range stream {
38+
switch e := event.Event.(type) {
39+
case *pb.TeamEvent_AgentSpawned:
40+
fmt.Printf("[spawned] %s (%s)\n", e.AgentSpawned.AgentName, e.AgentSpawned.Role)
41+
case *pb.TeamEvent_Token:
42+
fmt.Print(e.Token.Content)
43+
case *pb.TeamEvent_AgentMessage:
44+
fmt.Printf("[%s → %s] %s\n", e.AgentMessage.FromAgent, e.AgentMessage.ToAgent, e.AgentMessage.Content)
45+
case *pb.TeamEvent_Complete:
46+
fmt.Printf("\nTeam complete: %s\n", e.Complete.Summary)
47+
case *pb.TeamEvent_Error:
48+
fmt.Fprintf(os.Stderr, "error: %s\n", e.Error.Message)
49+
}
50+
}
1551
case "status":
16-
fmt.Println("team status: not yet implemented")
52+
resp, err := c.GetTeamStatus(context.Background(), "")
53+
if err != nil {
54+
fmt.Fprintf(os.Stderr, "error: %v\n", err)
55+
os.Exit(1)
56+
}
57+
fmt.Printf("Team: %s Status: %s Task: %s\n", resp.TeamId, resp.Status, resp.Task)
58+
if len(resp.Agents) > 0 {
59+
fmt.Printf("%-20s %-10s %s\n", "NAME", "STATUS", "MODEL")
60+
for _, a := range resp.Agents {
61+
fmt.Printf("%-20s %-10s %s\n", a.Name, a.Status, a.Model)
62+
}
63+
}
1764
default:
1865
fmt.Printf("unknown team command: %s\n", args[0])
1966
}

internal/client/client.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,37 @@ func (c *Client) Shutdown(ctx context.Context) error {
164164
_, err := c.daemon.Shutdown(ctx, &pb.Empty{})
165165
return err
166166
}
167+
168+
// StartTeam starts a team and returns a channel of TeamEvents.
169+
func (c *Client) StartTeam(ctx context.Context, req *pb.StartTeamReq) (<-chan *pb.TeamEvent, error) {
170+
stream, err := c.daemon.StartTeam(ctx, req)
171+
if err != nil {
172+
return nil, err
173+
}
174+
175+
ch := make(chan *pb.TeamEvent, 64)
176+
go func() {
177+
defer close(ch)
178+
for {
179+
event, err := stream.Recv()
180+
if err == io.EOF {
181+
return
182+
}
183+
if err != nil {
184+
ch <- &pb.TeamEvent{
185+
Event: &pb.TeamEvent_Error{
186+
Error: &pb.ErrorEvent{Message: err.Error()},
187+
},
188+
}
189+
return
190+
}
191+
ch <- event
192+
}
193+
}()
194+
return ch, nil
195+
}
196+
197+
// GetTeamStatus returns the status of the active team.
198+
func (c *Client) GetTeamStatus(ctx context.Context, teamID string) (*pb.TeamStatus, error) {
199+
return c.daemon.GetTeamStatus(ctx, &pb.TeamStatusReq{TeamId: teamID})
200+
}

internal/daemon/service.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,36 +110,43 @@ func (s *Service) AddProvider(ctx context.Context, req *pb.AddProviderReq) (*pb.
110110
id := uuid.New().String()
111111
secretName := fmt.Sprintf("provider_%s", req.Alias)
112112

113-
// Clear existing default if this is the new default
113+
isDefault := 0
114114
if req.IsDefault {
115-
if _, err := s.engine.DB.ExecContext(ctx, `UPDATE llm_providers SET is_default = 0`); err != nil {
116-
return nil, status.Errorf(codes.Internal, "clear defaults: %v", err)
117-
}
115+
isDefault = 1
118116
}
119117

120-
isDefault := 0
118+
tx, err := s.engine.DB.BeginTx(ctx, nil)
119+
if err != nil {
120+
return nil, status.Errorf(codes.Internal, "begin transaction: %v", err)
121+
}
122+
defer tx.Rollback()
123+
124+
// Clear existing default if this is the new default
121125
if req.IsDefault {
122-
isDefault = 1
126+
if _, err := tx.ExecContext(ctx, `UPDATE llm_providers SET is_default = 0`); err != nil {
127+
return nil, status.Errorf(codes.Internal, "clear defaults: %v", err)
128+
}
123129
}
124130

125131
// DB insert before secret store to avoid orphaned secrets on constraint failure
126-
_, err := s.engine.DB.ExecContext(ctx,
132+
if _, err := tx.ExecContext(ctx,
127133
`INSERT INTO llm_providers (id, alias, type, model, secret_name, base_url, max_tokens, is_default) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
128134
id, req.Alias, req.Type, req.Model, secretName, req.BaseUrl, req.MaxTokens, isDefault,
129-
)
130-
if err != nil {
135+
); err != nil {
131136
return nil, status.Errorf(codes.Internal, "insert provider: %v", err)
132137
}
133138

134139
// Store API key after successful insert
135140
if req.ApiKey != "" {
136141
if err := s.engine.SecretsProvider.Set(ctx, secretName, req.ApiKey); err != nil {
137-
// Best-effort rollback
138-
s.engine.DB.ExecContext(ctx, `DELETE FROM llm_providers WHERE alias = ?`, req.Alias)
139142
return nil, status.Errorf(codes.Internal, "store api key: %v", err)
140143
}
141144
}
142145

146+
if err := tx.Commit(); err != nil {
147+
return nil, status.Errorf(codes.Internal, "commit: %v", err)
148+
}
149+
143150
s.engine.ProviderRegistry.InvalidateCacheAlias(req.Alias)
144151

145152
return &pb.Provider{

internal/hooks/hooks_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
func TestLoadEmpty(t *testing.T) {
1010
tmp := t.TempDir()
11+
t.Setenv("HOME", tmp)
1112
cfg, err := Load(tmp)
1213
if err != nil {
1314
t.Fatal(err)

internal/skills/skills.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ func Discover(workingDir string) []Skill {
2323
filepath.Join(workingDir, ".ratchet", "skills"),
2424
}
2525

26-
seen := make(map[string]bool)
2726
var skills []Skill
2827

2928
for _, dir := range searchDirs {
@@ -48,16 +47,13 @@ func Discover(workingDir string) []Skill {
4847
if err != nil {
4948
continue
5049
}
51-
// Later paths (project-level) override earlier paths (global)
52-
seen[name] = true
5350
skills = append(skills, Skill{
5451
Name: name,
5552
Path: path,
5653
Content: string(content),
5754
})
5855
}
5956
}
60-
_ = seen
6157
return dedup(skills)
6258
}
6359

internal/tui/pages/chat.go

Lines changed: 94 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,15 @@ import (
1414
)
1515

1616
// ChatEventMsg wraps an incoming proto ChatEvent for the TUI.
17+
// The channel is carried so handleChatEvent can schedule the next read.
1718
type ChatEventMsg struct {
1819
Event *pb.ChatEvent
20+
ch <-chan *pb.ChatEvent
1921
}
2022

23+
// chatStreamDoneMsg signals the event channel was closed.
24+
type chatStreamDoneMsg struct{}
25+
2126
type ChatModel struct {
2227
client *client.Client
2328
sessionID string
@@ -27,6 +32,7 @@ type ChatModel struct {
2732
viewport viewport.Model
2833
input components.InputModel
2934
statusBar components.StatusBar
35+
toolCalls components.ToolCallListModel
3036
messages []components.Message
3137
streaming string // current streaming response
3238
width int
@@ -48,6 +54,7 @@ func NewChat(c *client.Client, sessionID string, t theme.Theme, dark bool) ChatM
4854
viewport: vp,
4955
input: input,
5056
statusBar: statusBar,
57+
toolCalls: components.NewToolCallList(),
5158
ctx: context.Background(),
5259
}
5360
}
@@ -65,6 +72,13 @@ func (m ChatModel) Update(msg tea.Msg) (ChatModel, tea.Cmd) {
6572
m.height = msg.Height
6673
m.relayout()
6774

75+
case tea.KeyPressMsg:
76+
// Cancel in-flight streaming with Escape
77+
if msg.String() == "esc" && m.cancelChat != nil {
78+
m.cancelChat()
79+
m.cancelChat = nil
80+
}
81+
6882
case components.SubmitMsg:
6983
// Add user message and send to daemon
7084
m.messages = append(m.messages, components.Message{
@@ -76,7 +90,19 @@ func (m ChatModel) Update(msg tea.Msg) (ChatModel, tea.Cmd) {
7690
cmds = append(cmds, m.sendMessage(msg.Content))
7791

7892
case ChatEventMsg:
79-
cmds = append(cmds, m.handleChatEvent(msg.Event))
93+
cmds = append(cmds, m.handleChatEvent(msg)...)
94+
95+
case chatStreamDoneMsg:
96+
// Stream channel closed without a Complete event
97+
if m.streaming != "" {
98+
m.messages = append(m.messages, components.Message{
99+
Role: components.RoleAssistant,
100+
Content: m.streaming,
101+
})
102+
m.streaming = ""
103+
m.refreshViewport()
104+
}
105+
m.cancelChat = nil
80106
}
81107

82108
var inputCmd, vpCmd tea.Cmd
@@ -107,6 +133,11 @@ func (m *ChatModel) refreshViewport() {
107133
sb.WriteString(msg.Render(m.theme, m.width, m.dark))
108134
sb.WriteString("\n")
109135
}
136+
// Render any active tool calls
137+
toolView := m.toolCalls.View(m.theme, m.width)
138+
if toolView != "" {
139+
sb.WriteString(toolView)
140+
}
110141
if m.streaming != "" {
111142
assistantMsg := components.Message{
112143
Role: components.RoleAssistant,
@@ -124,7 +155,7 @@ func (m ChatModel) sendMessage(content string) tea.Cmd {
124155
return nil
125156
}
126157
ctx, cancel := context.WithCancel(m.ctx)
127-
_ = cancel // stored for potential cancellation
158+
m.cancelChat = cancel
128159

129160
ch, err := m.client.SendMessage(ctx, m.sessionID, content)
130161
if err != nil {
@@ -135,24 +166,69 @@ func (m ChatModel) sendMessage(content string) tea.Cmd {
135166
}}
136167
}
137168

138-
// Return first event; subsequent events come via streaming
169+
// Read first event and carry channel for subsequent reads
139170
event, ok := <-ch
140171
if !ok {
141-
return nil
172+
return chatStreamDoneMsg{}
173+
}
174+
return ChatEventMsg{Event: event, ch: ch}
175+
}
176+
}
177+
178+
// nextEvent returns a Cmd that reads the next event from the channel.
179+
func nextEvent(ch <-chan *pb.ChatEvent) tea.Cmd {
180+
return func() tea.Msg {
181+
event, ok := <-ch
182+
if !ok {
183+
return chatStreamDoneMsg{}
142184
}
143-
return ChatEventMsg{Event: event}
185+
return ChatEventMsg{Event: event, ch: ch}
144186
}
145187
}
146188

147-
func (m ChatModel) handleChatEvent(event *pb.ChatEvent) tea.Cmd {
189+
func (m *ChatModel) handleChatEvent(msg ChatEventMsg) []tea.Cmd {
190+
event := msg.Event
148191
if event == nil {
149192
return nil
150193
}
194+
195+
var cmds []tea.Cmd
196+
151197
switch e := event.Event.(type) {
152198
case *pb.ChatEvent_Token:
153199
m.streaming += e.Token.Content
154200
m.refreshViewport()
155-
return nil
201+
202+
case *pb.ChatEvent_ToolStart:
203+
m.toolCalls.AddCard(e.ToolStart.CallId, e.ToolStart.ToolName, e.ToolStart.ArgumentsJson)
204+
m.refreshViewport()
205+
206+
case *pb.ChatEvent_ToolResult:
207+
m.toolCalls.UpdateResult(e.ToolResult.CallId, e.ToolResult.ResultJson, e.ToolResult.Success)
208+
m.refreshViewport()
209+
210+
case *pb.ChatEvent_Permission:
211+
// Show permission prompt inline
212+
m.messages = append(m.messages, components.Message{
213+
Role: components.RoleTool,
214+
Content: "Permission required: " + e.Permission.ToolName + " — " + e.Permission.Description,
215+
})
216+
m.refreshViewport()
217+
218+
case *pb.ChatEvent_AgentSpawned:
219+
m.messages = append(m.messages, components.Message{
220+
Role: components.RoleTool,
221+
Content: "Agent spawned: " + e.AgentSpawned.AgentName + " (" + e.AgentSpawned.Role + ")",
222+
})
223+
m.refreshViewport()
224+
225+
case *pb.ChatEvent_AgentMessage:
226+
m.messages = append(m.messages, components.Message{
227+
Role: components.RoleTool,
228+
Content: "[" + e.AgentMessage.FromAgent + "] " + e.AgentMessage.Content,
229+
})
230+
m.refreshViewport()
231+
156232
case *pb.ChatEvent_Complete:
157233
if m.streaming != "" {
158234
m.messages = append(m.messages, components.Message{
@@ -162,17 +238,25 @@ func (m ChatModel) handleChatEvent(event *pb.ChatEvent) tea.Cmd {
162238
m.streaming = ""
163239
m.refreshViewport()
164240
}
165-
return nil
241+
m.cancelChat = nil
242+
return cmds // don't schedule next read — stream is done
243+
166244
case *pb.ChatEvent_Error:
167245
m.messages = append(m.messages, components.Message{
168246
Role: components.RoleTool,
169247
Content: "error: " + e.Error.Message,
170248
})
171249
m.streaming = ""
172250
m.refreshViewport()
173-
return nil
251+
m.cancelChat = nil
252+
return cmds // don't schedule next read — stream is done
253+
}
254+
255+
// Schedule read of next event from the channel
256+
if msg.ch != nil {
257+
cmds = append(cmds, nextEvent(msg.ch))
174258
}
175-
return nil
259+
return cmds
176260
}
177261

178262
func (m ChatModel) View(t theme.Theme) string {

0 commit comments

Comments
 (0)