Two PRs (#2193, #2200) merged ~2h apart each added a migration
file numbered 0020, since each branch only saw one such file at
CI time and neither was rebased against the other before merge.
Renumbers 0020_pr_reviews.sql to 0021_pr_reviews.sql.
TestMigrationVersionsAreUnique already existed and is correct;
it didn't catch this because it only runs against each PR's
branch state, not the merged result of both PRs together.
The reviewer's read-only guarantee was enforced only by the prompt. Add
AllowedTools/DisallowedTools to ports.LaunchConfig and plumb them through
the claude-code agent adapter to --allowedTools/--disallowedTools (each list
comma-joined into one value so a rule containing spaces like "Bash(git
diff:*)" is not split into separate tool names). Empty lists emit nothing, so
worker sessions are unaffected.
Launch the reviewer off bypassPermissions (which skips the permission system
and ignores allow/deny rules) in the default auto mode, with an allowlist
scoped to Read/Grep/Glob and the few Bash commands a reviewer needs (gh, git
diff/log/show/status, ao review submit) and an explicit deny list for
Edit/Write/NotebookEdit/git push/git commit as defense in depth.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(settings): IPC bridge for global update settings (get/set)
Expose readUpdateSettings/writeUpdateSettings to the renderer via
updateSettings:get/set so the Global Settings page can read and persist
the auto-update channel choice. Refs #2207.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(cli): show ao start download + install progress
The fetch path streamed a hundreds-of-MB asset and ran a silent install
with zero output, so a slow download looked like a hang. Print a start
line (asset, ~size, repo), stream a TTY-aware progress reader (live \r
percentage on a terminal, start+done lines off a TTY), and announce the
otherwise-silent unpack/install step. stderr only, so --json stdout stays
clean; no new dependency. Closes#2234.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(settings): Updates (channel) + Migration sections in Global Settings
- UpdatesSection: pick Stable vs Nightly auto-update channel, persisted to
~/.ao/update-settings.json via the new IPC bridge. Closes#2207.
- MigrationSection: drop-in card showing migration status + last report/error
and a Run/Re-run button hitting the idempotent POST /api/v1/import; works
even after Don't Migrate. Closes#2205.
- Wire both into the (previously blank) GlobalSettingsForm; cover both with tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: format with prettier [skip ci]
* fix(browser): re-measure preview bounds after layout transitions
The native WebContentsView preview is positioned from a renderer-measured
rect, re-measured on ResizeObserver + window resize/scroll. But a
ResizeObserver fires on size changes only, so a position-only layout shift
(entering/leaving pop-out moves the slot to another panel; opening the
inspector, which ao preview does, reflows its x) left the overlay at stale
bounds, visibly spilling over the sidebar/terminal until an unrelated resize
fixed it. Drive a settle re-measure on mount and on active/pop-out
transitions (immediate frame + once the ~240ms panel transition settles) so
the final geometry always wins.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore(backend): gofmt + goimports pre-existing files (fix red CI)
config.go, project_test.go, importer_test.go and spawn_windows.go were
committed unformatted earlier and fail go.yml's gofmt check and golangci's
goimports formatter on main. Apply `golangci-lint fmt`; formatting only, no
logic change. Unblocks this PR's lint/build-test jobs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(settings): manual Check for updates + Update button
Auto-update only checked at launch, so users with auto-updates off (or who
just want it now) had no way to update from the app. Add an on-demand flow:
checkForUpdatesNow/downloadUpdateNow/quitAndInstallUpdate in auto-updater
(works regardless of the opt-in; not-packaged dev surfaces 'unsupported'),
forwarded to the renderer as a live UpdateStatus over updates:status. The
Updates section gains a Check for updates button and an Update button that
downloads then installs (Restart & install), with checking/available/
downloading%/downloaded/error status. Refs #2207.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: format with prettier [skip ci]
* fix(sidebar): always offer Global settings in the footer menu
The footer Settings menu showed either Project settings (with a project
selected) or Global settings (with none), so global settings was
unreachable while inside a project. Always list Global settings; add
Project settings above it when a project is active. Refs #2205.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(settings): show current app version in Updates section
Surface app.getVersion() as a 'Current version vX.Y.Z' line above the
update controls so users can see what they're running. Refs #2207.
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>
waitForStopped removed the run-file and then additionally waited for the
daemon process to fully exit, erroring with "removed run-file but did not
exit within 10s" if it lingered past the stop timeout. The run-file is the
daemon's own liveness marker: once it is gone the daemon has committed to
stopping. When no desktop client is connected the daemon can drain its
background workers slower than the stop timeout, which made ao stop
spuriously report failure (the TestE2E_Lifecycle failure in #2214).
Treat run-file removal as stopped: keep polling for full process exit as a
best effort (so Windows releases inherited handles before callers clean the
data dir) but no longer error when that grace elapses.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* docs: design for dashboard legacy-migration popup + app-state marker
Spec for the app-side migration trigger (Approach A): projects-only import
daemon API + a migration marker in ~/.ao/app-state.json, with a launch-time
popup (Proceed / Skip / Don't Migrate). Settings redo path deferred to #2205.
Includes the projects-only import-offer backend plan it consumes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: implementation plan for legacy-migration popup + marker
Part A reuses the projects-only import API (import-offer plan) with an
availability-only Status; Part B adds the app-state migration marker (schema v2),
IPC, the useMigrationOffer gate, and the MigrationPopup (Proceed/Skip/Don't
Migrate). Settings redo path deferred to #2205.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(legacyimport): scope import to projects + settings only
Remove orchestrator/transcript import code (orchestrator.go, claude.go,
session_import_store.go and their tests). Trim Store, Options, Report to
projects-only fields. Drop defaultClaudeProjectsDir and projectSessionsDir
from paths.go. Add yaml.TypeError robustness in config.go. Update cli/import.go
confirm prompt and summary. Update importer_test.go to projects-only assertions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(importer): availability probe + projects-only run
Create service/importer.Manager with Status (physical availability check only,
no DB heuristic) and Run (delegates to legacyimport.Run). The app-state.json
marker governs whether to prompt; this service only answers whether legacy data
is physically present.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(httpd): import controller + DTOs (GET/POST /api/v1/import)
Add ImportStatusResponse/ImportRunResponse DTOs to dto.go. Create
ImportController with GET (status probe) and POST (run) handlers, both
returning 501 when Svc is nil. Wire APIDeps.Import + API.imports in api.go.
Add controller tests (status, status error, run, run error).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(apispec): describe /api/v1/import; regenerate openapi + schema.ts
Add import tag, importOperations() (GET + POST /api/v1/import), and schema
name mappings (ImportStatusResponse, ImportRunResponse, ImportReport) to
build.go. Regenerate openapi.yaml and frontend/src/api/schema.ts. Route-spec
parity test passes. Restore the nil-svc-501 import controller test now that
the spec includes the operation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(daemon): mount import service on the API
Wire importsvc.New(importsvc.Deps{Store: store}) into APIDeps.Import in
daemon.go so the daemon serves GET/POST /api/v1/import backed by the live
sqlite store. Projects-only; no DataDir.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore(cli): drop resolved §6.4 first-boot-import TODO
The legacy import is now a daemon API (GET/POST /api/v1/import) served by
the importer service and the desktop app handles the popup prompt via the
app-state.json migration marker. The TODO comment is resolved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore(legacyimport): remove dead isDir helper
isDir was only used by the deleted projectSessionsDir function.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: update package-lock.json after npm install for api:ts
openapi-typescript was missing from root node_modules; npm install
populated it so npm run api:ts could regenerate schema.ts.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(httpd): drop em-dash comment + unused ImportStatusResult alias
Review findings M1/M2 from G1 task review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(app-state): migration marker (schema v2) + updateMigration
Bump SCHEMA_VERSION to 2, add MigrationStatus/MigrationState types and
migration? field to AppStateMarker. Extract atomicWriteMarker helper and
reuse it. writeAppStateMarker now preserves an existing migration block
across launch writes. Add updateMigration (IPC setter) and readMigrationState
(IPC getter) exports. TDD: tests added first (red), then implementation (green).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ipc): expose app-state migration getter/setter to the renderer
Add appState:getMigration / appState:setMigration IPC handlers in main.ts.
Add ao.appState.getMigration / setMigration to preload.ts (typed via AoBridge).
Add appState preview fallback in bridge.ts and test setup so AoBridge stays
satisfied in both browser preview and test environments.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(app-state): cover corrupt-marker case + clean up temp dirs
G2 review findings I2 (corrupt-JSON branch untested) and m1 (temp dirs not cleaned).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(renderer): useMigrationOffer gate (marker + availability)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(renderer): MigrationPopup (Proceed / Skip / Don't Migrate)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(renderer): surface MigrationPopup on the dashboard
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>
* feat(start): implement ao start fetch/open for Windows and Linux
Fill in the non-darwin branches of the bootstrapper (T6/T7):
- assetName() selects the per-GOOS stable release asset: windows ->
agent-orchestrator-win32-x64.exe (NSIS installer), linux ->
agent-orchestrator-linux-x64.AppImage. amd64-only for now via
requireAMD64(), which returns a clear unsupported-arch error.
- fetchApp() dispatches per GOOS. Windows downloads the NSIS installer and
runs it silently (/S) to the default per-user dir, then resolves the
installed exe under %LOCALAPPDATA%\Programs. Linux downloads the AppImage
to a stable path under ~/.ao (atomic temp+rename), chmods it executable,
and skips any install step so re-runs resolve without re-fetching.
- knownAppLocations() scans the per-user and per-machine Windows install
dirs and the stable Linux AppImage path.
- isUsableBundle() treats a win exe / linux AppImage as a regular file
(darwin stays a directory).
- openApp() launches win/linux detached via the existing StartProcess seam,
forwarding --installed-via=npm-bootstrap, and falls back to manual-open on
spawn failure.
The Windows silent-install path is marked ponytail (untestable on the macOS
build host); a wrong install dir surfaces as a clear not-found error. Tests
cover asset naming, arch gating, scan locations, regular-file vs dir, and
the detached-spawn + fallback paths.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(release): build + publish a Linux AppImage for ao start (T7)
AppImage is the Linux fetch-and-run artifact for the bootstrapper (spec
§11.3): a single self-contained executable ao start downloads and runs
directly, no system package manager.
- makers/maker-appimage.ts: a MakerBase subclass bridging to
electron-builder's buildForge (appImage target), mirroring maker-nsis.ts,
since Forge has no first-party AppImage maker. publish:null so Forge owns
release uploads.
- forge.config.ts: register MakerAppImage for linux; keep deb/rpm for users
who want a system package.
- frontend-release.yml: on ubuntu-latest, copy the built AppImage to the
stable, space-free name agent-orchestrator-linux-x64.AppImage and upload
it to the v<version> release with --clobber, mirroring the Windows step.
Build-untested on this macOS host: the first ubuntu CI run must confirm the
electron-builder AppImage target token and the out/make/*.AppImage output
glob.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ci): green up ao start Win/Linux PR (lint, cross-OS test, container smoke)
Five golangci-lint findings in start.go, two cross-OS test failures, and the
fresh-install container smoke were broken by the Win/Linux bootstrapper diff.
Go lint (start.go):
- gocritic filepathJoin: build "/Applications/<bundle>" as a literal instead of
filepath.Join with a separator-bearing arg.
- gocritic httpNoBody: pass http.NoBody, not nil, to NewRequestWithContext.
- gosec G302: annotate the AppImage chmod 0755 with a nolint; an AppImage is a
self-contained executable and must be executable.
- nilerr: annotate openApp's intentional (false, nil) on launch failure; the
failure is reported via the bool, not as an error.
- unparam: resolveApp's error result was always nil; drop it and update callers.
Cross-OS tests (start_test.go):
- makeBundle created a directory, which only stats as usable on macOS. Make it a
regular file on Windows/Linux so the marker/scan resolve tests pass there,
matching isUsableBundle's per-OS rule.
Container fresh-install smoke (test/cli/install-check.sh, Dockerfile):
- ao start is now the desktop-app launcher and no longer runs a daemon, so the
old daemon status/shutdown/stop assertions could never pass. Assert instead
that on a fresh box start reaches the fetch path and exits non-zero with a
clear error (404 download on amd64, unsupported-arch on arm64). Refresh the
stale daemon-reaping comments in the Dockerfile.
Verified locally: go build/vet ok, golangci-lint v2.12.2 reports 0 issues,
go test -tags e2e ./internal/cli/... passes (the only remaining failure,
TestE2E_Lifecycle, is a pre-existing daemon-shutdown flake that fails the same
way on upstream/main on this host), and the container smoke passes on both
linux/arm64 and linux/amd64.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* docs: grounded spec for ao start bootstrapper + npm deprecation
Real-codebase-verified implementation spec (Track A: launcher + app-state
marker + release asset wiring). Replaces the somthing.md draft's aspirational
assumptions with file:line ground truth: correct bundle name (Agent
Orchestrator.app), publish repo (aoagents/agent-orchestrator), draft-release
+ asset-rename gaps, unsigned-build reality, and the already-wired
update-electron-app updater. Includes a dependency-ordered task breakdown
for AO execution.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: reframe spec around Go ao start subcommand (not a JS launcher)
The npm package ships the existing Go cobra CLI (backend/cmd/ao); this effort
rewrites the `ao start` subcommand to fetch+open the desktop app. Corrections:
- releases land on AgentWrapper/agent-orchestrator (aoagents was the temporary
rewrite home; forge publisher must be repointed)
- ao start stops starting the daemon; the frontend owns the daemon
- adds the real Go CLI command surface (1.7), the npm-delivery gap for the Go
binary (1.6), and the legacy first-boot import decision (6.4)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: separate prod vs test release targets in ao start spec
Dev/test loop must never cut a production release. Download repo + forge
publisher + npm scope are now build-time overridable:
- prod: AgentWrapper/agent-orchestrator + real package name
- test: harshitsinghbhandari/agent-orchestrator (fork) + @theharshitsingh/ao
T3/T5 now release+test against the fork and the test scope, with prod cut as a
separate gated step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(cli): rewrite ao start to fetch + open the desktop app (T1)
ao start no longer starts the daemon (the desktop app owns it). It now
resolves the installed app (~/.ao/app-state.json marker -> stat -> known-
location scan), fetches the latest release zip and ditto-unpacks it on macOS
when absent, opens it with --installed-via=npm-bootstrap, and prints an honest
deprecation notice. releaseRepo is build-time overridable (-ldflags) so test
builds fetch from the fork. Windows/Linux fetch/open are stubbed for T6/T7.
Review fixes folded in: download() copies deps.HTTPClient and drops its 2s
loopback-probe timeout (a real release asset is hundreds of MB), and fetchApp
clears ~/.ao/staging before unpacking. Regression test covers the timeout.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(desktop): write ~/.ao/app-state.json marker on launch (T4)
The app is the sole writer of the marker ao start reads to locate the bundle.
New frontend/src/main/app-state.ts does an atomic temp+rename write mirroring
the daemon's runfile.Write, preserving installedAt/installSource across launches
and refreshing appPath/version/lastReconciledAt. main.ts hooks it into
app.whenReady ordered relocate (macOS) -> write marker -> createWindow, both
non-fatal. Bundle path is derived from process.execPath (not app.getAppPath,
which is the asar path); JSON keys match start.go's appState reader exactly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* ci(desktop-release): overridable repo, published release, stable asset aliases (T3)
Make the bootstrapper's constant releases/latest/download URL resolve:
- forge publisher repo reads AO_RELEASE_REPO (default AgentWrapper/agent-
orchestrator); the workflow sets it to github.repository so a fork run
publishes to the fork and never to prod
- draft:false so the release is immediately live (constant URL needs it)
- add ubuntu-latest to the matrix (issue #2191)
- post-publish steps upload stable space-free aliases
(agent-orchestrator-darwin-arm64.zip, agent-orchestrator-win32-x64.exe)
matching exactly what start.go fetches
Review fix: the alias upload targeted GITHUB_REF_NAME (the git tag), but
publisher-github creates the release as v<package.json version>; retargeted to
that and relaxed the guard so workflow_dispatch also produces aliases.
Known gaps (documented inline): macOS x64 needs an Intel runner (macos-latest is
arm64-only); the Linux stable asset name awaits the deb/rpm-vs-AppImage decision.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: format with prettier [skip ci]
* feat(npm): deliver Go ao binary as @aoagents/ao via per-platform packages (T2)
Ship the ao CLI to npm with ZERO install scripts using the esbuild model:
- packages/ao: pure-JS @aoagents/ao with a bin/ao.js shim that resolves and
execs the matching @aoagents/ao-<platform>-<arch> optionalDependency
- four platform packages (darwin-arm64/x64, win32-x64, linux-x64), each os/cpu
gated so npm installs only the host's; binary cross-compiled CGO-free
(modernc.org/sqlite), gitignored, shipped via files
- build-binaries.sh cross-compiles all four; releaseRepo keeps its prod default
Not added as root workspaces on purpose: os/cpu-restricted members make root
`npm ci` fail EBADPLATFORM (CI's api-drift job runs it). Packages publish
standalone; the shim was verified in a published-like layout.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore(npm): set @aoagents/ao packages to 0.10.0 (above existing 0.9.5)
The existing @aoagents/ao on npm is 0.9.5; the launcher must publish a higher
version so npm latest advances and existing users get the new fetch-and-open
binary on update. All five packages plus the four optionalDeps pins set to
0.10.0.
* chore: format with prettier [skip ci]
* fix(start): capture install provenance before macOS relocation
moveToApplicationsFolder() relaunches the app from /Applications without
forwarding the --installed-via arg, and code past a successful move never
runs in the staging instance. The post-move instance therefore wrote
installSource="unknown", and writeAppStateMarker's sticky logic then locked
it there, losing the npm-bootstrap provenance in the exact path it exists for.
Write the marker before relocation when --installed-via is present so the
source is persisted while the arg is still available; the post-move launch
preserves it (sticky installSource) while refreshing appPath to /Applications.
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>
* fix(daemon): do not tear down live sessions on shutdown; adopt them on boot
Remove SaveAndTeardownAll from the graceful shutdown path. Live tmux/ConPTY
sessions survive daemon exit; Reconcile on the next boot adopts them via
reconcileLive, preserving session IDs and preventing the id-increment bug.
Add runShutdownSessionLifecycle as a testable seam and narrow the
sessionLifecycle interface to Reconcile+RestoreAll only.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(daemon): make shutdown-teardown regression test falsifiable
Re-add SaveAndTeardownAll to the sessionLifecycle interface and to
fakeSessionLifecycle so TestShutdown_DoesNotCallSaveAndTeardownAll is
genuinely falsifiable: the flag flips if runShutdownSessionLifecycle
ever calls sl.SaveAndTeardownAll, making the assertion meaningful.
Name the sl parameter (was discarded with _) so the seam is visible.
RED: with a temporary sl.SaveAndTeardownAll call, test fails.
GREEN: without it, test passes. go test ./internal/daemon/... -race: 23/23.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(daemon): guard shutdown-teardown at compile time via narrowed interface
Remove SaveAndTeardownAll from sessionLifecycle so daemon.Run physically
cannot call teardown on shutdown. Delete the no-op runShutdownSessionLifecycle
seam and its test. The narrowed interface is the guard: re-introducing teardown
requires a visible, reviewable interface change.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(session): make ensure-orchestrator idempotent so POST cannot mint a duplicate
When clean=false, SpawnOrchestrator now checks for an existing active orchestrator
and returns it directly instead of always calling Spawn. Adds RED/GREEN tests.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(session): drop redundant NoCleanSkipsKills, covered by SpawnsWhenNoneExists
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(daemon): supervisor watchdog
Adds backend/internal/daemon/supervisor: a transport-agnostic watchdog
that fires onLastClientGone() exactly once when the live connection count
drops to zero and stays zero for a configurable grace period. Arms only
after the first accepted connection (headless safety: CLI ao start never
self-stops). Reconnect before grace elapses cancels the pending timer.
Mutex guards liveCount/armed/pendingTimer; sync.Once guards the callback.
Tests use net.Pipe + a fake listener (no OS sockets); all three behavior
contracts verified with -race: never fires pre-connect, fires once on
last disconnect, reconnect within grace cancels and re-arms correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(daemon): supervisor watchdog review fixes (data race, ErrClosed, leak, cleanup)
- Fix data race: capture liveCount into local inside lock before logging
- Replace io.EOF with errors.Is(err, net.ErrClosed) for correct production behavior; update fakeListener to match
- Fix goroutine leak: derive cancellable child context in Serve so watcher always unblocks on return
- Simplify makePipe: drop dead error return, update 3 call sites
- Remove dead defensive pendingTimer nil-check in armGrace
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(daemon): OS-native supervisor listener triggers clean shutdown
Creates platform-split Listen() in supervisor package (Unix UDS sibling
of run-file; Windows named pipe via go-winio). Wires it into daemon.Run
before srv.Run: listener failure is non-fatal so headless ao-start works.
Adds RequestShutdown() on Server and three unit tests for listen_unix.go.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(daemon): log supervisor Serve error instead of discarding it
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(desktop): supervisor link; daemon self-stops (clean) on frontend exit
Add connectSupervisor() in frontend/src/main/supervisor-link.ts: holds one
client connection to the daemon's OS-native supervisor socket (Unix domain
socket on macOS/Linux, named pipe on Windows). Retries with bounded backoff
until the daemon is up; reconnects automatically on drop.
In main.ts, connect the link from reportBoundPort (once per daemon ready
transition). Remove the quit-time daemon teardown: the before-quit handler
now only disposes the browser view. The process.on("exit") killDaemon call
is also removed. When Electron exits for any reason the OS closes the fd,
the daemon detects EOF, and self-stops after its 5s grace period. tmux and
ConPTY sessions survive and are adopted on the next boot.
killDaemon and stopDaemon are kept for the explicit user-stop path
(ipcMain.handle("daemon:stop", ...)).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(desktop): guard against daemon orphan when supervisor link is down; tidy test setup
Expose a live `connected` getter on SupervisorLinkHandle and add a
last-resort process.on("exit") kill that fires only when the link is
not actually connected (UDS never bound or addr was null), preventing
orphan daemons while keeping the OS-fd teardown path for the normal
case. Log a warning when addr is null so the skip is diagnosable.
Invert the setup.ts guard to the natural form, dropping the empty
if/else skeleton.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(core): restore promptless sessions in place (reboot recovery, no increment)
Drop the empty-prompt early return in restoreArgv that returned ErrNotResumable
when the adapter could not resume and no prompt was saved. Now control falls
through to GetLaunchCommand unconditionally: a saved prompt is replayed, an
empty prompt (orchestrator) launches fresh with the system prompt only, same id,
same workspace. Removes the only producer of ErrNotResumable and deletes the
now-dead sentinel, its service mapping, and the corresponding tests.
Frontend reference to SESSION_NOT_RESUMABLE in TerminalPane.tsx is left intact
(the handler becomes harmlessly dead; the API simply stops returning that code).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(integration): dead-live session is restored, not abandoned, after reconcile
Task 5 made promptless sessions relaunch fresh instead of hitting ErrNotResumable.
The reconcile crash-recovery path (documented in reconcileLive) terminates a
dead-live session then RestoreAll relaunches it on the same boot. This test
asserted the old promptless-stays-terminated artifact; update it to the intended
restored end state (live again, same id, one fresh runtime Create).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* style(session): replace em dashes in service.go messages and comment
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(desktop): scope supervisor link to spawn path; dispose link on explicit stop
Document that the liveness link is established only when the app spawned the
daemon: the attach path intentionally does not link, to keep an `ao start`
daemon persistent (headless safety). Also dispose the link on an explicit
daemon:stop so its reconnect loop does not retry a deliberately-stopped daemon.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: format with prettier [skip ci]
* docs: add daemon-lifecycle adopt-on-shutdown implementation plan
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: format with prettier [skip ci]
* fix(daemon): keep supervisor watchdog alive across transient Accept errors
Review of #2185: Serve previously returned on the first unexpected Accept error,
silently disabling the watchdog (the 'restart is caller's job' comment described
a contract the caller never fulfilled). Back off and keep accepting instead, so
a transient error (e.g. EMFILE) cannot leave the daemon unable to self-stop on
frontend death. Also drop the stale Task 3/4 planning comment.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(core): leave promptless workers terminated on restore (orchestrators still relaunch fresh)
A promptless, unresumable KindWorker session had no prompt to replay and
no native session id to resume from. Blank-relaunching it via GetLaunchCommand
would silently drop its task. restoreArgv now returns ErrNotResumable for this
case, gated on (meta.Prompt == "" && kind != domain.KindOrchestrator). Orchestrators
are promptless by design and continue to relaunch fresh with the system prompt only.
Re-introduces ErrNotResumable sentinel and its Conflict API mapping.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(desktop): re-link supervisor on attach for app-owned daemons (close lingering-daemon gap)
- runfile.Info gets Owner field (omitempty): "app" when Electron spawned,
empty for headless `ao start`. server.go reads AO_OWNER env to set it.
- daemonEnv() injects AO_OWNER=app so spawned daemons self-identify.
- Extracted establishSupervisorLink() from inline reportBoundPort code.
Spawn path calls it unconditionally; attach paths call it only when
shouldLinkOnAttach(owner) is true (owner === "app").
- Both attach paths (inspectExistingDaemon, resolveDaemonFromPort) now
read the owner from the run-file and re-link when app-owned, preventing
a lingering app-spawned daemon from self-stopping mid-session.
- Headless ao start daemons stay unlinked: persistent across app quit.
- New daemon-owner.ts + daemon-owner.test.ts (4 vitest cases, all pass).
- Go: 9 tests pass with -race; vet clean.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: quiet expected ErrNotResumable log in RestoreAll; note attach TOCTOU
Review polish on #2185: a promptless worker left terminated on boot is expected,
not an operational error, so log it at Warn not Error. Document the narrow
run-file re-read TOCTOU on the port-attach re-link path (worst case is linking a
headless daemon; the dispose-idempotency guard prevents any leak).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* style: gofmt the review-fix files (CI format/lint check)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* 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>
On Windows the daemon shells out to console-subsystem processes without
suppressing the console window. The reaper polls session liveness on a
timer (and the attach loop retries), each call runs a zellij command, and
every invocation pops a console window that instantly closes, so the user
sees rapid blinking.
- Add a platform-split hideWindow(cmd) helper to the zellij package
(CREATE_NO_WINDOW + HideWindow on Windows, no-op elsewhere) and call it
in execRunner.Run so list-sessions and friends stop flashing.
- startBackgroundProcess now uses CREATE_NO_WINDOW instead of
CREATE_NEW_CONSOLE (the latter creates a console then hides it, flashing).
- Pass windowsHide: true to the daemon spawn in main.ts so the daemon's own
console stays hidden on a Windows GUI launch.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(cli): add examples to ao preview --help (#387)
Update the preview command help to include a concise Examples section
covering the four common workflows:
ao preview
ao preview file:///home/aoagent/ReverbCode/index.html
ao preview http://localhost:5173
ao preview clear
Replace the workspace-relative path (./dist/index.html) in the Long
description with the absolute file:// URL pattern agents actually use.
Command syntax and behavior are unchanged.
Closes#387
* fix(cli): clarify no-arg preview fallback behavior in help text
Address review feedback on #388: the no-argument description now
reflects the daemon's actual behavior — autodetect the workspace
entry point, fall back to the session's existing preview target
when none exists.
---------
Co-authored-by: AO Bot <ao-bot@composio.dev>
Fixes the packaged desktop app getting permanently stuck on "AO daemon
is not ready" (#385) via two daemon-lifecycle fixes.
1. Port conflict no longer exits the daemon. When the configured port
(default 127.0.0.1:3001) is held by a non-AO process, NewWithDeps now
falls back to an OS-assigned ephemeral port instead of returning a bind
error. A genuine peer AO daemon is already ruled out upstream (the
running.json + /healthz check in daemon.Run), so a conflict here means a
foreign holder. The bound port is logged ("daemon listening") and written
to running.json, both of which the supervisor reads, so the fallback
propagates to the renderer with no UI changes.
2. Detached daemon is torn down on more exit paths. before-quit already
group-kills the daemon, but app.exit() and some shutdown routes skip it,
orphaning the daemon so it keeps holding the port for the next launch. A
synchronous process 'exit' handler now also signals the daemon's process
group. A hard SIGKILL/crash still can't run JS, but fix#1 covers the
orphan that leaves behind.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(preview): add clear, reuse defaults, force refresh, local files (#379)
`ao preview` had four issues that made the desktop browser panel awkward
during sessions. This addresses all four:
1. No way to clear the panel. Adds `ao preview clear` (DELETE
/sessions/{id}/preview) which empties the stored target; the panel
loads about:blank and returns to its empty state.
2. Bare `ao preview` always autodetected index.html. It now reuses the
session's existing preview target (so each agent/context keeps its own
default), falling back to index.html only when nothing was previewed.
3. Re-running `ao preview <same-url>` never refreshed. The preview_url
alone could not distinguish a real re-run from a CDC replay of an
unrelated session update. A new monotonic preview_revision (bumped on
every set, migration 0018, added to the sessions_cdc_update trigger)
gives the renderer a per-command identity to key navigation on, so a
re-run always re-navigates while unrelated updates are ignored.
4. Local files could not be previewed. `ao preview ./dist/index.html`
(and other workspace-relative paths) now resolve server-side to the
preview/files proxy URL when the file exists; non-file targets stay
verbatim.
Backend, CLI, and renderer all covered by tests; OpenAPI spec and the
frontend schema are regenerated for the new DELETE route and field.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(cdc): include previewRevision in sessions update event payload
The CDC trigger watched preview_revision changes but didn't include it
in the JSON payload, so the frontend couldn't detect same-URL preview
refreshes via SSE events. This broke the core purpose of the feature.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(migration): renumber to 0019 to resolve conflict with main
Main branch now has migration 0018 (review_run_delivered_at), causing
a duplicate version conflict when CI merges the PR branch with main.
Renamed 0018_add_session_preview_revision.sql to 0019.
Also fixed the Down migration to properly restore the CDC trigger state
after migration 0017 (with previewUrl but without previewRevision).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Vaibhaav <user@example.com>
* 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>
* fix(frontend): add terminal controls and reliable copy
* chore: format with prettier [skip ci]
* fix(frontend): preserve terminal mouse scrolling
* fix(backend): enable zellij wheel scrolling
* fix(frontend): forward xterm scroll input
* fix(frontend): restore terminal drag selection copy
* fix(frontend): make terminal wheel scroll zellij scrollback
zellij 0.44.x with mouse-mode true acts on SGR wheel reports written to
its stdin and scrolls the focused pane, but it does not enable host mouse
reporting. xterm therefore never reports the wheel itself (protocol stays
NONE) and, with scrollback:0, converts the wheel into cursor-arrow keys,
which move the agent's cursor/history instead of scrolling.
Synthesize SGR wheel reports from a custom wheel handler and send them
through the existing input pipe; accumulate pixel deltas into line counts
to match xterm's native scroll feel. Ctrl/Cmd wheel is left for the
font-size zoom handler. Drag-copy is unaffected (separate selection path).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: format with prettier [skip ci]
* fix(frontend): handle line/page wheel modes for cross-platform scroll
The wheel-to-SGR translation only divided pixel deltas by row height,
which is the deltaMode browsers emit for trackpads and normalized wheels
(macOS). Many Linux/Windows mouse wheels report whole lines (deltaMode 1)
or pages (deltaMode 2) with small deltaY, which truncated to zero lines
and never scrolled. Mirror xterm's getLinesScrolled across all three
modes so scroll works on every platform.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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>
* fix(review): message worker on changes_requested instead of relying on SCM poll (#337)
review.Engine.Submit previously only persisted the verdict/body and left the
worker to learn about requested changes via the SCM poll loop, which is gated on
GitHub's reviewDecision and never reaches CHANGES_REQUESTED for self-reviews or
COMMENT-state reviews. Submit now nudges the worker's live pane directly via
ports.AgentMessenger (the same mechanism lifecycle uses) whenever the verdict is
changes_requested.
Extended flow: the reviewer reads back the GitHub review id it posted and passes
it through `ao review submit --review-id`; the id is stored on the review_run row
(new column + migration 0016) and included in the worker message so the worker
knows exactly which review to address and reply to.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): mark worker nudge as AO internal review, ask to reply + resolve
Distinguish the AO internal review nudge from the external GitHub-reviewer
feedback the lifecycle SCM loop relays. For an AO review the worker is now asked,
once it has pushed its fix, to reply on the review referencing its id with what
it changed and resolve the inline review comment threads it addressed (the
reviewer posts inline comments, so the per-finding threads are resolvable via
resolveReviewThread; the top-level review object is not, hence the reply).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): generalise the changes-requested worker nudge wording
Drop the "not an external GitHub PR reviewer" aside and the assumption that the
worker pushes a fix — it may resolve the feedback without code changes. The nudge
now reads "Review the feedback below and address it" and asks the worker to reply
with how it addressed the review and resolve the threads it addressed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): harden the review-id read-back against array order and empty results
The reviewer read the just-posted review id with `--jq '.[-1].id'`, which trusts
the REST API to return reviews in ascending submission order and errors when no
review exists. Review ids are monotonic, so select the highest id instead and
emit nothing when the list is empty: `--jq 'map(.id) | max // empty'`. Update the
matching `--review-id` flag help.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): post the review via gh api and capture its id from that response
The reviewer must use `gh api --method POST .../reviews` to attach inline
comments anyway (`gh pr review` cannot), and that response already contains the
created review's id. Capture `.id` from that single call instead of a second
read-back, dropping the array-ordering/pagination heuristics entirely — the id is
the exact review just created.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): send the review as a JSON body so inline comments are a real array
gh api -f/-F cannot build an array of objects: comments[][path] is sent as a
literal key, so the inline comments are dropped — defeating the reason for using
gh api over gh pr review. Post the review via --input JSON instead, keeping the
.id capture and the approve/COMMENT fallback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: format with prettier [skip ci]
* chore(review): drop accidentally committed reviewer scratch, write review out of tree
review.md was the reviewer agent's own writeup, swept onto the worker branch by a
stray `git add -A` in 5df20c9. Remove it, gitignore `/review.md` as a backstop,
and change the reviewer prompt to write its review to a temp file outside the
checkout instead of into the worktree (where it could be committed onto the
worker's branch).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): message the worker before marking the run complete
If messenger.Send failed after UpdateReviewRunResult had already flipped the run
to complete, a retried `ao review submit` tripped the status='running' guard and
could never record the result. Send first; only mark the run complete once the
worker has been notified, so a failed send leaves the run retryable. A landed
message followed by a failed DB write degrades to one extra nudge on retry — the
same trade lifecycle's sendOnce makes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(review): accept the review body on stdin so the reviewer writes no file
`ao review submit --body -` now reads the review from stdin, and the reviewer
prompt pipes its writeup via a heredoc instead of writing a file. Previously the
reviewer wrote review.md into its checkout to pass as --body, which could be
committed onto the worker's branch (as it just was). A file path is still
accepted for backward compatibility.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): always post approvals as COMMENT, drop the APPROVE attempt
The reviewer posts from the PR author's own GitHub account, so event=APPROVE
always 422s. Drop it: request changes with REQUEST_CHANGES, approve with a
COMMENT-event review whose body states it is an approval.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): post every review as event=COMMENT (author can't APPROVE or REQUEST_CHANGES own PR)
The reviewer posts from the PR author's own account, where GitHub rejects both
APPROVE and REQUEST_CHANGES. Always post a COMMENT-event review and state the
verdict in the body; the machine-readable verdict still reaches AO via
`ao review submit --verdict`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(cli): accept underscore flag names on `ao review submit`
Reviewer agents routinely invoke the submit command with --review_id
instead of --review-id, which cobra rejected as an unknown flag and
dropped the GitHub review id from the worker notification. Normalize
underscores to hyphens on the command's flags so both spellings resolve
to the same flag.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: sanitize review id in worker notifications
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Vaibhaav <user@example.com>
* fix(session-manager): terminate sessions even when handle is missing
Kill hard-failed with ErrIncompleteHandle before recording terminal
intent, so a session that lost its runtime/workspace handle (crash,
partial spawn) was un-killable and stuck forever on the dashboard.
MarkTerminated now runs unconditionally; each destroy step is gated
individually on whether that handle exists. If nothing is present to
tear down, the session still terminates — freed=false just signals
nothing was freed. ErrIncompleteHandle is still returned by Restore
and runtimeMessenger.Send, where a missing handle is genuinely fatal.
Updates TestKill_RefusesIncompleteHandle → TestKill_TerminatesIncompleteHandle.
* style: gofmt blank line between test functions
---------
Co-authored-by: AO Bot <ao-bot@composio.dev>
* fix: recover terminal reattach after daemon idle
* chore: format with prettier [skip ci]
* fix: harden daemon start recovery
* fix: cancel stale daemon start attempts
* fix: quarantine untrusted daemon base url
* chore: format with prettier [skip ci]
* fix: close daemon status race windows
* chore: format with prettier [skip ci]
* fix: bootstrap daemon trust before shell load
* test: trust mocked API base in PR hydration
---------
Co-authored-by: Vaibhaav <user@example.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
The orchestrator/worker system prompts (role, coordination, branch
conventions) contained no instruction telling the agent to treat them as
private, so a plain "give me your system prompt" made Claude Code dump
the role block verbatim.
Add a systemPromptGuard appended to every non-empty system prompt via
buildSystemPrompt, covering both spawn and restore paths. The guard
covers direct, indirect, and embedded reveal requests while leaving
general project/workflow questions answerable.
Adds TestSystemPrompt_AppendsConfidentialityGuard across orchestrator
and both worker variants.
Co-authored-by: i-trytoohard <i-trytoohard@users.noreply.github.com>
* test(storage): guard against duplicate goose migration version prefixes
Statically scans embedded migration filenames for repeated numeric
version prefixes and fails with a clear message, catching the class
of bug from #333 (two PRs adding 0014_*.sql independently) before
goose.Up() would panic at runtime.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(storage): dedupe migration versions by goose's parsed int64, not raw string
versionPrefix compared raw filename prefixes, so 014_x.sql and 0014_y.sql
were treated as distinct even though goose.NumericComponent parses both
to version 14 and panics on the collision. Use goose.NumericComponent
directly so the test enforces goose's actual collision rule.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(storage): trim PR-specific framing from migration version test comment
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(storage): renumber telemetry migration to 0015 to resolve goose version collision
* chore: format with prettier [skip ci]
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix(project): resolve default branch from origin/HEAD, not checked-out branch
Detecting the project default via `symbolic-ref --short HEAD` captured
whatever branch the repo happened to be on at add time. Adding a project
while on a feature branch (e.g. fix/pr-attachment) persisted that branch
as the default, so every session worktree based off it instead of main.
Prefer the remote default (origin/HEAD), falling back to the checked-out
branch only when no remote default is set. This still records a non-main
default like master correctly, while ignoring the active feature branch.
* test: cover branch-not-fetched API error
* 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>
* 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>
* feat(session): support multiple PRs per session
A session can now own several pull requests (a root plus stacked
children) instead of being capped at one. The SQLite schema was already
1-session->many-PR (pr.url PK, session_id a plain FK), so this is a
behavioural change across the observe -> persist -> derive -> react
pipeline, not a migration.
- observe: the SCM observer discovers every open PR whose source branch
matches a session branch or descends from it ("branch/..." stacking),
attributing each to the owning session; the longest matching branch
wins so a child session claims its own stacked PRs.
- derive: session status is a worst-wins aggregate over all owned PRs,
with a stack model (B is a child of A iff B.target == A.source and A is
open) exposed via prs[] on every session read DTO.
- react: per-PR reactions; a stacked child blocked by an open parent is
exempt from the rebase/merge-conflict nudge (only the bottom of the
stack is eligible), and the session completes only when no PR is open
and at least one merged.
- tests: unit coverage across stack/status/observer/lifecycle, a
real-SQLite ListPRFactsForSession test for the stacked-PR read path,
and a functional end-to-end integration test driving the real store +
lifecycle + observer through attribution, completion, and stacked-child
nudge suppression.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(scm): ignore fork heads in PR attribution and persist discovered siblings before completion
Branch-prefix attribution now requires a discovered PR's head branch to live
in the project repo. A fork PR can reuse a session's branch name while its
commits live in the fork, so the previous code could auto-claim foreign work.
Carry head repo full_name from the REST list response and skip any PR whose
head repo is not the base repo.
discoverNewPRs also writes each newly discovered PR as an open baseline row
before the refresh/lifecycle pass runs. A session can own several PRs, and a
terminal observation triggers a completion check that reads all of the
session's PRs from the store. Without the early write, an open sibling found
in the same poll was not yet durable and the session could terminate while
that PR was still open.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(session): surface actionable signals from blocked stacked children; clarify worker prompt
Status aggregation previously dropped any open PR blocked by an open parent,
hiding actionable child signals (failing CI, draft, requested changes,
unresolved comments) behind the parent's status. A blocked child still cannot
merge, so its readiness signals (mergeable/approved/review-pending/open) stay
suppressed, but its problem signals now contribute to the worst-wins aggregate.
The all-blocked fallback is preserved so a session never goes dark.
The worker multi-PR prompt said independent PRs could branch off the base
branch as usual, which conflicts with branch-prefix attribution. Clarify that
a PR may target the base branch, but its source branch must stay under the
session branch namespace for AO to track it.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(review): make the reviewer post to GitHub and record its verdict autonomously
The claude-code reviewer never completed a review on its own. Three defects
in the reviewer launch + flow:
- It launched with no permission mode, so a headless pane stalled on the
first tool-permission prompt and never ran gh/ao. Launch with
bypassPermissions (read-only is enforced by the prompt, not a sandbox).
- The reviewer pane got no pinned PATH, so `ao review submit` resolved to a
foreign `ao` on the inherited PATH and failed. Pin PATH to the daemon's
own dir the same way worker sessions do — export HookPATH and reuse it in
the launcher.
- The prompt did not enforce ordering. Make it post the review on the PR
via gh first, then run `ao review submit`.
Fixes#258
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): fall back to a comment review when self-approval is rejected
GitHub does not let an author approve their own PR, so a reviewer running
under the same account can't post an `approve`. Tell the reviewer to post
the approval as a regular comment review (COMMENT event stating it is an
approval) when the provider rejects the self-approval, instead of failing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(review): serialize concurrent triggers per worker to stop double-spawn
Engine.Trigger was a read-then-write (idempotency check -> reviewer spawn ->
InsertReviewRun) with no serialization and no backing constraint. Two near-
simultaneous triggers for the same worker at the same head SHA both passed the
GetReviewRunBySessionAndSHA check, both spawned a reviewer against the same
deterministic review-<id> handle, and both inserted a running run for one commit.
Add a per-worker keyed mutex (lockWorker) held across the whole Trigger body, so
the loser re-reads the freshly-recorded run and short-circuits to Created:false
instead of spawning. Back it with a partial unique index on
review_run(session_id, target_sha) (migration 0013) as a cross-restart safety
net; rows with an empty target_sha (head not yet observed) are excluded so they
are not blocked.
Adds a concurrency test asserting N simultaneous triggers spawn once and record
one run.
Closes#242
* fix(review): make migration 0013 dedup-safe and handle the unique conflict in Trigger
Pre-#242 daemons can already hold duplicate (session_id, target_sha)
review_run rows, on which CREATE UNIQUE INDEX fails and wedges startup.
Migration 0013 now collapses each duplicate group to a single survivor
(a completed pass over a still-running one, then newest by created_at)
before building the index.
Trigger now treats a unique-constraint hit as a fallback rather than an
error: InsertReviewRun maps it to the new domain.ErrDuplicateReviewRun
sentinel, and Trigger re-reads GetReviewRunBySessionAndSHA and returns
that run with Created:false instead of surfacing a raw error after the
reviewer may already have launched.
Two clocks defaulted to local time.Now while the rest of the codebase writes
UTC, so ao session get showed created and updated in different timezones:
- session manager clock → spawn-stamped CreatedAt/UpdatedAt
- lifecycle manager clock → activity-driven LastActivityAt/UpdatedAt
A real spawn made this visible: createdAt came back UTC but updatedAt/
lastActivityAt were local once the agent reported activity. Default both clocks
to UTC.
Closes#214
Register the repo's actual checked-out branch as the project default so
session worktrees base off a ref that exists. Previously Config.DefaultBranch
was left empty and defaulted to "main", so a repo on master/develop/trunk
failed every spawn with BRANCH_NOT_FETCHED and had no CLI workaround.
Detection is best-effort (symbolic-ref --short HEAD); a detached HEAD or git
error falls back to the existing main default. Only persist when the branch
diverges from main, so the common main repo keeps a NULL config.
Closes#208
validateBranch returned an untyped error for a name rejected by
git check-ref-format, so toAPIError fell through to INTERNAL_ERROR 500.
Add a ports.ErrWorkspaceBranchInvalid sentinel (mirroring the not-fetched /
checked-out-elsewhere ones), wrap it in validateBranch, and map it to
INVALID_BRANCH (400). Completes the residual of #152 Bug 3, which typed the
not-fetched and checked-out-elsewhere cases but left the invalid-format case
collapsing to 500.
Closes#212
An unknown --harness was only caught at the agent-registry lookup deep in
Spawn, after the seed row and worktree were already created: the untyped
error collapsed to INTERNAL_ERROR 500 and left a terminated orphan row.
Validate the harness against the registry before any durable state is
created and return a typed ErrUnknownHarness mapped to UNKNOWN_HARNESS (400).
Sibling to #152 Bug 6 (unknown binary on PATH), which did not cover a harness
with no registered adapter.
Closes#210
* 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>
A spawn with no explicit harness ran the daemon default (claude-code) but
stored an empty harness: effectiveHarness returned "", seedRecord persisted it,
and the empty->default resolution lived only inside agentRegistry.Agent. The API
then omitted harness and the UI defaulted to "codex" — mislabelling a Claude
Code session.
Inject the daemon's default agent (AO_AGENT / config.DefaultAgent) into the
session manager and resolve an unspecified harness to it before the seed row is
written, so the stored/returned harness matches the agent that actually runs.
Closes#220
* Use ~/.ao as canonical state home
* chore: format with prettier [skip ci]
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Dushyant Singh Hada
swyam sharma
Adil Shaikh
neversettle
Harshit Singh Bhandari
Priyanshu Choudhary
i-trytoohard
Vaibhaav Tiwari
Laxman
Khushi Diwan
Madhav Kumar
Pritom Mazumdar