Add ao spawn + ao project add (spawn a real worker end-to-end)#77
Conversation
Greptile SummaryThis PR wires up the full
Confidence Score: 5/5Safe to merge — all critical paths (repo resolution, branch defaulting, error mapping) are correct and covered by tests. The three core behavioural changes — DB-backed repo resolution, per-session branch defaulting, and the CLI commands — are all wired together correctly. Error wrapping with %w is consistent throughout so the ErrProjectNotResolvable to HTTP 400 mapping works end-to-end. The DTO-drift E2E test exercises the real HTTP router, catching any JSON field-name divergence at test time rather than at runtime. The zellij socket-dir fix is well-guarded (MkdirAll + warn-not-abort) and the socket-path budget test verifies the constraint quantitatively. No files require special attention. Important Files Changed
Reviews (6): Last reviewed commit: "fix(cli,daemon): address review findings..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR closes the loop for an end-to-end “spawn a real worker” flow by adding CLI commands to register a local repo as a project and then spawn a worker session against that registered project. On the daemon side, it replaces the prior always-failing static repo resolver with a DB-backed resolver and adds server-side branch defaulting to ensure git worktree creation succeeds when the user doesn’t specify a branch.
Changes:
- Default session worktree branch to
ao/<session-id>when none is provided during spawn. - Wire a DB-backed
RepoResolverthat resolves a project’s repo path from the projects table. - Add CLI commands
ao project addandao spawn, plus a sharedpostJSONclient helper and basic “missing required flags” tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/internal/session_manager/manager.go | Defaults spawn branch to a unique per-session branch before creating the worktree. |
| backend/internal/session_manager/manager_test.go | Adds unit tests for default-branch behavior and preserving explicit branches. |
| backend/internal/daemon/lifecycle_wiring.go | Switches gitworktree wiring to a DB-backed repo resolver and implements projectRepoResolver. |
| backend/internal/daemon/wiring_test.go | Adds a resolver test ensuring registered projects resolve to repo paths and unregistered projects error. |
| backend/internal/cli/root.go | Registers new spawn and project commands on the CLI root. |
| backend/internal/cli/client.go | Adds shared postJSON helper for POST requests to the daemon API. |
| backend/internal/cli/spawn.go | Implements ao spawn command (POST /api/v1/sessions). |
| backend/internal/cli/spawn_test.go | Adds fast-fail tests for missing required flags (no network calls). |
| backend/internal/cli/project.go | Implements ao project add (POST /api/v1/projects). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- `ao spawn` no longer prints a branch the sessions API doesn't return (session metadata is json:"-"), so the output is no longer misleading. - Unregistered/archived/no-path projects now surface a 400 PROJECT_NOT_RESOLVABLE with an actionable message instead of a generic 500: a new sessionmanager.ErrProjectNotResolvable sentinel the resolver wraps and writeSessionError maps. - postJSON reuses the injected Deps.HTTPClient (cloned, with a longer timeout) instead of a fresh client, keeping HTTP behaviour stubbable. - postJSON treats a stale run-file (dead PID) as "not running" via ProcessAlive, matching its docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make a registered project spawnable end-to-end from the CLI: - DB-backed RepoResolver: the daemon resolves a project's on-disk repo path from the projects table (replacing the empty StaticRepoResolver that failed every lookup), so a session's worktree is cut from the right repo. - session_manager defaults an empty spawn branch to ao/<session-id> — a fresh, unique branch per session, since gitworktree can't reuse a branch already checked out elsewhere (e.g. main). - `ao project add --path <repo>`: register a local git repo (POST /api/v1/projects). - `ao spawn --project <id> [--harness] [--branch] [--prompt] [--issue]`: spawn a worker session (POST /api/v1/sessions); harness defaults to the daemon's AO_AGENT. - Shared postJSON daemon client (reads the run-file for the port, surfaces the API error envelope). Stacked on #65, which lands the agent-adapter + session-manager wiring this depends on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- `ao spawn` no longer prints a branch the sessions API doesn't return (session metadata is json:"-"), so the output is no longer misleading. - Unregistered/archived/no-path projects now surface a 400 PROJECT_NOT_RESOLVABLE with an actionable message instead of a generic 500: a new sessionmanager.ErrProjectNotResolvable sentinel the resolver wraps and writeSessionError maps. - postJSON reuses the injected Deps.HTTPClient (cloned, with a longer timeout) instead of a fresh client, keeping HTTP behaviour stubbable. - postJSON treats a stale run-file (dead PID) as "not running" via ProcessAlive, matching its docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile review: harden TestProjectRepoResolver to verify the unregistered -project error wraps ErrProjectNotResolvable, so a future regression in the sentinel wrapping (which the HTTP 400 mapping relies on) is caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Root cause: the daemon built the zellij runtime with an empty SocketDir, so zellij fell back to its $TMPDIR-based default (long on macOS). That left almost none of the ~103-byte unix-socket-path budget for the session name, so a long session id (e.g. "aoagents-agent-orchestrator-1", derived from a long project id) was rejected by zellij with "session name must be less than 0 characters". runtime.Create failed, the spawn 500'd, and the worktree was rolled back (leaving an orphan ao/ branch). - New zellij.DefaultSocketDir(): a short, stable per-user socket dir (/tmp/ao-zellij-<uid>); the daemon uses it (and MkdirAll's it). - ao spawn's attach hint now prefixes ZELLIJ_SOCKET_DIR so it stays copy-pasteable against the daemon's socket dir. - Regression test guards that the socket dir leaves >= 48 bytes for the session name within the 103-byte limit. Verified: ao spawn against a long-id project now succeeds (session live, worktree created) where it previously 500'd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CLI keeps its own request structs (spawnRequest, addProjectRequest) separate from the daemon's canonical DTOs (controllers.SpawnSessionRequest, project.AddInput). Nothing verified the JSON field names agreed, so a renamed tag on either side would compile but break at runtime. Drive `ao spawn` and `ao project add` through the real httpd router and controllers (fakes only at the service layer) over a real loopback round trip via postJSON, asserting each field decodes into the right SpawnConfig/AddInput field. Runs in the normal test lane (no extra ports/processes). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1421da5 to
9a5e940
Compare
- spawn: print the sanitised zellij session name (zellij.SessionName) in the attach hint; a long/non-conforming session id is registered under a different name, so the raw id sent users to a missing session. - client: surface the daemon error envelope's requestId so a failed command can be correlated with daemon logs. - daemon: don't swallow the zellij socket-dir MkdirAll error — log it, since a failure otherwise surfaces later as an opaque socket-bind error on every spawn. - project: reject an embedded ".." in a project id up front; it passed the id pattern but yielded an invalid branch (ao/a..b-1) and an opaque 500 at spawn. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat(messenger): ao send + live zellij pane ping (live agent nudges)
Replace the daemon's noopMessenger stub with a composite AgentMessenger
that writes a durable inbox file (primary) and types a live pointer into
the running zellij pane (best-effort secondary), plus the `ao send` CLI
that drives the existing POST /api/v1/sessions/{id}/send route.
- composite: fans Send to inbox then panep, pinning one timestamp so both
derive the same filename; a secondary failure is logged at WARN and
swallowed (the file is on disk), a primary failure aborts the call.
- inbox: writes <workspace>/.ao/inbox/<rfc3339nano>_<hash>.md.
- panep: types "new message at .ao/inbox/<file>" + Enter via a new narrow
zellij WriteChars seam (RuntimePaneWriter), kept off ports.Runtime.
- wiring: newSessionMessenger composes inbox+panep over the shared store;
startSession takes the messenger instead of the noop stub.
Carries across @Aa-43's work from PR #74 (staging), adapted to main's
post-#65/#77 daemon wiring shape.
Closes #79
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(inbox): use O_EXCL so a filename collision errors instead of clobbering
os.WriteFile opens with O_CREATE|O_WRONLY|O_TRUNC, which silently overwrites
an existing file. The doc comment already stated the intent ("we do not retry
on EEXIST"), but O_TRUNC never yields EEXIST — two identical messages sent on
the same composite-pinned nanosecond would produce the same filename and the
second Send would silently lose the first message. Switch to
O_CREATE|O_EXCL|O_WRONLY so a collision surfaces as an error; O_EXCL also
refuses to follow a symlink at the final path component. Add a regression test.
Addresses greptile review on PR #83.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(inbox): remove the freshly-created file when write or close fails
The O_EXCL switch creates the inbox file before writing its body; if
WriteString or Close then fails, the empty/partial .md was left on disk and
the agent's next inbox scan would pick up a truncated ghost message. Remove
the file on those error paths. O_EXCL guarantees the file did not exist before
this call, so the cleanup can only delete our own partial write, never a
legitimate earlier message.
Addresses greptile review on PR #83.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(messenger): reduce ao send to live pane delivery
* fix(send): preserve messages and map lookup errors
* fix(send): reject terminated sessions
Summary
Closes the last gap to a live
ao spawn: a registered project can now spawn a real worker agent (claude-code/codex) in a zellij pane, end-to-end from the CLI. Stacked on #65 (the agent-adapter + session-manager/daemon wiring this builds on).What's new
RepoResolver— the daemon resolves a project's on-disk repo path from the projects table, replacing the emptyStaticRepoResolver{}that failed every lookup. A registered project is now spawnable.ao/<session-id>(unique per session; gitworktree can't add a worktree on a branch already checked out, e.g.main).ao project add --path <repo>— register a local git repo (POST /api/v1/projects).ao spawn --project <id> [--harness] [--branch] [--prompt] [--issue]— spawn a worker (POST /api/v1/sessions); harness defaults to the daemon'sAO_AGENT.postJSONdaemon client (run-file port lookup + API error envelope surfacing).Demo flow
Requires
zellijand theclaude/codexbinary on PATH.Tests
projectRepoResolverresolves a registered project / errors on an unregistered one.session_managerdefaults the branch toao/<id>and keeps an explicit branch.ao spawn/ao project addreject missing required flags before any network call.go build/go vet/golangci-lint(0 issues) / gofmt clean.