fix(agent): address code review feedback
- Replace deprecated strings.Title with manual capitalize - Fix race: set codexClient.onMessage before starting reader goroutine - Remove unused msgCh parameter from codexClient.handleLine - Route agent stderr through logger instead of dumping to os.Stderr - Use deterministic agent order in ensurePaired (prefer codex) - Increase message channel buffer from 64 to 256 - Rename test to match function rename (buildPrompt) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bb45f17cf9
commit
0d9b687d92
4 changed files with 47 additions and 25 deletions
|
|
@ -263,7 +263,7 @@ func (d *daemon) registerRuntimes(ctx context.Context) ([]daemonRuntime, error)
|
|||
continue
|
||||
}
|
||||
runtimes = append(runtimes, map[string]string{
|
||||
"name": fmt.Sprintf("Local %s", strings.Title(name)),
|
||||
"name": fmt.Sprintf("Local %s", strings.ToUpper(name[:1])+name[1:]),
|
||||
"type": name,
|
||||
"version": version,
|
||||
"status": "online",
|
||||
|
|
@ -293,13 +293,15 @@ func (d *daemon) registerRuntimes(ctx context.Context) ([]daemonRuntime, error)
|
|||
}
|
||||
|
||||
func (d *daemon) ensurePaired(ctx context.Context) (string, error) {
|
||||
// Use the first available agent for the pairing session metadata.
|
||||
// Use a deterministic agent for the pairing session metadata (prefer codex for backward compat).
|
||||
var firstName string
|
||||
var firstEntry agentEntry
|
||||
for name, entry := range d.cfg.Agents {
|
||||
firstName = name
|
||||
firstEntry = entry
|
||||
break
|
||||
for _, preferred := range []string{"codex", "claude"} {
|
||||
if entry, ok := d.cfg.Agents[preferred]; ok {
|
||||
firstName = preferred
|
||||
firstEntry = entry
|
||||
break
|
||||
}
|
||||
}
|
||||
version, err := agent.DetectVersion(ctx, firstEntry.Path)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ func TestResolveTaskWorkdirUsesRepoPathWhenPresent(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestBuildCodexPromptIncludesIssueAndSkills(t *testing.T) {
|
||||
func TestBuildPromptIncludesIssueAndSkills(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
prompt := buildPrompt(daemonTask{
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import (
|
|||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"log"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
|
|
@ -64,7 +65,7 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt
|
|||
cancel()
|
||||
return nil, fmt.Errorf("claude stdin pipe: %w", err)
|
||||
}
|
||||
cmd.Stderr = os.Stderr
|
||||
cmd.Stderr = newLogWriter(b.cfg.Logger, "[claude:stderr] ")
|
||||
|
||||
if err := cmd.Start(); err != nil {
|
||||
cancel()
|
||||
|
|
@ -73,7 +74,7 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt
|
|||
|
||||
b.cfg.Logger.Printf("[claude] started pid=%d cwd=%s model=%s", cmd.Process.Pid, opts.Cwd, opts.Model)
|
||||
|
||||
msgCh := make(chan Message, 64)
|
||||
msgCh := make(chan Message, 256)
|
||||
resCh := make(chan Result, 1)
|
||||
|
||||
go func() {
|
||||
|
|
@ -303,6 +304,8 @@ func trySend(ch chan<- Message, msg Message) {
|
|||
select {
|
||||
case ch <- msg:
|
||||
default:
|
||||
// Channel full — drop message. Final output is accumulated separately
|
||||
// in Result.Output, so only streaming consumers are affected.
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -322,3 +325,21 @@ func detectCLIVersion(ctx context.Context, execPath string) (string, error) {
|
|||
}
|
||||
return strings.TrimSpace(string(data)), nil
|
||||
}
|
||||
|
||||
// logWriter adapts a *log.Logger to an io.Writer for capturing stderr.
|
||||
type logWriter struct {
|
||||
logger *log.Logger
|
||||
prefix string
|
||||
}
|
||||
|
||||
func newLogWriter(logger *log.Logger, prefix string) *logWriter {
|
||||
return &logWriter{logger: logger, prefix: prefix}
|
||||
}
|
||||
|
||||
func (w *logWriter) Write(p []byte) (int, error) {
|
||||
text := strings.TrimSpace(string(p))
|
||||
if text != "" {
|
||||
w.logger.Printf("%s%s", w.prefix, text)
|
||||
}
|
||||
return len(p), nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ import (
|
|||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
"sync"
|
||||
|
|
@ -49,7 +48,7 @@ func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOpti
|
|||
cancel()
|
||||
return nil, fmt.Errorf("codex stdin pipe: %w", err)
|
||||
}
|
||||
cmd.Stderr = os.Stderr
|
||||
cmd.Stderr = newLogWriter(b.cfg.Logger, "[codex:stderr] ")
|
||||
|
||||
if err := cmd.Start(); err != nil {
|
||||
cancel()
|
||||
|
|
@ -58,15 +57,24 @@ func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOpti
|
|||
|
||||
b.cfg.Logger.Printf("[codex] started app-server pid=%d cwd=%s", cmd.Process.Pid, opts.Cwd)
|
||||
|
||||
msgCh := make(chan Message, 256)
|
||||
resCh := make(chan Result, 1)
|
||||
|
||||
var output strings.Builder
|
||||
|
||||
c := &codexClient{
|
||||
cfg: b.cfg,
|
||||
stdin: stdin,
|
||||
pending: make(map[int]*pendingRPC),
|
||||
// Set onMessage before starting the reader goroutine to avoid a race.
|
||||
onMessage: func(msg Message) {
|
||||
if msg.Type == MessageText {
|
||||
output.WriteString(msg.Content)
|
||||
}
|
||||
trySend(msgCh, msg)
|
||||
},
|
||||
}
|
||||
|
||||
msgCh := make(chan Message, 64)
|
||||
resCh := make(chan Result, 1)
|
||||
|
||||
// Start reading stdout in background
|
||||
go func() {
|
||||
scanner := bufio.NewScanner(stdout)
|
||||
|
|
@ -76,7 +84,7 @@ func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOpti
|
|||
if line == "" {
|
||||
continue
|
||||
}
|
||||
c.handleLine(line, msgCh)
|
||||
c.handleLine(line)
|
||||
}
|
||||
c.closeAllPending(fmt.Errorf("codex process exited"))
|
||||
}()
|
||||
|
|
@ -94,15 +102,6 @@ func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOpti
|
|||
startTime := time.Now()
|
||||
finalStatus := "completed"
|
||||
var finalError string
|
||||
var output strings.Builder
|
||||
|
||||
// Drain messages to accumulate output
|
||||
c.onMessage = func(msg Message) {
|
||||
if msg.Type == MessageText {
|
||||
output.WriteString(msg.Content)
|
||||
}
|
||||
trySend(msgCh, msg)
|
||||
}
|
||||
|
||||
// 1. Initialize handshake
|
||||
_, err := c.request(runCtx, "initialize", map[string]any{
|
||||
|
|
@ -308,7 +307,7 @@ func (c *codexClient) closeAllPending(err error) {
|
|||
}
|
||||
}
|
||||
|
||||
func (c *codexClient) handleLine(line string, msgCh chan<- Message) {
|
||||
func (c *codexClient) handleLine(line string) {
|
||||
var raw map[string]json.RawMessage
|
||||
if err := json.Unmarshal([]byte(line), &raw); err != nil {
|
||||
return
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue