From e1be377c851f1e7ce594fa3de6c429354bcedcce Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Wed, 18 Mar 2026 07:24:31 +0000 Subject: fix: address round 3 review feedback for container execution - Fix push failure swallowing and ensure workspace preservation on push error - Fix wrong session ID in --resume flag and BlockedError - Implement safer shell quoting for instructions in buildInnerCmd - Capture and propagate actual Claude session ID from stream init message - Clean up redundant image resolution and stale TODOs - Mark ADR-005 as Superseded - Consolidate RepositoryURL to Task level (removed from AgentConfig) - Add unit test for session ID extraction in parseStream --- internal/executor/container_test.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'internal/executor/container_test.go') diff --git a/internal/executor/container_test.go b/internal/executor/container_test.go index fbb4d7d..0e36def 100644 --- a/internal/executor/container_test.go +++ b/internal/executor/container_test.go @@ -33,6 +33,7 @@ func TestContainerRunner_BuildDockerArgs(t *testing.T) { "-e", "CLAUDOMATOR_DROP_DIR=/data/drops", } + if len(args) != len(expected) { t.Fatalf("expected %d args, got %d", len(expected), len(args)) } @@ -48,34 +49,37 @@ func TestContainerRunner_BuildInnerCmd(t *testing.T) { t.Run("claude-fresh", func(t *testing.T) { tk := &task.Task{Agent: task.AgentConfig{Type: "claude"}} - cmd := runner.buildInnerCmd(tk, "exec-456", false) + exec := &storage.Execution{} + cmd := runner.buildInnerCmd(tk, exec, false) cmdStr := strings.Join(cmd, " ") if strings.Contains(cmdStr, "--resume") { t.Errorf("unexpected --resume flag in fresh run: %q", cmdStr) } - if !strings.Contains(cmdStr, "cat /workspace/.claudomator-instructions.txt") { + if !strings.Contains(cmdStr, "INST=$(cat /workspace/.claudomator-instructions.txt); claude -p \"$INST\"") { t.Errorf("expected cat instructions in sh command, got %q", cmdStr) } }) t.Run("claude-resume", func(t *testing.T) { tk := &task.Task{Agent: task.AgentConfig{Type: "claude"}} - cmd := runner.buildInnerCmd(tk, "exec-456", true) + exec := &storage.Execution{ResumeSessionID: "orig-session-123"} + cmd := runner.buildInnerCmd(tk, exec, true) cmdStr := strings.Join(cmd, " ") - if !strings.Contains(cmdStr, "--resume exec-456") { - t.Errorf("expected --resume flag in resume run: %q", cmdStr) + if !strings.Contains(cmdStr, "--resume orig-session-123") { + t.Errorf("expected --resume flag with correct session ID, got %q", cmdStr) } }) t.Run("gemini", func(t *testing.T) { tk := &task.Task{Agent: task.AgentConfig{Type: "gemini"}} - cmd := runner.buildInnerCmd(tk, "exec-456", false) + exec := &storage.Execution{} + cmd := runner.buildInnerCmd(tk, exec, false) cmdStr := strings.Join(cmd, " ") - if !strings.Contains(cmdStr, "gemini") { - t.Errorf("expected gemini command, got %q", cmdStr) + if !strings.Contains(cmdStr, "gemini -p \"$INST\"") { + t.Errorf("expected gemini command with safer quoting, got %q", cmdStr) } }) } -- cgit v1.2.3