Commit Graph

18 Commits

Author SHA1 Message Date
Priyanshu Choudhary 8bbc4c94fe
fix: hide backend subprocess windows on Windows (#2179) 2026-06-25 16:21:18 +05:30
Harshit Singh Bhandari a96143b502
Zellij to tmux + ConPTY runtime, session save/restore, crash-proof reconcile (port #404) (#2183)
* Zellij to Tmux and some other fixes. (#404)

* feat(runtime): add tmux adapter package

Adds backend/internal/adapters/runtime/tmux implementing ports.Runtime via
the tmux CLI. Drop-in replacement for the zellij adapter on Darwin/Linux.

Key design points:
- Handle is a plain session id string (no pane-id split needed for tmux).
- Exact-match session targeting via = prefix for kill-session and has-session.
- Keep-alive shell appended to launch command so sessions survive agent exit.
- send-keys -l chunked for literal text delivery (no key-name interpretation).
- IsAlive distinguishes definitive-dead (missing/no-server output) from probe
  errors so the reaper never kills a session on a transient tmux failure.
- 34 tests pass: 32 unit tests via fakeRunner seam, 2 integration tests on
  real tmux 3.6b (TestRuntimeIntegration, TestRuntimeIntegrationExactSessionParsing).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(tmux): address four code-review findings in tmux runtime adapter

- Remove em dash from tmux_test.go:462 (project hard rule); replace with semicolon
- Derive integration test session IDs from t.Name() so concurrent runs do not collide on the same tmux session
- Remove dead scaffolding variables (r/fr, r2/fr2) in TestCreateDestroysAndReturnsErrorWhenNotAlive
- Quote \${SHELL:-/bin/sh} in buildLaunchCommand and update all asserting tests

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(runtime): wire tmux on Darwin/Linux via runtimeselect, keep zellij on Windows

- New package runtimeselect: Runtime union interface (ports.Runtime +
  SendMessage/GetOutput/AttachCommand) with compile-time assertions for
  both adapters. New(log) returns tmux on non-Windows, zellij on Windows
  (replicating the old daemon socket-dir setup).
- daemon.go: replace zellij-specific socket-dir block with
  runtimeselect.New(log); update comment to be runtime-neutral.
- lifecycle_wiring.go: startSession param changed from *zellij.Runtime
  to runtimeselect.Runtime.
- cli/doctor.go: runtime-aware checkTerminalRuntime (tmux on Darwin/Linux,
  zellij on Windows); added checkTmux.
- cli/spawn.go: attach hint prints tmux attach -t <name> on non-Windows,
  keeps zellij attach hint on Windows.
- wiring_test.go: startSession test uses runtimeselect.New(nil); zellij
  direct tests retained for zellij-specific coverage.
- doctor_test.go: replaced three zellij tool tests with tmux equivalents.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: tidy runtime-neutral comments and doctor import grouping

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(tmux): drop unused runner.Start seam

tmux creates sessions detached via new-session -d, so the Start method
(carried over from the zellij runner shape, where it backs the Windows
fire-and-forget spawn) is never called. Remove it from the interface and
its implementations to shrink the seam.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(conpty): add protocol codec and output ring buffer (pure Go, OS-agnostic)

Ports the ConPTY named-pipe binary framing protocol and rolling output
buffer from pty-host.ts to Go. Implements EncodeMessage, MessageParser
(handles arbitrary chunk boundaries, payload copy guarantee), and Ring
(MaxOutputLines=1000, ANSI-safe, concurrent Append+Snapshot). All 15
unit tests pass on Darwin; GOOS=windows build is also clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(conpty): harden copy-safety and add concurrent ring test

Strengthen TestParserPayloadIsCopy to catch internal-buffer aliasing:
feed frame1, capture its payload, feed frame2 of the same length so the
parser's buffer overwrites the frame1 region, then assert frame1's bytes
are unchanged. The prior test only mutated the input slice post-Feed and
did not exercise the real aliasing risk.

Add TestRingConcurrent: 10 writer goroutines (Append) and 10 reader
goroutines (Snapshot + Tail) running concurrently with a WaitGroup. The
test is meaningful only under the race detector and catches any missing
mu coverage on Ring's exported methods.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(ptyregistry): port Windows pty-host sideband registry to Go

Adds package ptyregistry under backend/internal/adapters/runtime/conpty/ptyregistry.
Ports windows-pty-registry.ts: defensive read, atomic temp+rename write,
delete-on-empty, register-replaces-same-ID, and auto-pruning List.
PID liveness isolated behind build tags (syscall.Kill on Unix,
OpenProcess on Windows). 10 tests all green on Darwin.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(sdd): phase B briefs and progress for B1-B3

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(conpty): add pty-host serve engine with loopback TCP transport (B3)

Ports pty-host.ts behavior to Go: ptyConn interface seam, Serve engine
with ring replay, fan-out broadcast, MSG_* handlers, PTY-exit keep-alive,
and graceful shutdown (ConPTY dispose first, 50ms grace, then clients and
listener). Real conptyConn is Windows-only via build tag; non-Windows stub
keeps the package importable on Darwin/Linux. Tests use a fake ptyConn
with real loopback sockets and the B1 MessageParser, passing with -race.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(conpty): deliver scrollback snapshot and register client atomically

Review of Task B3 found one Important bug and two minors.

Important: in handleConn the ring Snapshot and the client registration
ran under two separate h.mu acquisitions. A PTY chunk arriving in that
gap was in neither the snapshot nor that client's broadcast, so it was
silently dropped (a hole in the client's stream). Now take the snapshot,
write it to the conn, and add the conn to the clients set all under a
single h.mu hold; broadcast also takes h.mu so it cannot interleave.
Added TestScrollbackLiveOrdering_NoDrop, which emits a contiguous
numbered stream while a client connects and asserts the client's stream
has no internal gap. It reliably fails against the old two-step code and
passes under -race -count=20.

Minor (faithfulness): conptyConn.Close() now also best-effort
Process.Kill() (nil-guarded) so a child that ignores ConPTY EOF still
exits and Done() fires, mirroring pty.kill() in pty-host.ts.

Minor (simplify): use os.Environ() instead of
exec.Command(shellCmd).Environ() for the child env.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(sdd): B4 brief

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(conpty): add runtime adapter with loopback pty-client and session management (B4)

Implements the conpty Runtime adapter: injectable spawn seam, loopback
TCP client helpers (SendMessage/GetOutput/IsAlive/Kill), and Runtime
methods (Create/Destroy/IsAlive/SendMessage/GetOutput). Session resolution
uses an in-memory map with B2 registry fallback for daemon-restart
recovery. Windows-only detached spawn in spawn_windows.go; stub errors
on other OSes. All adapter methods are unit-tested on Darwin against an
in-process B3 Serve and fakePTY. 48 tests pass, all three GOOS builds
succeed, vet clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(conpty): split IsAlive dead-vs-transient for reaper safety

clientIsAlive collapsed every probe failure (dial timeout, read-deadline
expiry, write error, connection-refused) to false, which the reaper turns
into ProbeDead and the LCM can promote to a permanent reap. A single
transient 2s loopback timeout would spuriously kill a live idle session.

Now clientIsAlive returns (alive bool, transientErr error): a refused dial
is definitively gone (false, nil); a timeout or any connected-then-failed
I/O error is transient (false, err) so the reaper records ProbeFailed and
retries. Wire IsAlive to propagate it. Add regression test covering both
the refused-is-gone and timeout-is-transient paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(sdd): B5 brief + ledger

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(terminal): stream-based Attach for tmux/zellij/conpty

Evolve the terminal layer from argv-based attach (PTYSource.AttachCommand
+ injected spawnFunc) to stream-based attach (Source embedding
ports.Attacher). tmux/zellij keep spawning their attach CLI on a local
PTY via the new shared ptyexec.Spawn; conpty attaches by dialing its
loopback pty-host directly with a loopbackStream over the B1 framing
protocol. Reattach/backoff/size/SIGWINCH/detach semantics are unchanged.

- ports: add Stream + Attacher.
- ptyexec: new shared package holding the creack/pty (unix) and ConPTY
  (windows) spawn, moved verbatim from terminal with its tests.
- terminal: PTYSource -> Source, drop spawnFunc/WithSpawn, run loop calls
  src.Attach and uses ports.Stream.
- tmux/zellij: add Attach (argv via ptyexec.Spawn); conpty: add Attach
  (loopbackStream); ports.Attacher assertions on all three.
- runtimeselect: union embeds ports.Attacher in place of AttachCommand.
- tests migrated; new conpty attach_test against in-process Serve+fakePTY.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* style(ptyexec): replace em dashes carried from moved pty files

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(sdd): B6 brief + B5 ledger

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(runtime): select conpty on Windows, register pty-host subcommand, delete zellij

- runtimeselect.New: Windows branch now returns conpty.New(conpty.Options{}) instead
  of zellij; compile-time assertion updated to conpty.Runtime.
- cli/ptyhost.go: new hidden "ao pty-host" subcommand (DisableFlagParsing so agent
  shell args with leading dashes survive); calls conpty.RunHost and exits with its code.
- cli/root.go: wires newPtyHostCommand alongside newLaunchCommand.
- cli/doctor.go: Windows terminal-runtime check replaced with a static ConPTY
  built-in pass; zellij import and checkZellij function removed.
- cli/spawn.go: Windows attach hint updated to dashboard message (ConPTY has no
  CLI attach); zellij import removed.
- daemon/lifecycle_wiring.go: stale zellij comment updated to tmux/conpty.
- daemon/wiring_test.go: zellij import and TestDaemonZellijSocketDir test removed;
  TestWiring_StartLifecycleThreadsMessengerIntoLCM now uses tmux.New.
- terminal/attachment_integration_test.go: re-pointed at real tmux
  (TestAttachmentStreamsRealTmuxPane + TestAttachmentReattachAdoptsNewSize);
  sessions cleaned up in t.Cleanup.
- internal/adapters/runtime/zellij: deleted entirely.

All three GOOS builds pass; go test -race ./... 1607 passed; go vet clean;
grep -rn "runtime/zellij" returns nothing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(daemon): correct terminal-runtime comment to conpty on Windows

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(ptyexec): drop stale zellij reference in Windows spawn comment

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(sdd): final phase B ledger

* build(desktop): support local keychain signing for macOS builds

Bridge forge.config.ts to accept the local keychain flow (APPLE_SIGNING_IDENTITY
identity + AO_NOTARY_PROFILE notarytool profile) in addition to the existing CI
secrets path (CSC_LINK + APPLE_ID/app-specific-password). Enables a signed +
notarized macOS build from a developer Mac without exporting a .p12 or the Apple
ID app-specific password.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(daemon): default TERM so Finder-launched tmux attach works

A Finder/Dock launch starts the supervisor under launchd with no
controlling tty, so TERM is unset. The daemon inherits that, and its
tmux attach client (spawned with env=nil, inheriting the daemon env)
dies immediately with "open terminal failed: terminal does not support
clear" — the orchestrator terminal pane never opens.

Seed TERM=xterm-256color (what the renderer's xterm.js emulates) as the
base of buildDaemonEnv, the same place PATH is reconstructed for the same
class of "Finder launch lacks a terminal's env" bug. A real TERM from the
shell/process env still wins.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(lifecycle): plan for save-on-close/restore-on-open sessions

Captures the intended daemon lifecycle: on shutdown save every running
session (worker and orchestrator) plus its gitignore-respecting uncommitted
work to refs/ao/preserved/<id>, then force-remove worktrees; on boot recreate
worktrees, replay the preserved work, and restore all sessions. Reuses
existing SQLite state, session_worktrees.preserved_ref, manager.Restore, and
the /shutdown endpoint (no new file, migration, or route).

Also gitignore the built daemon binary copied into frontend/daemon/.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(frontend): sync regenerated pnpm-lock and routeTree

Working-tree regeneration of the pnpm lockfile and TanStack Router generated
route tree. No hand edits; generated output only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(workspace): add ForceDestroy for shutdown-path worktree removal

Adds ForceDestroy(ctx, info) to ports.Workspace and the gitworktree
adapter. It runs `git worktree remove --force`, then prune, then
os.RemoveAll as a backstop. A new worktreeForceRemoveArgs builder in
commands.go emits --force; the existing worktreeRemoveArgs is untouched
so Destroy still refuses dirty worktrees via ErrWorkspaceDirty.

TDD: test first creates a dirty worktree, confirms Destroy refuses with
ErrWorkspaceDirty, then confirms ForceDestroy succeeds and the path is
gone and deregistered. All 1609 backend tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(workspace): add StashUncommitted and ApplyPreserved for session lifecycle

Implements the correctness-critical save-on-close / restore-on-open pair
in the gitworktree adapter:

- StashUncommitted: captures uncommitted work (tracked edits and new
  non-ignored files) via a temp GIT_INDEX_FILE into a real commit stored
  at refs/ao/preserved/<session-id>. Never touches the real index or
  stash stack. Returns empty string for clean worktrees. Logs the count
  of .gitignore-skipped paths.
- ApplyPreserved: replays the preserve commit onto a freshly re-added
  worktree via "git checkout <SHA> -- .". Deletes the ref on clean
  success; keeps it and returns ErrPreservedConflict (wrapped) on
  content conflicts.
- Adds both methods to ports.Workspace interface and stubs them in
  integration and session_manager test doubles.

TDD: wrote two failing tests first (RED confirmed via build failure on
undefined methods), then implemented to GREEN. All 39 adapter tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workspace): replace path-checkout with cherry-pick in ApplyPreserved

git checkout <sha> -- . is a path-checkout that always exits 0 for
content divergence, making ErrPreservedConflict unreachable. Replace
with git cherry-pick --no-commit which performs a true three-way merge,
leaves textual conflict markers on conflict, and exits non-zero so the
sentinel is correctly returned. Conflict detection now uses exit code
only (locale-independent). Add TestWorkspaceIntegrationApplyPreservedConflict
to assert: error is ErrPreservedConflict, preserve ref is kept, conflict
markers appear in the file. All 40 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(session-manager): add SaveAndTeardownAll and RestoreAll for shutdown lifecycle

Implements Task 3: capture-then-destroy on shutdown and restore-all on startup.

- Adds ErrPreservedConflict to ports as a named sentinel; gitworktree aliases it
  (following the same pattern as ErrBranchCheckedOutElsewhere).
- Extends the Store interface with UpsertSessionWorktree and ListSessionWorktrees
  so the session manager can write the shutdown-saved marker and read it back.
- SaveAndTeardownAll: for every live session with a workspace path, stash
  uncommitted work, write the session_worktrees row (DB commit before worktree
  removal, crash-safety invariant), mark terminated, destroy runtime, force-remove
  the worktree. Best-effort per session; no kind filter.
- RestoreAll: for every terminated session that has a session_worktrees row (the
  marker written by SaveAndTeardownAll), re-create the worktree, apply any
  preserved ref (conflict logs and continues), then relaunch via the existing
  single-session Restore. Sessions killed by the user before shutdown (no row)
  are skipped. Best-effort per session; no kind filter.
- TDD: 9 new tests (RED confirmed via build failure, GREEN confirmed 63 pass).
  Full suite: 1621 tests across 77 packages.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(terminal): enable tmux mouse scroll and fix link clicking

On macOS the runtime is tmux, but two mouse interactions were broken in
the embedded terminal while copy/paste kept working:

- Scroll: the renderer drives scrolling by writing SGR mouse-wheel
  reports into the pane (the zellij `--mouse-mode true` model), but tmux
  ignores those reports unless mouse mode is on. Create only set `status
  off`, never `mouse on`, so wheel scrolling silently no-opped. Enable
  `set-option -t <id> mouse on`, mirroring the existing status-off step.

- Link clicking: the default WebLinksAddon handler calls window.open()
  with an empty URL and then assigns location.href. Electron's
  setWindowOpenHandler denies every window.open and only forwards the URL
  passed to it, so the empty open is dropped and clicks no-op. Pass the
  matched URL to window.open directly so the main process routes it to
  shell.openExternal (the OS browser).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(session-manager): assert UpsertSessionWorktree precedes ForceDestroy

Add a shared ordered call log (sharedLog *[]string) to both fakeStore
and fakeWorkspace. TestSaveAndTeardownAll_CaptureOrderAndMarker now
wires both fakes to the same slice and asserts upsertIdx < forceIdx,
enforcing the crash-safety invariant that the DB write is committed
before the worktree is force-destroyed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(daemon): wire RestoreAll/SaveAndTeardownAll into boot/shutdown sequence

Exposes session manager through a minimal sessionLifecycle interface
(RestoreAll, SaveAndTeardownAll) returned from startSession, then calls
RestoreAll (best-effort) before srv.Run and SaveAndTeardownAll with a
fresh 30s-bounded context after srv.Run returns. Both SIGTERM and POST
/shutdown funnel through srv.Run returning, so the single save call site
covers both paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(daemon): fix seam-test tautology and lifecycle variable shadow

Finding 1: dispatch both sessionLifecycle methods through an interface
variable (var sl sessionLifecycle = fake) so the runtime body exercises
interface dispatch, not just direct struct method calls.

Finding 2: rename local variable 'lifecycle' to 'lc' in
TestWiring_StartSessionBuildsSessionService to remove the shadow of the
imported lifecycle package.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(frontend): call POST /shutdown before killing daemon on quit

In before-quit, POST /shutdown (8s AbortSignal.timeout) so the daemon
saves sessions gracefully before the SIGTERM kill. Adds a re-entrancy
guard (quitting flag) so a concurrent app.quit() cannot double-preventDefault.
Falls back to killDaemon on fetch failure or timeout: quit is never blocked.
Keeps the process.on('exit') SIGTERM fallback intact.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(storage): guard session_worktrees.state against empty-string CHECK violation; add ponytail comments

The save path (saveAndTeardownOne) never sets domain.SessionWorktreeRecord.State,
so it arrives at UpsertSessionWorktree as "". The generated upsert includes state
in the INSERT column list, so the DB default ('active') is never applied and the
CHECK constraint (state IN ('active', ...)) would fire at the first real shutdown.

Fix: default to 'active' in the store adapter when row.State is "". No schema
change, no migration, no gen edit.

Also add ponytail: comments on the State field (domain type), the write path, and
the read path, documenting that state is unused multi-repo scaffolding and that the
upgrade path is to wire a real value when multi-repo worktree lifecycle states ship.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(storage): add real-SQLite test for empty-State guard in UpsertSessionWorktree

Adds TestUpsertSessionWorktreeEmptyStateDefaultsToActive to the store
test file. It inserts a SessionWorktreeRecord with State at zero value
"" via UpsertSessionWorktree against a real SQLite DB, then reads the
row back and asserts State == "active". This directly exercises the
guard added in the prior commit and would fail if the guard were
removed (the CHECK constraint rejects ""). Mirrors the helpers and
setup pattern of TestSessionWorktreesRoundTrip exactly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(comments): correct shutdown-mechanism and task-ref inaccuracies

Fix 1: daemon.go comment near SaveAndTeardownAll now correctly states
that POST /shutdown closes the shutdownRequested channel (not cancel ctx).
Also tighten the RestoreAll comment to remove the inaccurate claim.

Fix 2: remove "Task 2's" phrasing from ForceDestroy ponytail comment in
workspace.go; condition still references StashUncommitted by name.

Fix 3: add note in main.ts that the 8s fetch timeout is shorter than the
daemon's 30s save bound, so a SIGTERM after fetch abort does not cut the
in-flight save short.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: remove .superpowers workflow scratch from repo

These SDD workflow artifacts (task briefs, agent reports, progress ledger,
review packages) were committed by accident in prior work, against the
.superpowers/sdd/.gitignore intent. Remove them from the repo; they remain
local-only scratch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(spec): graceful restore + post-failure orchestrator recreate

Fix the opaque 500 when restoring an un-resumable session (typed 409
SESSION_NOT_RESUMABLE), and add a post-failure popup that offers to recreate a
fresh orchestrator on the same branch (cleaning the worktree, preserving
committed history). Orchestrators only; recreate fires only after a restore
attempt confirms the session cannot be resumed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(plan): restore-recreate orchestrator; reuse existing /orchestrators clean=true

Planning discovery: the recreate capability already ships via POST /orchestrators
(clean=true), which kills the dead orchestrator and re-spawns on the canonical
branch (addWorktree reattaches an existing branch). So the feature collapses to a
typed-error fix plus a frontend popup. Spec updated to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(session): return typed SESSION_NOT_RESUMABLE instead of 500 on un-resumable restore

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(renderer): offer recreate-orchestrator popup when a session cannot be restored

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(spec): drop stale OpenAPI-regen note (feature adds no route)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(ci): gofmt/goimports, golangci-lint hygiene, and Windows-aware doctor tests

Formatting: ran gofmt and goimports (with local-prefixes) on the 8 listed
files plus ptyexec/spawn_unix.go which the linter also flagged.

Lint (25 issues fixed):
- gosec G115: EncodeMessage now returns ([]byte, error) with an explicit
  bounds check before the int->uint32 conversion; all callers updated.
- govet nilness: removed dead `if lastErr == nil` branch in clientIsAlive;
  lastErr is provably non-nil at that point (real bug).
- nilerr: extracted runAcceptLoop helper so Accept-error-on-close is not
  flagged; listener close is normal shutdown, not a caller error.
- staticcheck SA4010: removed dead `full = append(...)` loop in host_test.
- revive var-declaration: `var prev int = -1` -> `prev := -1`.
- revive redefines-builtin-id: deleted local `min` helper; builtin covers it.
- unparam (2): dropped always-nil env return from attachCommand; dropped
  unused shellPath param from buildLaunchCommand; updated callers.
- errcheck (8): deferred Close/Remove calls wrapped in func(){_ = ...}();
  type assertion in host_main.go uses ok-form; fmt.Fprintf to stdout uses
  _, _ = pattern; workspace.go tmpIdx.Close() uses _ =.
- gocritic nestingReduce: inverted if+continue in runtime.go resolve loop.

Windows E2E: skip TestDoctorChecksTmuxVersion,
TestDoctorChecksTmuxVersionFailsOnError, TestDoctorWarnsWhenTmuxMissing on
windows (ao doctor emits a conpty check there, not tmux).

Verified: gofmt -l . clean, golangci-lint 0 issues, go build ok,
go test -race 1624/1624 pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): set git identity in worktree clone fixture; loosen tmux reattach timeouts

The preserve round-trip/conflict tests commit inside a worktree of the cloned
repo, which had no git identity; CI runners cannot auto-derive one, failing with
"empty ident name". Set user.email/user.name on the clone in setupOriginClone so
its worktrees inherit it.

The tmux reattach test drives a real shell and parses stty output, which is slow
under -race on CI; raise its echo-write and SIZE-output waits.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(terminal): resend size probe on tmux reattach until the shell answers

Bumping timeouts was the wrong fix: a 30s wait still failed, so the probe output
deterministically never appeared, not slowness. onOpen signals the stream accepts
input, not that the reattached sh -i is at a prompt, so the first echo keystroke
can be dropped. Resend the probe each poll until SIZE output lands, and on timeout
dump the captured pane buffer so a remaining failure is self-explaining.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(terminal): set TERM for real-tmux attach tests so they run in CI

Root cause (from the buffer dump the prior commit added): with TERM unset on CI
runners, tmux refuses to attach a client and prints "open terminal failed:
terminal does not support clear", so the pane never runs the size probe. The
daemon defaults TERM in production; the tests bypass it. Set TERM=xterm-256color
in both real-tmux tests. Reproduced locally with `env -u TERM` (fails the same
way) and verified the fix passes under it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(spec): crash-proof session reconcile design

Boot-time reconcile makes live tmux + worktree state match the DB on every
daemon start, so a SIGKILL/crash/force-quit that skips SaveAndTeardownAll no
longer leaks an orphaned daemon, tmux sessions, or worktrees. Adopt
crash-surviving tmux sessions, preserve-and-terminate dead ones, reap
in-namespace orphans, and add a frontend kill+replace branch for a wedged
orphan daemon.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(spec): simplify reconcile to per-session IsAlive, drop ListSessions

Every leak in the incident maps to a DB row, so orphan-reap is a per-session
IsAlive+Destroy over terminated rows; no runtime enumeration, no ports/conpty/
runtimeselect changes. Reaping a tmux session with no DB row is deferred (YAGNI).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(plan): crash-proof session reconcile implementation plan

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(session): reconcile live pass (adopt alive, stash+terminate dead)

* feat(session): reconcile reap pass and Reconcile entry point

* feat(daemon): run Reconcile on boot in place of bare RestoreAll

* test(integration): reconcile terminates dead-live sessions and reaps leaked tmux

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(integration): correct misleading CreateSession comment in reconcile test

* feat(frontend): kill+replace a wedged orphan daemon on launch

When both inspectExistingDaemon and resolveDaemonFromPort return null but
a process still holds the daemon port (a crashed/orphaned daemon), spawning
a new Go child would collide on the port and exit 1. Detect this case, SIGTERM
the holder (via the run-file PID, falling back to the probe PID), poll until the
port is free (up to 8s), clear the stale run-file, then proceed to spawn fresh.
The healthy-daemon reuse path is unchanged.

Pure helper: src/shared/daemon-takeover.ts (planDaemonTakeover)
Unit tests:  src/shared/daemon-takeover.test.ts (3 tests, TDD red-green)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(frontend): fire orphan-daemon takeover when a holder actually exists

Replace planDaemonTakeover (inverted logic: ran kill block only when probe
was null) with shouldReplacePortHolder(probe, holderPidAlive) which returns
true when a real holder exists: non-null probe (rejected responder) OR a
run-file PID that is still alive (hung holder). Update main.ts call site to
compute PID liveness before gating the kill block. Update tests to cover all
three distinct outcomes non-vacuously.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs+test: accurate takeover comments, reconcileLive probe-error test, Reconcile doc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(session): restore promptless orchestrators and crash-orphaned sessions

The orchestrator was abandoned on every app open: a fresh orchestrator
spawned each launch and the prior conversation appeared lost (it was not;
the transcript stays in ~/.claude, resumable by the deterministic
--session-id AO pins). Two defects combined:

1. Restore's guard rejected any session with no agentSessionId AND no
   prompt as ErrNotResumable. But Claude resumes via a deterministic
   session id regardless of those fields, so promptless orchestrators
   were perfectly resumable yet always rejected. Workers slipped through
   only because they carry a prompt. Move the resumability decision to the
   adapter: restoreArgv returns ErrNotResumable only when GetRestoreCommand
   reports it cannot resume AND there is no prompt to fresh-launch from.

2. reconcileLive marked a crash-orphaned (dead-runtime) session terminated
   without a restore marker, so RestoreAll skipped it and it stayed dead.
   It now saves-and-tears-down to the same end state a graceful shutdown
   produces (capture work, write the session_worktrees marker, terminate,
   remove the worktree), so RestoreAll relaunches it on the same boot,
   resuming history. Crash recovery now matches graceful restart. If work
   capture fails it terminates without a marker rather than risk losing
   un-preserved work.

Tests: promptless orchestrator restores via adapter resume; promptless
session with a non-resuming adapter still returns ErrNotResumable;
reconcileLive writes the marker + tears down the worktree. Full backend
suite green (1632), gofmt/vet clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 9ae05735d6f06ac989857534bae2766392772c71)

* chore: format with prettier [skip ci]

* docs: scrub stale zellij references after tmux/conpty migration (#409)

PR #404 migrated the runtime adapter from Zellij to tmux (Darwin/Linux)
plus conpty (Windows), selected via runtimeselect, but ~30 stale zellij
references lingered in comments and docs describing zellij as the current
runtime. This is a comments/docs-only cleanup with no behavioral change:
comments now say tmux (or tmux/conpty when both platforms are relevant),
terminal/doc.go and docs/backend-code-structure.md are rewritten to
reflect the tmux + conpty + runtimeselect attach model, and the daemon
environment, STATUS, stack, architecture, and CLI docs are updated.

Also gitignore the local .codegraph/ and .cursor/ tooling dirs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: format with prettier [skip ci]

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2026-06-25 15:05:41 +05:30
Vaibhaav Tiwari c6d9692d37
feat(frontend): add live browser panel (#375)
* feat(frontend): add live browser panel

* chore: format with prettier [skip ci]

* feat: preserve and auto-open browser previews

* fix: retry browser preview after session updates

* fix: wait for browser view before preview navigation

* fix: reopen preview after session switches

* fix: preserve browser views across session switches

* chore: format with prettier [skip ci]

* feat: add `ao preview` command to drive the session browser panel

Replaces the browser panel's auto-detect with an explicit, session-scoped
command. `ao preview [url]` runs inside a session (derives the target from
AO_SESSION_ID; rejects when unset or when the session is unknown):
- with a url, opens it verbatim (file://, http, https; no sanitization for now)
- with no url, autodetects index.html in the workspace as before

The resolved target is persisted as a new `previewUrl` session field and fans
out over the existing CDC /events stream (the sessions update trigger now fires
on preview_url and carries previewUrl in its payload). The desktop browser panel
reflects session.previewUrl: it opens, switches the center pane to the browser,
and navigates, re-navigating only when the target changes.

ponytail: file:// preview targets are accepted unsanitized; agent-trusted for now.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(cli): document the `ao preview` command

Add `ao preview` to the CLI command tables in README.md and docs/cli/README.md,
noting it resolves its session from AO_SESSION_ID and its no-arg autodetect vs
explicit-URL behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: format with prettier [skip ci]

* fix(frontend): reveal `ao preview` in the inspector Browser tab, not the center pane

`ao preview` set session.previewUrl, and SessionView surfaced it by
popping the browser into the center pane, replacing the terminal. Reveal
it in the inspector rail's Browser tab instead (opening the rail if it is
collapsed); the manual pop-out button still expands it to the center.

Lifts the inspector's active tab to an optional controlled prop so
SessionView can drive it, and adds a regression test asserting the center
pane keeps the terminal while the rail switches to Browser.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs: instruct agents to `ao preview` when showing frontend changes

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Vaibhaav <user@example.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Harshit Singh Bhandari <dev@theharshitsingh.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-22 04:14:18 +05:30
Laxman 43ee6c9b02
Feat/backend telemetry v0 (#307)
* feat(backend): add telemetry event plumbing

* feat(backend): emit session telemetry events

* feat(backend): add http and cli telemetry export

* feat(backend): add onboarding and dwell telemetry

* feat(frontend): add renderer posthog telemetry

* feat(frontend): bundle posthog project defaults

* feat(telemetry): add canonical active-user event

* fix(telemetry): repair cli test expectations

* fix(telemetry): sanitize remote payloads and respect event toggles
2026-06-18 22:00:25 +05:30
Madhav Kumar 6885af26b2
feat(windows): ConPTY terminal, zellij discovery, agent launcher trampoline, codex shim resolution (#310)
* fix(daemon): self-heal a stale run-file instead of refusing to start

On Windows the desktop supervisor can only TerminateProcess the daemon
(no POSIX signal reaches a detached child), so the daemon's graceful
shutdown never runs and ~/.ao/running.json is never removed. The leaked
file survives into the next launch, and because Windows reuses PIDs
aggressively the recorded PID usually belongs to an unrelated process.
The startup pre-flight trusted PID liveness alone (runfile.CheckStale ->
processalive.Alive), so it concluded a daemon was "already running" and
exited with "refusing to start" on every restart. A dead daemon then
makes the renderer's loopback REST calls (e.g. Spawn Orchestrator) fail
silently.

Verify the recorded port is actually served by an AO daemon with the
recorded PID (a /healthz probe matching service + pid, the same ground
truth inspectDaemon already uses) before refusing. A run-file left by a
crashed, hard-killed, or reused-PID predecessor is treated as stale and
overwritten, so startup is robust to a leaked run-file from any cause.

Fixes #256

* fix(release): build the desktop daemon natively on each target OS

build-daemon.mjs compiles the bundled `ao` daemon with the build host's
GOOS and names it off the host platform (ao.exe only when the builder is
Windows). The release workflow ran only on macos-latest, so a Windows
package would ship a macOS binary named `ao` with no `ao.exe`, and the
app could not launch a valid Windows daemon ("This program cannot be run
in DOS mode" / binary not found).

Run the release as a per-OS matrix (macOS + Windows) so host == target
and each installer bundles a daemon compiled for its own platform, and
pin the Go toolchain with setup-go since build-daemon needs it on every
runner.

Fixes #235

* feat(terminal): Windows ConPTY support for /mux attach

Replaces the Windows stub in internal/terminal/pty_windows.go with a real ConPTY implementation backed by github.com/aymanbagabas/go-pty, so the daemon's /mux attach can stream a live terminal to the renderer on Windows.

PTYSource.AttachCommand now returns (argv, env, err). On Windows the zellij attach is spawned directly (no powershell.exe wrapper) — wrapping ConPTY startup around a shell surfaces as modal application-error dialogs — and the per-session ZELLIJ_SOCKET_DIR is delivered via the spawn's CreateProcess env block instead of an 'env -u NO_COLOR' shim. Unix continues to use the env-shim wrapper and returns nil env.

Adds go-pty v0.2.3 (+ bumps golang.org/x/sys to v0.44.0 transitively). Updates the in-process test fakes (terminal/fakes_test.go, httpd/terminal_mux_test.go) for the new signature.

* feat(zellij): discover zellij binary on Windows and raise command timeout

Defaults the zellij binary to whatever exec.LookPath finds first (preferring zellij.exe on Windows), falling back to LOCALAPPDATA\Programs\zellij\zellij.exe and ProgramFiles{,(x86)}\{zellij,Zellij}\zellij.exe so a fresh-installed Windows user gets a working runtime without setting Options.Binary.

Raises the per-command timeout from 5s to 30s on Windows: the first zellij invocation after install routinely takes longer than 5s on Windows due to filesystem/AV warmup, which was causing benign DeadlineExceeded failures during session create.

* feat(zellij,cli): Windows agent launcher trampoline for codex argv

On Windows, zellij's KDL `args` quoting cannot round-trip codex's --config key=value flags (or any argv with embedded quotes), and shell-wrapping the agent in powershell/cmd quoting is equally unsound. This adds a small launch trampoline so zellij runs a known-fixed argv and the real argv is delivered out-of-band.

How it works on Windows:

1. zellij.Runtime.writeLayout persists cfg.Argv to a temp JSON spec via the new agentlaunch package (AO_LAUNCH_SPEC env var points at the file).

2. The KDL layout runs the trampoline as `<ao.exe> launch` (windowsLaunchArgv); PATH is augmented so the trampoline resolves.

3. The new hidden `ao launch` subcommand reads the spec, deletes the temp file, and execs the real agent with cfg.Argv inside cfg.WorkspacePath.

Also adds:

- runner.Start fire-and-forget path (process_windows.go uses powershell.exe -EncodedCommand + Start-Process -WindowStyle Hidden with CREATE_NEW_CONSOLE so the daemon is not blocked on zellij's --create-background settling).

- powerShellEncodedCommand helper and switch from -Command to -EncodedCommand for the existing powershell shellLaunchSpec (avoids brittle KDL→PowerShell quoting round-trips).

Unix is unchanged: writeLayout passes cfg.Env straight through, createSession stays synchronous via runner.Run, and process_other.go is a stub that returns an error if anyone calls into the background path.

* feat(codex): Windows binary resolution, terminal compat flags, TOML literal strings

Three Windows-targeted refinements to the codex agent plugin so a default Windows install lands in a working state:

1. ResolveCodexBinary now follows .cmd/.ps1 shims to the underlying codex.exe (resolveNativeWindowsCodex + windowsNativeCodexCandidatesForShim). The npm-distributed codex shim cannot be exec'd directly under ConPTY without a shell wrapper; jumping straight to the .exe avoids that wrapper.

2. appendTerminalCompatibilityFlags adds Windows-specific args (e.g. --no-alt-screen) so codex's TUI renders correctly inside zellij's pane without the alternate-screen buffer churn that breaks ConPTY redraws.

3. hooks.go gains codexTOMLLiteralString / codexTOMLConfigString / containsTOMLControl so paths and other values with backslashes and quotes round-trip through codex's --config TOML parser using literal strings ('...') when basic strings would require unsafe escaping.

* fix(lint): paramTypeCombine in pty_unix.go, revive doc comments in agentlaunch, codex test quotes

* fix: stabilize windows zellij sessions

---------

Co-authored-by: harshitsinghbhandari <24b4506@iitb.ac.in>
Co-authored-by: Madhav <madhavkumar@microsoft.com>
Co-authored-by: Vaibhaav <user@example.com>
2026-06-18 21:51:05 +05:30
Harshit Singh Bhandari 5fff98087a
feat(import): rewrite-side legacy → rewrite first-boot import (#314)
* feat(import): rewrite-side legacy → rewrite first-boot import

Port the legacy-side TS reader (AgentWrapper #2144/#2129) to Go and run the
migration inside the rewrite as an opt-in import, per the FINAL v2 plan. Reads
the legacy flat-file store (~/.agent-orchestrator) read-only and writes the
rewrite's own SQLite DB via the native storage layer; legacy files are never
touched, and a re-run skips existing rows, so a declined or failed import loses
nothing.

What's included:
- internal/legacyimport: Go reader + field mappers (issue #247). Lifecycle
  double-decode (lifecycle key or statePayload+stateVersion:"2"),
  role/orchestrator detection, sessionPrefix fallback (first 12 chars of id),
  8→4 activity-state map, per-harness resume-id selection, permission/harness
  remap, and the claude transcript slug + relocation to the rewrite's
  orchestrator worktree path ({DataDir}/worktrees/{id}/orchestrator/{prefix}-orchestrator).
- store.ImportSession: verbatim session insert (explicit id/num, ON CONFLICT
  DO NOTHING) so the orchestrator lands at id "{prefix}-orchestrator", num 0.
- `ao import`: explicit, idempotent import with --from/--dry-run/--yes/--json.
  Refuses while a live daemon owns the run-file (the daemon is sole writer; the
  import runs offline, matching the #2129 reference).
- First-boot opt-in: `ao start` offers the import before launching the daemon
  when legacy data is present and the rewrite DB has no projects yet. Declining
  or any failure is non-fatal; a non-interactive boot prints a hint instead of
  auto-importing.

Scope (gist §6): all projects + per-project settings, and the single
non-terminated orchestrator session per project (claude-code/codex/opencode;
aider skipped with a note). Workers are not imported (they respawn fresh).

Resume-id mapping (#247 §2.2): agent_session_id carries claudeSessionUuid /
codexThreadId / opencodeSessionId by harness. codexModel and
restoreFallbackReason have no rewrite column, so they are dropped and surfaced
as import notes — codex resumes from the thread id alone, the rest is forensic.

Gate: `go build ./... && go test -race ./...` green (1423 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(import): resolve golangci-lint errcheck/gocritic/nilerr findings

- start.go: check fmt.Fprint* returns in the first-boot import path
- project.go: combine same-typed return params (gocritic paramTypeCombine)
- claude.go: use a pathExists helper so a missing transcript source is a normal
  skip, not an err-then-return-nil (nilerr)
- importer.go: fold best-effort transcript relocation into a switch so the
  non-fatal path no longer returns nil from an error branch (nilerr)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(import): resolve transcript dest path like the daemon; harden lifecycle parse

Code-review follow-ups on the legacy importer:

- claude.go: compute the Claude transcript DESTINATION slug from the
  symlink-resolved orchestrator worktree path (new resolvePhysical, mirroring
  gitworktree.physicalAbs), not the literal path. The daemon resolves that cwd
  through physicalAbs before `claude --resume` runs, so a literal-path slug
  missed the resume bucket whenever any component of AO_DATA_DIR was a symlink
  (custom data dir, macOS /tmp→/private/tmp, symlinked $HOME) — the orchestrator
  would have resumed without its prior context. Source slug now uses the same
  resolver for symmetry.
- orchestrator.go: accept a numeric stateVersion (JSON 2 → float64) as well as
  the string "2" when falling back to statePayload, so a V2 record carried only
  in statePayload is not misparsed as stateless.
- orchestrator.go: build the dropped-resume-metadata note as a joined list
  instead of string concatenation.

Tests: added a symlinked-data-dir dest-slug test and a numeric-stateVersion
fallback test. Gate green: `go build ./... && go test -race ./...` (1425).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-18 19:54:51 +05:30
neversettle da30da5a45
feat(review): configurable AO code review backend (V1) (#192) (#197)
* feat(review): configurable AO code review backend (V1)

Add per-project configurable code review of a worker's PR. A reviewer
agent runs one-shot over the worker's own worktree and posts its result
to the PR; the worker picks the feedback up through the existing SCM
observer review-nudge path.

- domain: ProjectConfig.reviewers (+ default reviewer harness), Review /
  ReviewRun types and verdict/status vocab.
- storage: review + review_run tables (0011), sqlc queries, store methods.
- service/review: rewrite the in-memory stub as a persisted ReviewService
  (Trigger/Submit/List) with a reviewer Runner over agent resolver +
  runtime; ports.PRReviewPoster implemented on the GitHub adapter.
- http: session-scoped routes POST /sessions/{id}/reviews/trigger,
  POST .../submit, GET .../reviews; regenerated OpenAPI + TS types.
- cli: ao review trigger|submit|list.
- frontend: adapt ReviewDashboard to the per-worker reviews API.

Closes #192

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): address review — drop submit/poster/CLI, default reviewer to worker harness

Per PR #197 review feedback:
- Reviewer agent posts its review to the PR itself, so remove the
  ports.PRReviewPoster port, the GitHub review poster, the submit HTTP
  route + DTO, and the service Submit method (#1, #4, #7).
- Trigger spawns the reviewer agent over the worker's worktree with its
  own review prompt, mirroring the session launch flow (resolve agent by
  harness -> argv -> runtime.Create) (#8, #9).
- Default reviewer harness reuses the worker's harness when supported,
  falling back to claude-code; reviewer config stays independent of the
  worker override (#5, #6).
- Drop the `ao review` CLI for this PR's scope (#2, #3).

Regenerated OpenAPI + TS types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(review): restore ao review submit (records verdict+body in AO)

Per maintainer request, bring back `ao review submit`. AO records the
reviewer's verdict and body on the review_run and marks the pass complete;
it does not post to GitHub — the reviewer agent posts its review to the PR
itself.

- storage: add review_run.body (0011), persist via Insert/UpdateReviewRunResult.
- service: restore Submit (no SCM poster) storing verdict + body.
- http: restore POST /sessions/{id}/reviews/submit + SubmitReviewInput.
- cli: ao review submit [worker] --verdict --body (worker from arg/--session/$AO_REVIEW_WORKER).
- runner: reviewer prompt instructs posting to GitHub and recording via ao review submit.

Regenerated OpenAPI + TS types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): move reviewer runner to its own package; sharpen prompt

Per PR #197 review:
- Move the concrete reviewer runner out of the service layer into a new
  internal/review_runner package (package reviewrunner), beside other
  orchestration packages like session_manager. The service keeps only the
  Runner interface + RunSpec it depends on; the agent-resolver + runtime
  launch flow lives in review_runner.
- Sharpen the reviewer prompt: tell the agent to diff against the PR base,
  focus on high-confidence findings, post via `gh pr review`, and record
  the result with `ao review submit`; review-only (no commits/edits).
- Add unit tests for the runner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): simplify review_run schema; provider-agnostic reviewer prompt

Per PR #197 review:
- review_run: status default 'running' (drop 'pending'), drop CHECK
  constraints on status/verdict, drop the updated_at column and the
  session/iteration index. Propagated through queries, domain, store,
  service, and tests.
- Reviewer prompt no longer hardcodes GitHub/gh commands — it instructs the
  agent to use whatever review tooling the provider offers, keeping the
  flow extensible across SCM providers.

Regenerated sqlc + OpenAPI/TS.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): launch reviewer before persisting the run

Trigger now spawns the reviewer agent first and then writes the review_run
with a status derived from the launch outcome (running on success, failed
if it never started), instead of inserting a running row and correcting it
to failed afterwards.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): pluggable reviewer registry distinct from worker harnesses

Reviewers are now their own pluggable adapter set, separate from the worker
agent registry — adding a reviewer (claude-code today, greptile tomorrow) is
a one-line registration that does not widen the worker harness vocabulary,
and a worker harness does not automatically become a valid reviewer.

- domain.ReviewerHarness: a distinct vocabulary (AllReviewerHarnesses) with
  its own IsKnown; ReviewerConfig/Review/ReviewRun use it. ResolveReviewerHarness
  reuses the worker harness only when it is itself a supported reviewer, else
  falls back to claude-code.
- ports.Reviewer: a reviewer-specific contract (ReviewCommand → argv + env)
  that models one-shot / non-prompt CLIs natively instead of forcing every
  reviewer through the worker's interactive GetLaunchCommand(Prompt:...).
- internal/adapters/reviewer: a separate registry + resolver (mirrors the
  worker agent registry) with the claude-code reviewer adapter, which owns the
  review prompt and reuses the worker claude-code launch construction.
- review_runner resolves via the reviewer registry (not the worker
  AgentResolver) and merges AO_REVIEW_WORKER into the adapter's env.
- daemon wires the reviewer resolver. Registry/domain parity is test-enforced.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(review): cover run-scoped reviewer submit

* fix(api): update generated review submit schema

* refactor(review): split core engine (internal/review) from API service

Move the review orchestration (Trigger/Submit/List, run-id generation,
deps, RunSpec/Runner, sentinels) into a transport-independent core package
internal/review (Engine). internal/service/review is now a thin API-flow
boundary: the controller-facing Manager interface + a Service that delegates
to the engine + error re-exports.

This keeps the service layer to API concerns and lets the same engine back a
future in-process CLI trigger without going through HTTP. review_runner now
depends on the core package; daemon builds the engine and wraps it in the
service. No API/schema changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(review): commit-aware trigger, reviewer handle for UI, no env vars

Reworks the review trigger lifecycle and drops env-based coupling:

- review_run gains target_sha (the reviewed commit) and drops iteration.
  A repeat trigger for the same PR head short-circuits to the existing run.
- review gains reviewer_handle_id: the live reviewer pane's runtime handle,
  reused across passes and exposed in the reviews API so the UI can attach
  its terminal over /mux.
- Trigger flow: if a live reviewer pane exists and a new commit arrived,
  message it to re-review; otherwise spawn a fresh reviewer. The run is
  recorded only after the reviewer is launched.
- No environment variables: the reviewer adapter embeds the explicit
  `ao review submit --session <w> --run <id>` command in the spawn prompt
  and the re-review message. CLI submit requires --run/--session (no env
  fallbacks).
- Merge review_runner into internal/review as a Launcher (spawn/notify/alive).
- Trigger returns 201 for a new pass, 200 when reusing an existing run.

Regenerated sqlc + OpenAPI/TS.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): author the reviewer prompt centrally, not in the adapter

Mirror the worker model (session_manager builds the prompt; adapters just
place it via LaunchConfig.Prompt). The reviewer prompt now lives in
internal/review/prompt.go and is passed through ports.ReviewInvocation.Prompt;
the claude-code reviewer adapter just feeds inv.Prompt to its launch command
and returns it as the re-review message. One-shot CLI reviewers may ignore it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(review): split reviewer prompt into system+task, mirroring buildSpawnTexts

Mirror session_manager.buildSpawnTexts for the reviewer: a standing role goes
in the system prompt, the per-pass task (PR/commit + exact `ao review submit`
command) goes in the user prompt. internal/review/prompt.go now returns
(prompt, systemPrompt); both flow through ports.ReviewInvocation and the
claude-code adapter places them via LaunchConfig{Prompt, SystemPrompt}. The
re-review message reuses the per-pass prompt (role already established in the
running pane).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Vaibhaav <user@example.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
2026-06-15 01:17:17 +05:30
yyovil 3c7344b233
[codex] add ao hooks activity command (#113)
* feat: add ao hooks activity command

* fix(activity): address review nits

- lcm: sameActivity ignores LastActivityAt so same-state repeats no-op
  and don't churn UpdatedAt / CDC events.
- cli/hooks: surface stdin read errors to stderr for parity with the
  daemon-error path; still exit 0 so a failed hook can't break the agent.
- claudecode: GetAgentHooks docstring covers Notification + SessionEnd
  (the slice already included them; only the comment was stale).

---------

Co-authored-by: harshitsinghbhandari <dev@theharshitsingh.com>
2026-06-06 20:29:00 +05:30
Harshit Singh Bhandari 3413acca33
feat: ao session claim-pr + spawn --claim-pr wiring (#101)
* feat: add session PR claiming CLI and API

* fix: tighten PR claim rollback and CDC facts

* fix: align PR claim branch with latest main
2026-06-06 00:01:03 +05:30
Harshit Singh Bhandari 210c9df758
feat(cli): enrich ao doctor (#90) (#99)
* feat(cli): enrich ao doctor

* fix(cli): address doctor review feedback
2026-06-03 16:50:54 +05:30
Harshit Singh Bhandari bab0d2d167
feat: add light backend CLI commands (#98) 2026-06-03 16:18:00 +05:30
Harshit Singh Bhandari 010b422bb5
feat(cli): add ao session ls/get/kill/restore (#90) (#92)
* feat(cli): add session commands

* test(cli): cover session json output

* chore(cli): trim unused session response fields
2026-06-03 04:50:45 +05:30
Harshit Singh Bhandari 5435246c9a
feat(cli): add minimal ao send (#83)
* 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
2026-06-02 20:02:47 +05:30
yyovil 57bb63701d
Add `ao spawn` + `ao project add` (spawn a real worker end-to-end) (#77)
* Add `ao spawn` and `ao project add`; resolve project repos for worktrees

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>

* Address Copilot review on #77

- `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>

* Assert the project-not-resolvable sentinel in the resolver test

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>

* Fix ao spawn 500 on long session ids (zellij socket-path overflow)

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>

* test(cli): guard CLI/daemon DTO drift with an e2e round-trip

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>

* fix(cli,daemon): address review findings on ao spawn

- 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>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: harshitsinghbhandari <24b4506@iitb.ac.in>
2026-06-02 18:39:13 +05:30
prateek a34094e7d8
refactor: simplify session lifecycle and zellij runtime (#62)
* refactor: remove canonical lifecycle state

* refactor: move sqlite stores into subpackage (#62)

* refactor: strengthen sqlite generated model types (#62)

* refactor: remove lifecycle notifications (#62)

* docs: remove notification cleanup leftovers (#62)

* refactor: narrow lifecycle manager scope (#62)

* refactor: keep PR nudges in lifecycle (#62)

* refactor: trim unused storage and lifecycle contracts (#62)

* refactor: align storage and runtime observation surfaces (#62)

* refactor: remove stale daemon and adapter bloat (#62)

* test: fix terminal ring race assertion (#62)

* refactor: trim lifecycle and http boilerplate (#62)

* refactor: expose sqlite CDC source directly (#62)

* refactor: share process liveness checks (#62)

* test: trim lifecycle store fake surface (#62)

* refactor: separate PR observations from storage rows (#62)

* refactor: trim remaining cleanup surfaces (#62)

* refactor: narrow observation and PR display APIs (#62)

* refactor: move PR write DTOs out of domain (#62)

* refactor: normalize PR domain storage types (#62)

* refactor: remove unused session port interface (#62)

* fix: reject unexpected CLI arguments (#62)

* refactor: use session metadata for spawn completion (#62)

* refactor: narrow session runtime dependency (#62)

* fix: validate zellij version in doctor (#62)

* refactor: split observation port DTOs (#62)

* chore: add sqlc generation script (#62)

* refactor: clarify terminal mux naming (#62)

* fix: tolerate nil loggers (#62)

---------

Co-authored-by: itrytoohard <ayetrytoohard@gmail.com>
2026-06-01 08:42:49 +05:30
itrytoohard 2d00e4675d fix(cli): harden daemon control surface and stop CLI from writing the store
Addresses review findings on PR #53 (on top of the rebase onto main).

- doctor: stop opening/migrating SQLite. The daemon is the sole store
  writer/migrator (architecture.md §7); the CLI must not run migrations or
  open a second writer against a DB a live daemon owns. doctor now reports
  database-file presence and gains --json.
- stop: only remove running.json when it still belongs to the PID we
  stopped, so a concurrent `ao start` that wrote a new run-file is not
  clobbered into looking stopped.
- httpd: gate POST /shutdown to loopback callers with no Origin header,
  closing the CSRF / DNS-rebinding vector against an unauthenticated,
  state-changing endpoint.
- start: detach the spawned daemon into its own session/process group so a
  Ctrl-C while `ao start` waits for readiness doesn't also kill it.
- cli: exit 2 for usage errors (bad flag / arg count) vs 1 for runtime
  failures.
- daemon: unexport newLogger (only used in-package).
- tests: /shutdown guard (cross-origin + rebinding) and stop run-file
  ownership guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-01 01:55:14 +05:30
Dhruv Sharma 2f4662b470 fix(cli): handle stale start and graceful shutdown 2026-06-01 01:45:59 +05:30
Dhruv Sharma 4671d27307 feat(backend): add cobra cli foundation 2026-06-01 01:44:43 +05:30