diff options
| -rw-r--r-- | .agent/worklog.md | 129 | ||||
| -rw-r--r-- | internal/executor/container.go | 4 | ||||
| -rw-r--r-- | internal/executor/container_test.go | 92 | ||||
| -rw-r--r-- | internal/executor/executor.go | 12 | ||||
| -rw-r--r-- | internal/executor/executor_test.go | 8 | ||||
| -rw-r--r-- | internal/task/task.go | 1 |
6 files changed, 187 insertions, 59 deletions
diff --git a/.agent/worklog.md b/.agent/worklog.md index 6fb8033..d747b1c 100644 --- a/.agent/worklog.md +++ b/.agent/worklog.md @@ -1,72 +1,91 @@ -# SESSION_STATE.md +# Worklog ## Current Task Goal -ADR-007 implementation: Epic→Story→Task→Subtask hierarchy, project registry, Doot integration +Move Claudomator UI auth into Doot: replace Apache proxy rules with a Doot-side +reverse proxy, gating `/claudomator/*` behind Doot's session auth. -## Status: IN_PROGRESS +## Status: PLAN — awaiting user confirmation --- -## Completed Items - -| Step | Description | Test / Verification | -|------|-------------|---------------------| -| Phase 1 | Doot dead code removal: Bug struct, BugToAtom, bug store methods, bug handlers, bug routes, bugs.html template, TypeNote, AddMealToPlanner stub | `go test ./...` in /workspace/doot — all pass (2 pre-existing failures unrelated) | -| Phase 2 | Claudomator project registry: `task.Project` type, storage CRUD + UpsertProject, seed.go, API endpoints (GET/POST /api/projects, GET/PUT /api/projects/{id}), legacy AgentConfig.ProjectDir/RepositoryURL/SkipPlanning fields removed, container.go fallback removed, fallbackGitInit removed, processResult changestats extraction removed (pool-side only) | `TestCreateProject`, `TestListProjects`, `TestUpdateProject`, `TestProjects_CRUD` — all pass | +## Plan: Claudomator UI behind Doot auth + +### Architecture +``` +Browser → Apache (SSL) → Doot :38080 → [session auth] → Claudomator :8484 +``` +Apache currently proxies `/claudomator/*` directly to :8484 with no auth. +Goal: move the proxy into Doot so session middleware gates it. +Two processes, two systemd units — unchanged. +Claudomator base-path already hardcoded to `/claudomator` in web/index.html. + +### Step 1 — Doot: add `ClaudomatorURL` config +- `internal/config/config.go` — add `ClaudomatorURL string` (env: `CLAUDOMATOR_URL`, default: `http://127.0.0.1:8484`) +- Tests: default + override + +### Step 2 — Doot: HTTP + WebSocket reverse proxy handler +- New file: `internal/handlers/claudomator_proxy.go` +- `httputil.ReverseProxy` for normal requests; WS connection hijacker for upgrades +- Director strips `/claudomator` prefix from both `URL.Path` AND `URL.RawPath` (handles encoded chars in task names/IDs) +- Do NOT set `ReadDeadline`/`WriteDeadline` on hijacked WS connections (kills long-lived task monitoring) +- Preserve `Service-Worker-Allowed` response header so SW scopes correctly under `/claudomator` +- Tests: HTTP forward, prefix strip, WS tunnel + +### Step 3 — Doot: restructure CSRF middleware, mount proxy +- `cmd/dashboard/main.go`: move CSRF out of global middleware into a route group +- `/claudomator` → redirect 301 to `/claudomator/` (trailing slash; prevents asset fetch breakage) +- `/claudomator/api/webhooks/github` → exempt from `RequireAuth` (GitHub POSTs have no session; endpoint does its own HMAC validation) +- `/claudomator/*` route: `RequireAuth` only (no CSRF — SPA doesn't send Doot's CSRF token) +- All other routes: wrapped in CSRF group (behavior unchanged) + +### Step 4 — Apache: remove Claudomator proxy rules +- Remove 4 lines from `/etc/apache2/sites-enabled/doot.terst.org-le-ssl.conf` +- `apache2ctl configtest && apache2ctl graceful` + +### Step 5 — Smoke tests +- Unauthenticated `/claudomator/` → 302 to `/login` +- `/claudomator` (no slash) → 301 to `/claudomator/` +- Authenticated: UI loads, task CRUD works, WS live updates, log streaming +- GitHub webhook POST to `/claudomator/api/webhooks/github` → not redirected to login + +### Risks +- CSRF restructure: verify all existing Doot routes still pass their tests after moving CSRF to a group +- SecurityHeaders CSP already allows `wss: ws:` — no change needed +- Claudomator :8484 remains accessible on localhost without auth (acceptable for now) +- Future: `/claudomator/api/*` technically CSRF-vulnerable from other origins; mitigate later by injecting `XSRF-TOKEN` cookie --- -## Next Steps (Claudomator tasks created) - -Phases 3–6 are queued as Claudomator tasks. See `ct task list` or the web UI. +## Previous Task: ADR-007 — Epic→Story→Task hierarchy (IN_PROGRESS) -| Task ID | Phase | Status | Depends On | -|---------|-------|--------|------------| -| f8829d6f-b8b6-4ff2-9c1a-e55dd3ab300e | Phase 3: Stories data model | PENDING | — | -| c8a0dc6c-0605-4acb-a789-1155ad8824cb | Phase 4: Story execution and deploy | PENDING | Phase 3 | -| faf5a371-8f1c-46a3-bb74-b0df1f062dee | Phase 5: Story elaboration | PENDING | Phase 3 | -| f39af70f-72c5-4ac1-9522-83c2e11b37c9 | Phase 6: Doot — Claudomator integration | PENDING | Phase 3 | - -Instruction files: `scripts/.claude/phase{3,4,5,6}-*-instructions.txt` - -### Phase 3: Stories data model (claudomator repo) -- `internal/task/story.go` — Story struct + ValidStoryTransition -- `internal/storage/db.go` — stories table + story_id on tasks, CRUD + ListTasksByStory -- `internal/api/stories.go` — story API endpoints -- Tests: ValidStoryTransition, CRUD, depends_on auto-wire +### Completed Items -### Phase 4: Story execution and deploy (claudomator repo, depends Phase 3) -- `internal/executor/executor.go` — checkStoryCompletion → SHIPPABLE -- `internal/executor/container.go` — checkout story branch after clone -- `internal/api/stories.go` — POST /api/stories/{id}/branch - -### Phase 5: Story elaboration (claudomator repo, depends Phase 3) -- `internal/api/elaborate.go` — POST /api/stories/elaborate + approve -- SeedProjects called at server startup - -### Phase 6: Doot — Claudomator integration (doot repo, depends Phase 3) -- `internal/api/claudomator.go` — ClaudomatorClient -- `internal/models/atom.go` — StoryToAtom, SourceClaudomator -- `internal/handlers/atoms.go` — BuildUnifiedAtomList extended -- `cmd/dashboard/main.go` — wire ClaudomatorURL config +| Step | Description | Test / Verification | +|------|-------------|---------------------| +| Phase 1 | Doot dead code removal: Bug struct, BugToAtom, bug store methods, bug handlers, bug routes, bugs.html template, TypeNote, AddMealToPlanner stub | `go test ./...` in /workspace/doot — all pass | +| Phase 2 | Claudomator project registry: `task.Project` type, storage CRUD + UpsertProject, seed.go, API endpoints, legacy fields removed | `TestCreateProject`, `TestListProjects`, `TestUpdateProject`, `TestProjects_CRUD` | +| Phase 3 | Stories data model: Story struct + ValidStoryTransition, stories table, CRUD, story API endpoints | committed 5081b0c | +| Phase 4 | Story execution and deploy: checkStoryCompletion → SHIPPABLE, story branch checkout, POST /api/stories/{id}/branch | committed 15a46b0 | +| Phase 5 | Story elaboration: POST /api/stories/elaborate + approve, SeedProjects at startup, GetProject on executor Store interface | committed bc62c35 | ---- +### Pending (Claudomator tasks queued) -## Key Files Changed (Phases 1-2) +| Task ID | Phase | Status | +|---------|-------|--------| +| f39af70f-72c5-4ac1-9522-83c2e11b37c9 | Phase 6: Doot — Claudomator integration | QUEUED | -### Claudomator -- `internal/task/project.go` — new Project struct -- `internal/task/task.go` — removed Agent.ProjectDir, Agent.RepositoryURL, Agent.SkipPlanning -- `internal/storage/db.go` — projects table migration + CRUD -- `internal/storage/seed.go` — SeedProjects upserts claudomator + nav on startup -- `internal/api/projects.go` — project CRUD handlers -- `internal/api/server.go` — project routes; processResult no longer extracts changestats -- `internal/api/deployment.go` + `task_view.go` — use tk.RepositoryURL (was tk.Agent.ProjectDir) -- `internal/executor/container.go` — fallback logic removed; requires t.RepositoryURL +### Key Files Changed (Phases 1–5) -### Doot +#### Claudomator +- `internal/task/project.go` — Project struct +- `internal/task/story.go` — Story struct + ValidStoryTransition +- `internal/task/task.go` — removed Agent.ProjectDir/RepositoryURL/SkipPlanning +- `internal/storage/db.go` — projects + stories tables, CRUD +- `internal/storage/seed.go` — SeedProjects +- `internal/api/projects.go`, `stories.go`, `elaborate.go` — handlers +- `internal/executor/executor.go` — GetProject on Store interface, RepositoryURL resolution +- `internal/cli/serve.go` — SeedProjects at startup + +#### Doot - Bug feature removed entirely (models, handlers, store, routes, template, migration) -- `migrations/018_drop_bugs.sql` — DROP TABLE IF EXISTS bugs -- `internal/api/interfaces.go` — AddMealToPlanner removed from PlanToEatAPI -- `internal/api/plantoeat.go` — AddMealToPlanner stub removed - `internal/models/atom.go` — SourceBug, TypeBug, TypeNote, BugToAtom removed diff --git a/internal/executor/container.go b/internal/executor/container.go index 5e1a026..8b244c6 100644 --- a/internal/executor/container.go +++ b/internal/executor/container.go @@ -109,6 +109,10 @@ func (r *ContainerRunner) Run(ctx context.Context, t *task.Task, e *storage.Exec } } } + // Fall back to task-level BranchName (e.g. set explicitly by executor or tests). + if storyBranch == "" { + storyBranch = t.BranchName + } // 2. Clone repo into workspace if not resuming. // git clone requires the target directory to not exist; remove the MkdirTemp-created dir first. diff --git a/internal/executor/container_test.go b/internal/executor/container_test.go index b6946ef..15c147f 100644 --- a/internal/executor/container_test.go +++ b/internal/executor/container_test.go @@ -514,3 +514,95 @@ func TestContainerRunner_AuthError_SyncsAndRetries(t *testing.T) { t.Error("expected sync-credentials to be called, but marker file not found") } } + +func TestContainerRunner_ClonesStoryBranch(t *testing.T) { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + var checkoutArgs []string + runner := &ContainerRunner{ + Logger: logger, + Image: "busybox", + Command: func(ctx context.Context, name string, arg ...string) *exec.Cmd { + if name == "git" && len(arg) > 0 && arg[0] == "clone" { + dir := arg[len(arg)-1] + os.MkdirAll(dir, 0755) + return exec.Command("true") + } + // Capture checkout calls: both "git checkout <branch>" and "git -C <dir> checkout <branch>" + for i, a := range arg { + if a == "checkout" { + checkoutArgs = append([]string{}, arg[i:]...) + break + } + } + if name == "docker" { + return exec.Command("sh", "-c", "exit 1") + } + return exec.Command("true") + }, + } + + tk := &task.Task{ + ID: "story-branch-test", + RepositoryURL: "https://example.com/repo.git", + BranchName: "story/my-feature", + Agent: task.AgentConfig{Type: "claude"}, + } + e := &storage.Execution{ID: "exec-1", TaskID: "story-branch-test"} + + runner.Run(context.Background(), tk, e) + os.RemoveAll(e.SandboxDir) + + // Assert git checkout was called with the story branch name. + if len(checkoutArgs) == 0 { + t.Fatal("expected git checkout to be called for story branch, but it was not") + } + found := false + for _, a := range checkoutArgs { + if a == "story/my-feature" { + found = true + break + } + } + if !found { + t.Errorf("expected git checkout story/my-feature, got args: %v", checkoutArgs) + } +} + +func TestContainerRunner_ClonesDefaultBranchWhenNoBranchName(t *testing.T) { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + var cloneArgs []string + runner := &ContainerRunner{ + Logger: logger, + Image: "busybox", + Command: func(ctx context.Context, name string, arg ...string) *exec.Cmd { + if name == "git" && len(arg) > 0 && arg[0] == "clone" { + cloneArgs = append([]string{}, arg...) + dir := arg[len(arg)-1] + os.MkdirAll(dir, 0755) + return exec.Command("true") + } + if name == "docker" { + return exec.Command("sh", "-c", "exit 1") + } + return exec.Command("true") + }, + } + + tk := &task.Task{ + ID: "no-branch-test", + RepositoryURL: "https://example.com/repo.git", + Agent: task.AgentConfig{Type: "claude"}, + } + e := &storage.Execution{ID: "exec-2", TaskID: "no-branch-test"} + + runner.Run(context.Background(), tk, e) + os.RemoveAll(e.SandboxDir) + + for _, a := range cloneArgs { + if a == "--branch" { + t.Errorf("expected no --branch flag for task without BranchName, got args: %v", cloneArgs) + } + } +} diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 22273d9..8dfb196 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -278,6 +278,12 @@ func (p *Pool) executeResume(ctx context.Context, t *task.Task, exec *storage.Ex t.RepositoryURL = proj.RemoteURL } } + // Populate BranchName from Story if missing (ADR-007). + if t.BranchName == "" && t.StoryID != "" { + if story, err := p.store.GetStory(t.StoryID); err == nil && story.BranchName != "" { + t.BranchName = story.BranchName + } + } err = runner.Run(ctx, t, exec) exec.EndTime = time.Now().UTC() @@ -744,6 +750,12 @@ func (p *Pool) execute(ctx context.Context, t *task.Task) { t.RepositoryURL = proj.RemoteURL } } + // Populate BranchName from Story if missing (ADR-007). + if t.BranchName == "" && t.StoryID != "" { + if story, err := p.store.GetStory(t.StoryID); err == nil && story.BranchName != "" { + t.BranchName = story.BranchName + } + } // Run the task. err = runner.Run(ctx, t, exec) diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index 1e92093..b93e819 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -1056,10 +1056,10 @@ func (m *minimalMockStore) UpdateExecutionChangestats(execID string, stats *task m.mu.Unlock() return nil } -func (m *minimalMockStore) RecordAgentEvent(_ storage.AgentEvent) error { return nil } -func (m *minimalMockStore) GetProject(_ string) (*task.Project, error) { return nil, nil } -func (m *minimalMockStore) GetStory(_ string) (*task.Story, error) { return nil, nil } -func (m *minimalMockStore) ListTasksByStory(_ string) ([]*task.Task, error) { return nil, nil } +func (m *minimalMockStore) RecordAgentEvent(_ storage.AgentEvent) error { return nil } +func (m *minimalMockStore) GetProject(_ string) (*task.Project, error) { return nil, nil } +func (m *minimalMockStore) GetStory(_ string) (*task.Story, error) { return nil, nil } +func (m *minimalMockStore) ListTasksByStory(_ string) ([]*task.Task, error) { return nil, nil } func (m *minimalMockStore) UpdateStoryStatus(_ string, _ task.StoryState) error { return nil } func (m *minimalMockStore) lastStateUpdate() (string, task.State, bool) { diff --git a/internal/task/task.go b/internal/task/task.go index ee79668..ba87360 100644 --- a/internal/task/task.go +++ b/internal/task/task.go @@ -82,6 +82,7 @@ type Task struct { Tags []string `yaml:"tags" json:"tags"` DependsOn []string `yaml:"depends_on" json:"depends_on"` StoryID string `yaml:"-" json:"story_id,omitempty"` + BranchName string `yaml:"-" json:"branch_name,omitempty"` State State `yaml:"-" json:"state"` RejectionComment string `yaml:"-" json:"rejection_comment,omitempty"` QuestionJSON string `yaml:"-" json:"question,omitempty"` |