Commit Graph

7 Commits

Author SHA1 Message Date
Harshit Singh Bhandari ea54f31b24
fix(daemon): fall back to ephemeral port on conflict + harden teardown (#386)
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>
2026-06-22 22:05:14 +05:30
Vaibhaav Tiwari 708ec5db56
fix: recover terminal reattach after daemon idle (#354)
* 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>
2026-06-21 13:59:15 +05:30
neversettle 7880f59cf5
refactor(backend): LLD maintainability fixes in controllers/service layers (#95) (#96)
* refactor(backend): LLD maintainability fixes in controllers/service layers

Addresses the high + medium severity findings from the LLD review of
backend/internal/httpd and backend/internal/service (#95):

1. Controllers no longer import internal/session_manager. Session sentinel
   errors are now *domain.ServiceError values carrying their own HTTP mapping,
   so the controller translates them with one generic errors.As — no
   cross-package sentinel imports.
2. One error pattern across services: project.Error is now an alias of the
   shared domain.ServiceError, and session_manager sentinels use it too. A
   single writeServiceError replaces the per-resource error switches.
3. Clean-orchestrator business logic moved out of the controller into
   session.Service.SpawnOrchestrator(ctx, projectID, clean).
4. isGitRepo no longer treats case-different paths as equal on case-sensitive
   filesystems; case-insensitive compare is gated to darwin/windows via samePath.
5. Project repo check sits behind an injectable GitChecker, so the service is
   testable without a real git binary.
6. httpd exports only the production constructors (NewWithDeps,
   NewRouterWithControl); the 3 test-only wrappers are removed and the
   "router with empty deps" convenience moved to an unexported test helper.

Closes #95

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

* refactor(backend): standardize service errors on internal/httpd/errors

Replace the domain.ServiceError approach with a REST-API-scoped error package
and a single envelope renderer, per review feedback:

- Add internal/httpd/errors (package errors, aliased apierr): one structured
  Error type with semantic Kinds (Internal/Invalid/NotFound/Conflict) and
  constructors. Imports nothing, so any layer can depend on it.
- envelope.WriteError is now the single path from a service error to the wire
  APIError, and the only place a Kind becomes an HTTP status/word. The
  per-resource writeProjectError/writeSessionError translators are gone.
- Delete domain/errors.go (keeps domain pure of HTTP-flavored kinds) and
  service/project/errors.go (no per-service error files); services build
  errors inline via apierr constructors.
- session_manager sentinels are apierr.Error values (pointer identity still
  works with errors.Is).

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

* revert(backend): drop GitChecker seam and isGitRepo case-sensitivity change

Defer findings #4 (isGitRepo case-sensitivity) and #5 (GitChecker seam) out
of this PR. Restores the original exec-based isGitRepo and the New(store)
constructor; removes git.go, git_test.go, and the test-only export shims. The
error-standardization and other findings are unaffected.

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

* refactor(session): translate engine errors to API errors at the facade

The session_manager is the internal command engine and must not depend on the
REST API error vocabulary. Revert its sentinels to plain errors.New values and
move the engine→API translation into the service/session facade (toAPIError),
which is the correct boundary. Controllers still see apierr.Error and never
import the engine; the engine no longer imports internal/httpd/errors.

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

* docs(session): tighten error comments to state what the code does

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

* style(envelope): make KindInternal an explicit case in httpStatus

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

* refactor(apierr): rename package, test SpawnOrchestrator, parity fixes

Address review feedback on PR #96:
- Rename internal/httpd/errors → internal/httpd/apierr (package apierr) so
  importers no longer alias around the stdlib errors package.
- Add a commander seam to session.Service and unit-test the relocated
  clean-orchestrator rule: clean=true kills all active orchestrators before
  spawning; clean=false spawns without kills.
- project.Add: wrap the UpsertProject store error in apierr.Internal for parity
  with its sibling paths (was a raw 500).
- Document that KindInternal is iota's zero value, so a zero-value Error
  defaults to 500.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-03 18:09:02 +05:30
harshitsinghbhandari 0d8ffcd17a fix(httpd): update server_test for termMgr arg after rebase
The shutdown endpoint test was authored against the pre-rebase
httpd.New(cfg, log) signature. After rebasing onto main, the terminal
manager (from #50) made termMgr a required third arg. Pass nil — the
test exercises /shutdown, not /mux, so the terminal surface stays off.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-01 01:50:43 +05:30
Dhruv Sharma 2f4662b470 fix(cli): handle stale start and graceful shutdown 2026-06-01 01:45:59 +05:30
Pritom14 edcc631037 feat(httpd): mount terminal-streaming WebSocket at /mux
Add the /mux route: httpd performs the WebSocket upgrade (coder/websocket)
and adapts the connection to terminal.wsConn via wsjson, then hands it to
terminal.Manager.Serve. httpd owns only the upgrade and transport
adaptation; all stream logic stays in internal/terminal.

The route is mounted outside the per-request Timeout middleware (the
connection is long-lived) and is omitted entirely when no manager is wired,
so the daemon degrades to no terminal surface rather than failing. New/
NewRouter take the manager; main.go passes nil until commit 3 wires it.

mux_test.go drives the real upgrade + wsjson + Serve + creack/pty path with
a throwaway shell command, so it needs no tmux.
2026-05-31 19:23:35 +05:30
neversettle 59a654afea
feat(backend): HTTP daemon skeleton (Phase 1a) — #10 (#14)
* feat(backend): HTTP daemon skeleton — config, health, runfile, graceful shutdown (#10)

Phase 1a of the Go HTTP daemon lane (#10). Stands up the loopback-only
sidecar skeleton the later REST/SSE/WS/static surfaces build on:

- config: env-driven (AO_HOST/PORT/ENV/timeouts/run-file) with zero-config
  defaults; binds 127.0.0.1:3001; validates and fails fast on bad input.
- httpd: chi router with the recoverer → request-id → logger → real-ip
  middleware stack and /healthz + /readyz probes. Per-request timeout is
  carried in config but intentionally not global — it scopes to /api/v1 in
  Phase 1b so it never throttles SSE/WS/health.
- runfile: atomic PID + port handshake (running.json) for the Electron
  supervisor, with a dead-PID stale check so a crashed predecessor doesn't
  block startup while a live one fails fast.
- server: bind-before-publish (port conflict fails fast), graceful shutdown
  on SIGINT/SIGTERM via signal.NotifyContext with a 10s hard timeout, and
  run-file cleanup on exit.

Why: the daemon must be safely supervisable as a child process — the
supervisor needs a discoverable PID/port and the daemon must not leave a
half-started process or stale handshake behind. Locking the lifecycle down
now keeps the future port split a small change rather than a rewrite.

Tests cover config defaults/overrides/validation, run-file round-trip and
live/dead PID detection, health probes, full Run lifecycle, and port-conflict
fail-fast.

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

* refactor(backend): drop Env config field — not needed yet (#10)

Per review on #14: AO_ENV / Config.Env / IsProduction() weren't load-bearing
for Phase 1a — they only switched the slog handler. Removing them now keeps
the surface minimal; the env knob can come back later when a real consumer
needs it.

- config: remove Env field, AO_ENV parsing, and IsProduction helper.
- main: collapse newLogger to a single text-handler path.
- httpd: drop the env field from the listening log line.
- tests: drop the env assertions and AO_ENV fixture.

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

* docs: add backend run + config quick-start to README (#10)

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

* fix(backend): address Phase 1a review comments (#10)

- config: drop AO_HOST entirely — the daemon is loopback-only by design,
  so making the bind host env-configurable was a security footgun
- config: use net.JoinHostPort in Addr() so IPv6 literals stay valid
- config: reject zero/negative AO_REQUEST_TIMEOUT and AO_SHUTDOWN_TIMEOUT
  (time.ParseDuration accepts both; either would silently break the
  daemon — instant request expiry / no graceful drain)
- runfile: split processAlive into unix/windows build-tagged files so
  liveness detection is reliable on both platforms (Windows uses
  OpenProcess; POSIX keeps signal 0)
- runfile: document os.Rename overwrite semantics (atomic on POSIX,
  REPLACE_EXISTING on Windows) so the temp-then-rename pattern's
  cross-platform behaviour is explicit
- httpd tests: give probe/waitForHealth clients an explicit per-request
  timeout so a stalled connect can't hang the test on the outer deadline

* fix(backend): strip trailing blank line from runfile.go (#10)

gofmt CI was failing because removing the orphan processAlive doc
comment left an extra newline at EOF.

* fix(backend): cross-platform run-file replace + AO_HOST rationale (#10)

- runfile: introduce build-tagged atomicReplace — POSIX rename(2) on
  Unix, MoveFileEx with MOVEFILE_REPLACE_EXISTING on Windows. The Go
  runtime happens to do the Windows call internally already, but
  invoking it directly makes the cross-platform contract explicit
  instead of a runtime implementation detail
- runfile: tighten process_unix.go build tag from `!windows` to `unix`
  so plan9/js/wasm fail to build rather than silently using a broken
  signal-0 probe
- runfile: add TestWriteOverwritesExisting covering the stale run-file
  replace path that none of the previous tests exercised
- config: anchor the loopback-only decision in the LoopbackHost doc so
  the next contributor doesn't reintroduce AO_HOST without the security
  rationale

* fix(backend): route chi access logs through slog/stderr (#10)

chi's middleware.Logger writes via stdlib log to stdout, but the
daemon's slog logger writes to stderr — so REST traffic and daemon
logs landed on different streams in different formats. Replace it
with a small slog-backed requestLogger that:

- Wraps the response writer via middleware.NewWrapResponseWriter so
  status/bytes are accurate even when handlers return without an
  explicit WriteHeader.
- Reads the request id off the context set by middleware.RequestID
  (kept mounted just before this middleware so the id is available).
- Emits one structured Info line per request with method, path,
  status, bytes, duration, and remote — same key=value shape as the
  rest of the daemon, one stream for the Electron supervisor to
  capture.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-29 10:02:53 +05:30