Skip to content

Commit 8813873

Browse files
authored
Phase 2: Architecture & maintainability improvements (#4)
* refactor: consolidate 6 per-session maps into sessionHandle struct Replaces sessions, ptys, cmds, doneChans, frameDetectors, and responders maps with a single handles map[string]*sessionHandle. Eliminates the risk of map synchronization drift across handlers. * refactor: remove Session struct, promote fields into sessionHandle Eliminates the intermediate Session struct that duplicated SessionMeta fields. All session state now lives directly in sessionHandle, removing one layer of indirection and a source of state drift. * refactor: extract waitForSettle helper from handleSnapshot Deduplicates the two near-identical settle polling loops in handleSnapshot into a shared helper function. * refactor: consolidate send+wait+read into Client.Exec Extracts the duplicated "send input, wait for output, return new content" pattern from cmd/exec.go and mcp/tools.go into a single Client.Exec method. Callers now use daemon.ExecOptions instead of manually orchestrating Read/Send/ForOutput. Also refactors MCP ToolRegistry to use registration-based pattern: adding a tool is now a single register() call instead of coordinated edits to List() schema, Call() switch, and handler method. * refactor: add protocol versioning between daemon and client Client sends ProtocolVersion=1 with every request. Daemon rejects mismatched versions with a clear error message telling the user to restart the daemon. Version 0 (omitted) is accepted for backward compatibility during rollout. * feat: add --log-file flag and panic recovery to daemon Daemon previously ran with stdout/stderr discarded, making crash debugging impossible. Now supports --log-file to capture logs and redirect stderr. Added panic recovery in handleConn and captureOutput goroutines to log stack traces instead of crashing silently. Also fixes unparam lint: waitForSettle return value was unused.
1 parent 9460c76 commit 8813873

File tree

7 files changed

+567
-570
lines changed

7 files changed

+567
-570
lines changed

.claude/skills/tui-test/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ The lead agent (you) does the following:
3838

3939
### Phase 2: Parallel Testing
4040

41-
Spawn one teammate per app (max 5-6 concurrent to avoid system overload). Use `general-purpose` agent type with `bypassPermissions` mode.
41+
Spawn one teammate per app (max 5-6 concurrent to avoid system overload). Use `general-purpose` agent type with `bypassPermissions` mode and `model: "sonnet"` (cheaper, sufficient for mechanical test execution).
4242

4343
Each teammate gets:
4444
- The **Teammate Test Protocol** section below (copy it fully into the prompt)

cmd/daemon.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cmd
22

33
import (
44
"fmt"
5+
"log"
56
"os"
67
"os/signal"
78
"path/filepath"
@@ -22,6 +23,7 @@ var (
2223
daemonDataDirFlag string
2324
daemonMemoryBackend bool
2425
daemonStoppedTTLFlag string
26+
daemonLogFileFlag string
2527
)
2628

2729
var daemonCmd = &cobra.Command{
@@ -42,13 +44,27 @@ func init() {
4244
"Use in-memory storage instead of file-based (no persistence)")
4345
daemonCmd.Flags().StringVar(&daemonStoppedTTLFlag, "stopped-ttl", "",
4446
"Auto-cleanup stopped sessions after duration (e.g., 5m, 1h, 24h)")
47+
daemonCmd.Flags().StringVar(&daemonLogFileFlag, "log-file", "",
48+
"Write daemon logs to file (default: discard)")
4549
}
4650

4751
func runDaemon(cmd *cobra.Command, args []string) error {
4852
if daemonMCPFlag {
4953
return runMCPServer()
5054
}
5155

56+
if daemonLogFileFlag != "" {
57+
f, err := os.OpenFile(daemonLogFileFlag, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
58+
if err != nil {
59+
return fmt.Errorf("open log file: %w", err)
60+
}
61+
defer f.Close()
62+
log.SetOutput(f)
63+
log.SetFlags(log.Ldate | log.Ltime | log.Lmicroseconds)
64+
os.Stderr = f
65+
log.Println("daemon starting")
66+
}
67+
5268
var opts []daemon.ServerOption
5369

5470
if daemonDataDirFlag == "" {

cmd/exec.go

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
"github.com/schovi/shelli/internal/ansi"
99
"github.com/schovi/shelli/internal/daemon"
10-
"github.com/schovi/shelli/internal/wait"
1110
"github.com/spf13/cobra"
1211
)
1312

@@ -59,49 +58,35 @@ func runExec(cmd *cobra.Command, args []string) error {
5958
return fmt.Errorf("daemon: %w", err)
6059
}
6160

62-
_, startPos, err := client.Read(name, "all", 0, 0)
63-
if err != nil {
64-
return err
65-
}
66-
67-
if err := client.Send(name, input, true); err != nil {
68-
return err
69-
}
70-
7161
var pattern string
7262
var settleMs int
7363

7464
if hasWait {
7565
pattern = execWaitFlag
76-
settleMs = 0
7766
} else {
78-
pattern = ""
7967
settleMs = execSettleFlag
8068
}
8169

82-
output, pos, err := wait.ForOutput(
83-
func() (string, int, error) { return client.Read(name, "all", 0, 0) },
84-
wait.Config{
85-
Pattern: pattern,
86-
SettleMs: settleMs,
87-
TimeoutSec: execTimeoutFlag,
88-
StartPosition: startPos,
89-
SizeFunc: func() (int, error) { return client.Size(name) },
90-
},
91-
)
70+
result, err := client.Exec(name, daemon.ExecOptions{
71+
Input: input,
72+
SettleMs: settleMs,
73+
WaitPattern: pattern,
74+
TimeoutSec: execTimeoutFlag,
75+
})
9276
if err != nil {
9377
return err
9478
}
9579

80+
output := result.Output
9681
if execStripAnsiFlag {
9782
output = ansi.Strip(output)
9883
}
9984

10085
if execJsonFlag {
10186
out := map[string]interface{}{
102-
"input": input,
87+
"input": result.Input,
10388
"output": output,
104-
"position": pos,
89+
"position": result.Position,
10590
}
10691
data, err := json.MarshalIndent(out, "", " ")
10792
if err != nil {

internal/daemon/client.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"os"
88
"os/exec"
99
"time"
10+
11+
"github.com/schovi/shelli/internal/wait"
1012
)
1113

1214
type Client struct{}
@@ -372,6 +374,56 @@ func (c *Client) Size(name string) (int, error) {
372374
return int(sizeFloat), nil
373375
}
374376

377+
type ExecOptions struct {
378+
Input string
379+
SettleMs int
380+
WaitPattern string
381+
TimeoutSec int
382+
}
383+
384+
type ExecResult struct {
385+
Input string
386+
Output string
387+
Position int
388+
}
389+
390+
func (c *Client) Exec(name string, opts ExecOptions) (*ExecResult, error) {
391+
_, startPos, err := c.Read(name, "all", 0, 0)
392+
if err != nil {
393+
return nil, err
394+
}
395+
396+
if err := c.Send(name, opts.Input, true); err != nil {
397+
return nil, err
398+
}
399+
400+
settleMs := opts.SettleMs
401+
if opts.WaitPattern == "" && settleMs == 0 {
402+
settleMs = 500
403+
}
404+
405+
timeoutSec := opts.TimeoutSec
406+
if timeoutSec == 0 {
407+
timeoutSec = 10
408+
}
409+
410+
output, pos, err := wait.ForOutput(
411+
func() (string, int, error) { return c.Read(name, "all", 0, 0) },
412+
wait.Config{
413+
Pattern: opts.WaitPattern,
414+
SettleMs: settleMs,
415+
TimeoutSec: timeoutSec,
416+
StartPosition: startPos,
417+
SizeFunc: func() (int, error) { return c.Size(name) },
418+
},
419+
)
420+
if err != nil {
421+
return nil, err
422+
}
423+
424+
return &ExecResult{Input: opts.Input, Output: output, Position: pos}, nil
425+
}
426+
375427
func (c *Client) send(req Request) (*Response, error) {
376428
conn, err := net.Dial("unix", SocketPath())
377429
if err != nil {
@@ -381,6 +433,8 @@ func (c *Client) send(req Request) (*Response, error) {
381433

382434
conn.SetDeadline(time.Now().Add(ClientDeadline))
383435

436+
req.Version = ProtocolVersion
437+
384438
if err := json.NewEncoder(conn).Encode(req); err != nil {
385439
return nil, err
386440
}

internal/daemon/constants.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package daemon
33
import "time"
44

55
const (
6+
ProtocolVersion = 1
7+
68
ReadBufferSize = 4096
79
KillGracePeriod = 100 * time.Millisecond
810
ClientDeadline = 30 * time.Second

0 commit comments

Comments
 (0)