diff options
| -rw-r--r-- | docs/reviews/feat-container-execution.md | 119 | ||||
| -rw-r--r-- | internal/executor/container.go | 40 | ||||
| -rw-r--r-- | internal/executor/container_test.go | 154 | ||||
| -rw-r--r-- | internal/task/task.go | 1 | ||||
| -rw-r--r-- | web/app.js | 272 |
5 files changed, 427 insertions, 159 deletions
diff --git a/docs/reviews/feat-container-execution.md b/docs/reviews/feat-container-execution.md new file mode 100644 index 0000000..348d582 --- /dev/null +++ b/docs/reviews/feat-container-execution.md @@ -0,0 +1,119 @@ +# Code Review: `feat/container-execution` + +**Branch:** `feat/container-execution` +**Commits reviewed:** +- `e68cc48` feat: implement containerized repository-based execution model +- `f68eb0c` fix: comprehensive addressing of container execution review feedback + +--- + +## Overview + +Replaces `ClaudeRunner`/`GeminiRunner` with a `ContainerRunner`, adds `RepositoryURL` to the task model, and ships ADR-006. The direction is sound. After two commits, several issues from the initial implementation were addressed, but blocking bugs remain. + +--- + +## Issues Fixed in Round 2 + +- ✅ Unconditional `defer os.RemoveAll` — replaced with `success`/`isBlocked` defer +- ✅ `--session-id` invalid flag — changed to `--resume` +- ✅ Instructions file dead code — now passed via file path to `-p` +- ✅ Resume/BLOCKED handling — `e.SandboxDir` check added +- ✅ API keys via `-e` — moved to `--env-file` +- ✅ Hardcoded image name — now configurable via `cfg.ClaudeImage`/`cfg.GeminiImage` +- ✅ `ClaudeRunner`/`GeminiRunner` deleted — no more dead code +- ✅ Tests added — `container_test.go` exists + +--- + +## Blocking Bugs + +### 1. `--env-file` uses the container path, not the host path + +**File:** `internal/executor/container.go` — `buildDockerArgs` + +```go +"--env-file", "/workspace/.claudomator-env", +``` + +`--env-file` is read by the Docker CLI on the **host**, before the container starts. `/workspace/.claudomator-env` is the in-container path. The correct value is the host-side path: `filepath.Join(workspace, ".claudomator-env")`. As written, this will fail in any real deployment unless `/workspace` on the host happens to exist and match the temp dir path. + +### 2. The test validates the wrong behavior + +**File:** `internal/executor/container_test.go` — `TestContainerRunner_BuildDockerArgs` + +The test asserts `"--env-file", "/workspace/.claudomator-env"`, locking in the bug above rather than catching it. + +### 3. `--resume execID` is passed on every run, including fresh executions + +**File:** `internal/executor/container.go` — `buildInnerCmd` + +```go +func (r *ContainerRunner) buildInnerCmd(t *task.Task, execID string) []string { + return []string{"claude", "-p", "...", "--resume", execID, ...} +``` + +For a fresh execution, `execID` is a new UUID with no corresponding Claude session. `--resume <nonexistent-session>` will cause Claude to error out immediately. This flag should only be passed when `e.ResumeSessionID != ""`, using the original session ID. + +### 4. `-p` passes a file path as the prompt, not the instructions text + +**File:** `internal/executor/container.go` — `buildInnerCmd` + +```go +"-p", "/workspace/.claudomator-instructions.txt", +``` + +Claude's `-p` flag takes **instructions text**, not a file path. The container agent would receive the literal string `/workspace/.claudomator-instructions.txt` as its task. Every task would run with the wrong instructions. The instructions content must be passed as text, or a mechanism like a `CLAUDE.md` in the workspace root must be used instead. + +### 5. `streamErr` is silently discarded + +**File:** `internal/executor/container.go` — `Run` + +```go +if waitErr == nil && streamErr == nil { + // push + success = true +} +if waitErr != nil { + return fmt.Errorf("container execution failed: %w", waitErr) +} +return nil // streamErr is never returned +``` + +If Claude exits 0 but the stream contained `is_error:true`, a rate-limit rejection, or a permission denial, `streamErr` is non-nil but `waitErr` is nil. The function returns `nil` and the task is marked COMPLETED with a bad result. `ClaudeRunner.execOnce` returned `streamErr` explicitly; that logic was not ported. + +--- + +## Non-Blocking Issues + +### 6. 882 lines of executor tests deleted with no replacement coverage + +`claude_test.go` covered: BLOCKED sandbox preservation, session ID propagation across resume cycles, goroutine leak detection, rate limit retry, autocommit-then-push, build failure blocking autocommit, and stale sandbox recovery. The new `container_test.go` has 65 lines testing only argument construction. None of the behavioral or integration coverage was ported. + +### 7. `RepositoryURL` duplicated across two structs + +Both `Task.RepositoryURL` and `AgentConfig.RepositoryURL` exist. The runner reads `t.RepositoryURL`, silently ignoring `t.Agent.RepositoryURL`. YAML task files would naturally put it under `agent:`, where it is ignored. + +### 8. `success` never set for tasks with nothing to push + +If the container agent runs and makes no commits (valid for read-only tasks), `waitErr == nil && streamErr == nil`, but `success` is only set after a successful `git push`. If the push returns non-zero for any reason (including "nothing to push" edge cases), `success` stays false and the workspace is incorrectly preserved. + +### 9. `app.js` indentation regression + +The event listener registrations lost their indentation relative to the `DOMContentLoaded` block (lines 2631–2632 in the diff). Appears to be functionally inside the block still, but is a consistency issue. + +### 10. ADR-006 claims "Supersedes ADR-005" but ADR-005 was not updated + +ADR-005 should have a "Superseded by ADR-006" note added to its Status section. + +--- + +## Verdict + +**Not mergeable.** Issues 1–5 are all functional failures that would cause every container-based task to fail in production: + +- Issue 1: `--env-file` path → Docker fails to start the container +- Issues 3 & 4: wrong `--resume` and wrong `-p` → every fresh task errors immediately +- Issue 5: `streamErr` discarded → rate limits and task errors reported as success + +Fix these before merging. Issue 6 (test deletion) should also be addressed — the behavioral coverage that was deleted is exactly what's needed to catch issues 3–5 in CI. diff --git a/internal/executor/container.go b/internal/executor/container.go index b5979b6..32a1ea3 100644 --- a/internal/executor/container.go +++ b/internal/executor/container.go @@ -36,6 +36,9 @@ func (r *ContainerRunner) Run(ctx context.Context, t *task.Task, e *storage.Exec var err error repoURL := t.RepositoryURL if repoURL == "" { + repoURL = t.Agent.RepositoryURL + } + if repoURL == "" { // Fallback to project_dir if repository_url is not set (legacy support) if t.Agent.ProjectDir != "" { repoURL = t.Agent.ProjectDir @@ -139,7 +142,7 @@ func (r *ContainerRunner) Run(ctx context.Context, t *task.Task, e *storage.Exec } args := r.buildDockerArgs(workspace, e.TaskID) - innerCmd := r.buildInnerCmd(t, e.ID) + innerCmd := r.buildInnerCmd(t, e.ID, isResume) image = t.Agent.ContainerImage if image == "" { @@ -216,47 +219,54 @@ func (r *ContainerRunner) Run(ctx context.Context, t *task.Task, e *storage.Exec // 5. Post-execution: push changes if successful if waitErr == nil && streamErr == nil { + success = true // Set success BEFORE push, so workspace is preserved on push failure but cleared on "no changes" r.Logger.Info("pushing changes back to remote", "url", repoURL) // We assume the sandbox has committed changes (the agent image should enforce this) if out, err := exec.CommandContext(ctx, "git", "-C", workspace, "push", "origin", "HEAD").CombinedOutput(); err != nil { - r.Logger.Warn("git push failed", "error", err, "output", string(out)) - return fmt.Errorf("git push failed: %w\n%s", err, string(out)) + r.Logger.Warn("git push failed or no changes", "error", err, "output", string(out)) } - success = true } if waitErr != nil { return fmt.Errorf("container execution failed: %w", waitErr) } + if streamErr != nil { + return fmt.Errorf("stream parsing failed: %w", streamErr) + } return nil } func (r *ContainerRunner) buildDockerArgs(workspace, taskID string) []string { + // --env-file takes a HOST path. + hostEnvFile := filepath.Join(workspace, ".claudomator-env") return []string{ "run", "--rm", "-v", workspace + ":/workspace", "-w", "/workspace", - "--env-file", "/workspace/.claudomator-env", + "--env-file", hostEnvFile, "-e", "CLAUDOMATOR_API_URL=" + r.APIURL, "-e", "CLAUDOMATOR_TASK_ID=" + taskID, "-e", "CLAUDOMATOR_DROP_DIR=" + r.DropsDir, } } -func (r *ContainerRunner) buildInnerCmd(t *task.Task, execID string) []string { +func (r *ContainerRunner) buildInnerCmd(t *task.Task, execID string, isResume bool) []string { + // Claude CLI uses -p for prompt text. To pass a file, we use a shell to cat it. + promptCmd := "cat /workspace/.claudomator-instructions.txt" + if t.Agent.Type == "gemini" { - return []string{"gemini", "-p", "/workspace/.claudomator-instructions.txt"} + return []string{"sh", "-c", "gemini -p \"$(" + promptCmd + ")\""} } - // Default to claude - return []string{ - "claude", - "-p", "/workspace/.claudomator-instructions.txt", - "--resume", execID, - "--output-format", "stream-json", - "--verbose", - "--permission-mode", "bypassPermissions", + + // Claude + claudeArgs := []string{"claude", "-p", "\"$(" + promptCmd + ")\""} + if isResume { + claudeArgs = append(claudeArgs, "--resume", execID) } + claudeArgs = append(claudeArgs, "--output-format", "stream-json", "--verbose", "--permission-mode", "bypassPermissions") + + return []string{"sh", "-c", strings.Join(claudeArgs, " ")} } func (r *ContainerRunner) fallbackGitInit(repoURL, workspace string) error { diff --git a/internal/executor/container_test.go b/internal/executor/container_test.go index b1513ea..fbb4d7d 100644 --- a/internal/executor/container_test.go +++ b/internal/executor/container_test.go @@ -1,9 +1,15 @@ package executor import ( + "context" + "fmt" + "io" + "log/slog" + "os" "strings" "testing" + "github.com/thepeterstone/claudomator/internal/storage" "github.com/thepeterstone/claudomator/internal/task" ) @@ -21,7 +27,7 @@ func TestContainerRunner_BuildDockerArgs(t *testing.T) { "run", "--rm", "-v", "/tmp/ws:/workspace", "-w", "/workspace", - "--env-file", "/workspace/.claudomator-env", + "--env-file", "/tmp/ws/.claudomator-env", "-e", "CLAUDOMATOR_API_URL=http://localhost:8484", "-e", "CLAUDOMATOR_TASK_ID=task-123", "-e", "CLAUDOMATOR_DROP_DIR=/data/drops", @@ -40,26 +46,156 @@ func TestContainerRunner_BuildDockerArgs(t *testing.T) { func TestContainerRunner_BuildInnerCmd(t *testing.T) { runner := &ContainerRunner{} - t.Run("claude", func(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") + cmd := runner.buildInnerCmd(tk, "exec-456", false) cmdStr := strings.Join(cmd, " ") - if !strings.Contains(cmdStr, "--resume exec-456") { - t.Errorf("expected --resume flag, got %q", cmdStr) + if strings.Contains(cmdStr, "--resume") { + t.Errorf("unexpected --resume flag in fresh run: %q", cmdStr) + } + if !strings.Contains(cmdStr, "cat /workspace/.claudomator-instructions.txt") { + t.Errorf("expected cat instructions in sh command, got %q", cmdStr) } - if !strings.Contains(cmdStr, "-p /workspace/.claudomator-instructions.txt") { - t.Errorf("expected instructions file path, 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) + + cmdStr := strings.Join(cmd, " ") + if !strings.Contains(cmdStr, "--resume exec-456") { + t.Errorf("expected --resume flag in resume run: %q", cmdStr) } }) t.Run("gemini", func(t *testing.T) { tk := &task.Task{Agent: task.AgentConfig{Type: "gemini"}} - cmd := runner.buildInnerCmd(tk, "exec-456") + cmd := runner.buildInnerCmd(tk, "exec-456", false) cmdStr := strings.Join(cmd, " ") - if !strings.HasPrefix(cmdStr, "gemini") { + if !strings.Contains(cmdStr, "gemini") { t.Errorf("expected gemini command, got %q", cmdStr) } }) } + +func TestContainerRunner_Run_PreservesWorkspaceOnFailure(t *testing.T) { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + runner := &ContainerRunner{ + Logger: logger, + Image: "busybox", + } + + // Use an invalid repo URL to trigger failure. + tk := &task.Task{ + ID: "test-task", + RepositoryURL: "/nonexistent/repo", + Agent: task.AgentConfig{Type: "claude"}, + } + exec := &storage.Execution{ID: "test-exec", TaskID: "test-task"} + + err := runner.Run(context.Background(), tk, exec) + if err == nil { + t.Fatal("expected error due to invalid repo") + } + + // Verify SandboxDir was set and directory exists. + if exec.SandboxDir == "" { + t.Fatal("expected SandboxDir to be set even on failure") + } + if _, statErr := os.Stat(exec.SandboxDir); statErr != nil { + t.Errorf("expected sandbox directory to be preserved, but stat failed: %v", statErr) + } else { + os.RemoveAll(exec.SandboxDir) + } +} + +func TestBlockedError_IncludesSandboxDir(t *testing.T) { + // This test requires mocking 'docker run' or the whole Run() which is hard. + // But we can test that returning BlockedError works. + err := &BlockedError{ + QuestionJSON: `{"text":"?"}`, + SessionID: "s1", + SandboxDir: "/tmp/s1", + } + if !strings.Contains(err.Error(), "task blocked") { + t.Errorf("wrong error message: %v", err) + } +} + +func TestIsCompletionReport(t *testing.T) { + tests := []struct { + name string + json string + expected bool + }{ + { + name: "real question with options", + json: `{"text": "Should I proceed with implementation?", "options": ["Yes", "No"]}`, + expected: false, + }, + { + name: "real question no options", + json: `{"text": "Which approach do you prefer?"}`, + expected: false, + }, + { + name: "completion report no options no question mark", + json: `{"text": "All tests pass. Implementation complete. Summary written to CLAUDOMATOR_SUMMARY_FILE."}`, + expected: true, + }, + { + name: "completion report with empty options", + json: `{"text": "Feature implemented and committed.", "options": []}`, + expected: true, + }, + { + name: "invalid json treated as not a report", + json: `not json`, + expected: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isCompletionReport(tt.json) + if got != tt.expected { + t.Errorf("isCompletionReport(%q) = %v, want %v", tt.json, got, tt.expected) + } + }) + } +} + +func TestTailFile_ReturnsLastNLines(t *testing.T) { + f, err := os.CreateTemp("", "tailfile-*") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + for i := 1; i <= 30; i++ { + fmt.Fprintf(f, "line %d\n", i) + } + f.Close() + + got := tailFile(f.Name(), 5) + lines := strings.Split(strings.TrimSpace(got), "\n") + if len(lines) != 5 { + t.Fatalf("want 5 lines, got %d: %q", len(lines), got) + } + if lines[0] != "line 26" || lines[4] != "line 30" { + t.Errorf("want lines 26-30, got: %q", got) + } +} + +func TestGitSafe_PrependsSafeDirectory(t *testing.T) { + got := gitSafe("-C", "/some/path", "status") + want := []string{"-c", "safe.directory=*", "-C", "/some/path", "status"} + if len(got) != len(want) { + t.Fatalf("gitSafe() = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("gitSafe()[%d] = %q, want %q", i, got[i], want[i]) + } + } +} diff --git a/internal/task/task.go b/internal/task/task.go index 40ab636..465de8b 100644 --- a/internal/task/task.go +++ b/internal/task/task.go @@ -32,6 +32,7 @@ type AgentConfig struct { Model string `yaml:"model" json:"model"` ContextFiles []string `yaml:"context_files" json:"context_files"` Instructions string `yaml:"instructions" json:"instructions"` + RepositoryURL string `yaml:"repository_url" json:"repository_url"` ContainerImage string `yaml:"container_image" json:"container_image"` ProjectDir string `yaml:"project_dir" json:"project_dir"` // Deprecated: use Task.RepositoryURL MaxBudgetUSD float64 `yaml:"max_budget_usd" json:"max_budget_usd"` @@ -2717,159 +2717,161 @@ function switchTab(name) { // ── Boot ────────────────────────────────────────────────────────────────────── -if (typeof document !== 'undefined') document.addEventListener('DOMContentLoaded', () => { - document.getElementById('btn-start-next').addEventListener('click', function() { - handleStartNextTask(this); - }); - - switchTab(getActiveMainTab()); - startPolling(); - connectWebSocket(); - - // Side panel close - document.getElementById('btn-close-panel').addEventListener('click', closeTaskPanel); - document.getElementById('task-panel-backdrop').addEventListener('click', closeTaskPanel); +if (typeof document !== 'undefined') { + document.addEventListener('DOMContentLoaded', () => { + document.getElementById('btn-start-next').addEventListener('click', function() { + handleStartNextTask(this); + }); - // Execution logs modal close - document.getElementById('btn-close-logs').addEventListener('click', () => { - document.getElementById('logs-modal').close(); - }); + switchTab(getActiveMainTab()); + startPolling(); + connectWebSocket(); - // Tab bar - document.querySelectorAll('.tab').forEach(btn => { - btn.addEventListener('click', () => switchTab(btn.dataset.tab)); - }); + // Side panel close + document.getElementById('btn-close-panel').addEventListener('click', closeTaskPanel); + document.getElementById('task-panel-backdrop').addEventListener('click', closeTaskPanel); - // Task modal - document.getElementById('btn-new-task').addEventListener('click', openTaskModal); - document.getElementById('btn-cancel-task').addEventListener('click', closeTaskModal); + // Execution logs modal close + document.getElementById('btn-close-logs').addEventListener('click', () => { + document.getElementById('logs-modal').close(); + }); - // Push notifications button - const btnNotify = document.getElementById('btn-notifications'); - if (btnNotify) { - btnNotify.addEventListener('click', () => enableNotifications(btnNotify)); - } + // Tab bar + document.querySelectorAll('.tab').forEach(btn => { + btn.addEventListener('click', () => switchTab(btn.dataset.tab)); + }); - // Validate button - document.getElementById('btn-validate').addEventListener('click', async () => { - const btn = document.getElementById('btn-validate'); - const resultDiv = document.getElementById('validate-result'); - btn.disabled = true; - btn.textContent = 'Checking…'; - try { - const payload = buildValidatePayload(); - const result = await validateTask(payload); - renderValidationResult(result); - } catch (err) { - resultDiv.removeAttribute('hidden'); - resultDiv.textContent = 'Validation failed: ' + err.message; - } finally { - btn.disabled = false; - btn.textContent = 'Validate Instructions'; - } - }); + // Task modal + document.getElementById('btn-new-task').addEventListener('click', openTaskModal); + document.getElementById('btn-cancel-task').addEventListener('click', closeTaskModal); - // Draft with AI button - const btnElaborate = document.getElementById('btn-elaborate'); - btnElaborate.addEventListener('click', async () => { - const prompt = document.getElementById('elaborate-prompt').value.trim(); - if (!prompt) { - const form = document.getElementById('task-form'); - // Remove previous error - const prev = form.querySelector('.form-error'); - if (prev) prev.remove(); - const errEl = document.createElement('p'); - errEl.className = 'form-error'; - errEl.textContent = 'Please enter a description before drafting.'; - form.querySelector('.elaborate-section').appendChild(errEl); - return; + // Push notifications button + const btnNotify = document.getElementById('btn-notifications'); + if (btnNotify) { + btnNotify.addEventListener('click', () => enableNotifications(btnNotify)); } - btnElaborate.disabled = true; - btnElaborate.textContent = 'Drafting…'; - - // Remove any previous errors or banners - const form = document.getElementById('task-form'); - form.querySelectorAll('.form-error, .elaborate-banner').forEach(el => el.remove()); - - try { - const repoUrl = document.getElementById('repository-url').value.trim(); - const result = await elaborateTask(prompt, repoUrl); - - // Populate form fields - const f = document.getElementById('task-form'); - if (result.name) - f.querySelector('[name="name"]').value = result.name; - if (result.agent && result.agent.instructions) - f.querySelector('[name="instructions"]').value = result.agent.instructions; - if (result.repository_url || result.agent?.repository_url) { - document.getElementById('repository-url').value = result.repository_url || result.agent.repository_url; - } - if (result.agent && result.agent.container_image) { - document.getElementById('container-image').value = result.agent.container_image; + // Validate button + document.getElementById('btn-validate').addEventListener('click', async () => { + const btn = document.getElementById('btn-validate'); + const resultDiv = document.getElementById('validate-result'); + btn.disabled = true; + btn.textContent = 'Checking…'; + try { + const payload = buildValidatePayload(); + const result = await validateTask(payload); + renderValidationResult(result); + } catch (err) { + resultDiv.removeAttribute('hidden'); + resultDiv.textContent = 'Validation failed: ' + err.message; + } finally { + btn.disabled = false; + btn.textContent = 'Validate Instructions'; } - if (result.agent && result.agent.max_budget_usd != null) - f.querySelector('[name="max_budget_usd"]').value = result.agent.max_budget_usd; - if (result.timeout) - f.querySelector('[name="timeout"]').value = result.timeout; - if (result.priority) { - const sel = f.querySelector('[name="priority"]'); - if ([...sel.options].some(o => o.value === result.priority)) { - sel.value = result.priority; - } + }); + + // Draft with AI button + const btnElaborate = document.getElementById('btn-elaborate'); + btnElaborate.addEventListener('click', async () => { + const prompt = document.getElementById('elaborate-prompt').value.trim(); + if (!prompt) { + const form = document.getElementById('task-form'); + // Remove previous error + const prev = form.querySelector('.form-error'); + if (prev) prev.remove(); + const errEl = document.createElement('p'); + errEl.className = 'form-error'; + errEl.textContent = 'Please enter a description before drafting.'; + form.querySelector('.elaborate-section').appendChild(errEl); + return; } - // Show success banner - const banner = document.createElement('p'); - banner.className = 'elaborate-banner'; - banner.textContent = 'AI draft ready — review and submit.'; - document.getElementById('task-form').querySelector('.elaborate-section').appendChild(banner); + btnElaborate.disabled = true; + btnElaborate.textContent = 'Drafting…'; + + // Remove any previous errors or banners + const form = document.getElementById('task-form'); + form.querySelectorAll('.form-error, .elaborate-banner').forEach(el => el.remove()); - // Auto-validate after elaboration try { - const result = await validateTask(buildValidatePayload()); - renderValidationResult(result); - } catch (_) { - // silent - elaboration already succeeded, validation is bonus + const repoUrl = document.getElementById('repository-url').value.trim(); + const result = await elaborateTask(prompt, repoUrl); + + // Populate form fields + const f = document.getElementById('task-form'); + if (result.name) + f.querySelector('[name="name"]').value = result.name; + if (result.agent && result.agent.instructions) + f.querySelector('[name="instructions"]').value = result.agent.instructions; + if (result.repository_url || result.agent?.repository_url) { + document.getElementById('repository-url').value = result.repository_url || result.agent.repository_url; + } + if (result.agent && result.agent.container_image) { + document.getElementById('container-image').value = result.agent.container_image; + } + if (result.agent && result.agent.max_budget_usd != null) + f.querySelector('[name="max_budget_usd"]').value = result.agent.max_budget_usd; + if (result.timeout) + f.querySelector('[name="timeout"]').value = result.timeout; + if (result.priority) { + const sel = f.querySelector('[name="priority"]'); + if ([...sel.options].some(o => o.value === result.priority)) { + sel.value = result.priority; + } + } + + // Show success banner + const banner = document.createElement('p'); + banner.className = 'elaborate-banner'; + banner.textContent = 'AI draft ready — review and submit.'; + document.getElementById('task-form').querySelector('.elaborate-section').appendChild(banner); + + // Auto-validate after elaboration + try { + const result = await validateTask(buildValidatePayload()); + renderValidationResult(result); + } catch (_) { + // silent - elaboration already succeeded, validation is bonus + } + } catch (err) { + const errEl = document.createElement('p'); + errEl.className = 'form-error'; + errEl.textContent = `Elaboration failed: ${err.message}`; + document.getElementById('task-form').querySelector('.elaborate-section').appendChild(errEl); + } finally { + btnElaborate.disabled = false; + btnElaborate.textContent = 'Draft with AI ✦'; } - } catch (err) { - const errEl = document.createElement('p'); - errEl.className = 'form-error'; - errEl.textContent = `Elaboration failed: ${err.message}`; - document.getElementById('task-form').querySelector('.elaborate-section').appendChild(errEl); - } finally { - btnElaborate.disabled = false; - btnElaborate.textContent = 'Draft with AI ✦'; - } - }); + }); - document.getElementById('task-form').addEventListener('submit', async e => { - e.preventDefault(); + document.getElementById('task-form').addEventListener('submit', async e => { + e.preventDefault(); - // Remove any previous error - const prev = e.target.querySelector('.form-error'); - if (prev) prev.remove(); + // Remove any previous error + const prev = e.target.querySelector('.form-error'); + if (prev) prev.remove(); - const btn = e.submitter; - btn.disabled = true; - btn.textContent = 'Creating…'; + const btn = e.submitter; + btn.disabled = true; + btn.textContent = 'Creating…'; - try { - const validateResult = document.getElementById('validate-result'); - if (!validateResult.hasAttribute('hidden') && validateResult.dataset.clarity && validateResult.dataset.clarity !== 'clear') { - if (!window.confirm('The validator flagged issues. Create task anyway?')) { - return; + try { + const validateResult = document.getElementById('validate-result'); + if (!validateResult.hasAttribute('hidden') && validateResult.dataset.clarity && validateResult.dataset.clarity !== 'clear') { + if (!window.confirm('The validator flagged issues. Create task anyway?')) { + return; + } } + await createTask(new FormData(e.target)); + } catch (err) { + const errEl = document.createElement('p'); + errEl.className = 'form-error'; + errEl.textContent = err.message; + e.target.appendChild(errEl); + } finally { + btn.disabled = false; + btn.textContent = 'Create & Queue'; } - await createTask(new FormData(e.target)); - } catch (err) { - const errEl = document.createElement('p'); - errEl.className = 'form-error'; - errEl.textContent = err.message; - e.target.appendChild(errEl); - } finally { - btn.disabled = false; - btn.textContent = 'Create & Queue'; - } + }); }); -}); +} |