Introduces backend/.golangci.yml (27 linters across correctness, dead-code/
boilerplate, style, and security), wires it into CI as a blocking job, and
fixes every finding so the tree starts at zero.
Config:
- 27 linters: errcheck, govet, staticcheck, errorlint, bodyclose,
sqlclosecheck, rowserrcheck, nilerr, makezero, unused, unparam, unconvert,
wastedassign, copyloopvar, prealloc, dupl, revive (incl. exported-symbol doc
comments), gocritic, misspell, usestdlibvars, predeclared, nakedret, gosec, …
- Tuned for signal over noise: govet/shadow and gocritic hugeParam/rangeValCopy/
unnamedResult disabled (idiomatic-Go false positives); sqlc-generated code and
tests get scoped exclusions; gosec G304 excluded (paths are config/run-file/
worktree-derived, not user input); nilerr excluded in cli/status.go (probe
failures are the reported status, not a command error).
CI:
- New blocking lint job (golangci-lint-action, latest binary for Go-version
compatibility).
- go-version now read from go.mod (was pinned 1.22 while go.mod declares 1.25).
Cleanup to reach zero (no behavior change):
- errcheck: wrap deferred/inline Close()/Remove()/Rollback() with `_ =`.
- gosec: tighten dir/file perms (0755->0750, 0644->0600).
- unparam: drop always-nil error return from startLifecycle; drop unused
shellPath param (zellij PowerShell) and always-500 fallbackStatus param
(writeProjectError).
- gocritic: regexp \d, s != "", switch->if, combined appends.
- revive: doc comments on all exported symbols; rename project.ProjectRow ->
project.Row (stutter); rename `max` locals shadowing the builtin.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Re-add the blank-identifier interface assertions lost when wiring.Adapter was
collapsed: *Store now directly satisfies ports.SessionStore and ports.PRWriter,
so prove it at the point of definition. Drift between either port and the
implementation now fails here instead of at the call sites in lifecycle_wiring
or tests.
Addresses greptile review comment on #60.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each PR-child table (pr / pr_checks / pr_comment) had three near-identical
structs — gen.* (generated), sqlite.*Row, and ports.* — with wiring.Adapter
copying field-by-field between them. Collapse to one shared definition per
table in domain (PRRow / PRCheckRow / PRComment), used by both the PRWriter
port and the sqlite store; gen.* stays sealed inside the storage layer.
- *sqlite.Store now satisfies ports.SessionStore + ports.PRWriter directly,
so the entire wiring.Adapter package is deleted (lifecycle.New(store, store)).
- The bool PR state <-> single state column, int<->int64, and enum-default
translation now lives only at the gen<->domain boundary in pr_store.go.
- WritePRObservation renamed WritePR to match the port; the integration test
and composition root drop their adapter copies.
Net -280 lines, behaviour unchanged. go test -race ./... green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replaces the growing bash smoke test with a Go os/exec suite behind the `e2e`
build tag (backend/internal/cli/e2e_test.go). It builds the real binary and
drives start/status/doctor/stop + the daemon-control HTTP surface against
isolated state (temp dir + OS-assigned free port), and now runs natively on
ubuntu + macOS + WINDOWS in CI — finally covering the Windows
CREATE_NEW_PROCESS_GROUP detach path and per-OS os.UserConfigDir resolution
that a Linux container can't observe. `go test -tags e2e -v` logs every command
and its output, replacing the bash -v flag.
- backend/internal/cli/e2e_test.go: 8 table-style TestE2E_* cases; strips any
inherited AO_* env so a real daemon's AO_PORT can't leak in.
- test/cli/install-check.sh: small, linear fresh-install proof the Dockerfile
runs (binary on PATH, no toolchain) — kept as the hardening tier.
- test/cli/Dockerfile: run install-check.sh instead of the full bash suite.
- .github/workflows/cli-e2e.yml: `native` is now a go test matrix over
ubuntu+macos+windows; `container` builds the image and runs it with --init.
- Removes test/cli/smoke.sh and test/cli/run-local.sh (superseded by `go test`).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
run-local.sh -v (or AO_SMOKE_VERBOSE=1) makes smoke.sh echo every command and
its complete output, indented, alongside the PASS/FAIL — for auditing exactly
what the suite runs and what the CLI returns. Default output is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes the one nit from the regression audit: the exit-code wiring was correct
and covered end-to-end by the smoke test, but not pinned by a unit test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stale-daemon assertion does not depend on container PID-1 reaping — it
writes a fabricated dead PID rather than killing a real process. --init is
still run so the real daemon spawned by the `start` test (detached via setsid)
is reaped after `stop` instead of lingering as a zombie. Reword the README,
Dockerfile, and workflow comments to say that accurately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a fresh-machine, install→use→verify E2E test for the `ao` CLI and wires
it into CI. The suite drives the real binary (start/status/doctor/stop + the
daemon-control HTTP surface) against fully isolated state — its own temp
run-file, data dir, and an auto-picked free loopback port — so it never
collides with a developer's real AO install or daemon.
- test/cli/smoke.sh: 40 assertions covering install resolution, version/help
(daemon hidden), doctor text+json (and that it does NOT migrate SQLite),
status stopped/stale/ready, start fresh+idempotent, daemon-created store,
/healthz identity, the /shutdown CSRF + DNS-rebinding guard (403 + survives),
graceful/stale/idempotent stop, run-file ownership cleanup, exit codes
(2 usage / 1 runtime), and completion for all four shells. It deliberately
ignores an inherited AO_PORT and self-allocates a free port for isolation.
- test/cli/Dockerfile: models installing ao on a fresh machine — builds the
binary, drops it on PATH in a clean Debian image with only runtime deps
(git/tmux/curl), runs the suite as a non-root user.
- test/cli/run-local.sh: build-from-source + native run convenience wrapper.
- .github/workflows/cli-e2e.yml: two tiers — `native` runs the suite on a
ubuntu+macos runner matrix (the real VMs, to cover the unix setsid detach and
macOS os.UserConfigDir paths a Linux container can't), and `container` runs
the fresh-machine Docker image with --init (real PID-1 reaper so the
stale-daemon assertion is reliable).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review findings on PR #53 (on top of the rebase onto main).
- doctor: stop opening/migrating SQLite. The daemon is the sole store
writer/migrator (architecture.md §7); the CLI must not run migrations or
open a second writer against a DB a live daemon owns. doctor now reports
database-file presence and gains --json.
- stop: only remove running.json when it still belongs to the PID we
stopped, so a concurrent `ao start` that wrote a new run-file is not
clobbered into looking stopped.
- httpd: gate POST /shutdown to loopback callers with no Origin header,
closing the CSRF / DNS-rebinding vector against an unauthenticated,
state-changing endpoint.
- start: detach the spawned daemon into its own session/process group so a
Ctrl-C while `ao start` waits for readiness doesn't also kill it.
- cli: exit 2 for usage errors (bad flag / arg count) vs 1 for runtime
failures.
- daemon: unexport newLogger (only used in-package).
- tests: /shutdown guard (cross-origin + rebinding) and stop run-file
ownership guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
A session can exit and run onExit (which deletes c.terms[id]) in the gap
between subscribe returning exited=false and openTerminal assigning
c.terms[id]. The delete is a no-op there since the key isn't set yet, so
the later assign resurrects a stale entry for a dead pane, trapping every
future open for that id on the connection. Re-apply the delete after the
assign when onExit fired in the window, tracked by a c.mu-guarded flag.
Add a stress regression test that races the exit against the assign.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The exit callback enqueued the exited frame before deleting c.terms[id],
so a client reopening on receipt of exited could hit the open guard while
the entry was still set and have its open dropped. Delete first so the
cleared entry is visible by the time the client sees exited.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Opening a terminal whose session has exited left c.terms[id] set to a
no-op (already-exited path) or to a never-cleared unsubscribe (exit after
open), so the open guard silently dropped every later open for that id on
the connection until close/reconnect. Clients also saw exited/data before
the opened ack.
Ack opened before subscribe so it always precedes replay/data/exited;
have subscribe report whether the pane was already terminal and skip
registering in that case; and clear the connection entry from the exit
callback for panes that exit after open.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The session run loop closes the PTY after copyOut returns, and session.close
(via Manager.Close) closes the same PTY again. creackPTY.Close called cmd.Wait
each time, and a second concurrent Wait on the same process blocks forever, so
daemon shutdown deadlocked whenever a terminal was still attached. fakePTY is
idempotent via sync.Once, so the unit suite never exercised this; a real tmux
attach surfaced it.
Guard close+kill+wait with a sync.Once so Wait runs exactly once. Add a
regression test that double-closes a real PTY under a watchdog.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* 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.
* feat(api): projects route shell (7 routes, REST-corrected) — #20
Mounts the /api/v1 surface on the skeleton router (#10·1a) and registers
the 7 canonical project routes as 501 stubs that emit a structured
PlannedRoute body documenting the future contract. Shared scaffolding
landed here (api.go, errors.go, stubs/, controllers/) so #21/#22 plug in
without re-touching the wiring.
WHY: opens the route-shell PRs in the Go HTTP daemon lane. Doing it
interface-first lets the dashboard team build against the contract
before any handler logic exists; the locked APIError envelope and
PlannedRoute shape become #19's OpenAPI source-of-truth.
REST audit corrections vs the legacy TS surface:
R3 PUT /projects/:id alias of PATCH: PUT not registered → 405.
R4 POST /projects/:id repair overload: canonical /repair; legacy 405.
R5 degraded GET returns 200 with error field: discriminator status.
R6 ok/success flag flips: drop on 2xx; return affected resource.
R9 bare {error: msg}: locked {error,code,message,requestId,details?}.
Legacy paths are deliberately NOT registered; each canonical handler
carries PlannedRoute.Legacy so consumers can discover the migration.
Zod schemas (TrackerConfig, SCMConfig, AgentConfig, ReactionConfig,
LocalProjectConfig, RoleAgentConfig) ported to typed Go structs with an
Extra map reserved for .passthrough() round-tripping in later PRs.
Closes part of #18; targets feat/issue-10 until #14 merges.
* refactor(api): collapse ProjectService → ProjectManager — #20
Controllers now depend on ONE inbound interface per resource — ports.ProjectManager —
mirroring the existing ports.SessionManager + LifecycleManager pattern.
Whether the manager impl reaches into the registry, the LCM, an outbound
port, or all three is its own concern; the HTTP layer no longer has to
know any of that.
WHY: the original split named the boundary type "ProjectService" and put
it in a sibling services.go. That implied a second category of port
distinct from inbound.go's *Manager interfaces, even though they play
the same role (things HTTP/CLI call into the core). Per review feedback,
collapse them onto one Manager-per-resource pattern.
Mechanical changes:
- ports/inbound.go gains ProjectManager next to SessionManager.
- ports/services.go renamed to projects.go; keeps only the DTOs the
ProjectManager methods take/return.
- ProjectsController.Svc renamed to Mgr; APIDeps.Projects type bumped
to ports.ProjectManager.
All tests pass unchanged; no behavioural change.
* refactor(api): replace stubs/ with OpenAPI-as-source-of-truth — #20
The first cut of the route shell duplicated each route's contract twice:
once as a Go literal (stubs.PlannedRoute{...}) in the controller, and
implicitly in the PR description. The Go literal was ~230 LoC of pure
throwaway that would be deleted in handler-impl PRs.
This commit eliminates the duplication:
- backend/internal/httpd/apispec/openapi.yaml: full OpenAPI 3.1 doc
covering the 7 project routes + shared schemas (Project, APIError,
config types). x-replaces records the legacy → canonical mapping
REST-audit corrections produced.
- apispec/apispec.go: //go:embed the YAML, expose Operation(method,
path) → the spec slice as a map, NotImplemented(w, r, method, path)
→ 501 with that slice embedded as `spec`.
- controllers/projects.go: each of 7 handlers is now a one-liner:
apispec.NotImplemented(w, r, "GET", "/api/v1/projects").
- /api/v1/openapi.yaml serves the embedded document so tooling
(SDK gen, the validator slated for #19, dashboard dev tools) can
fetch the whole spec from the same origin as the routes.
- stubs/ package deleted.
When a real handler lands, only the apispec.NotImplemented line goes
away — nothing else does. The spec stays as documentation; consumers
never had to know it was throwaway. #19 (OpenAPI follow-up) is now
half-folded into this PR; the validation middleware remains its own
follow-up.
Tests reshaped: assert envelope + spec.operationId + spec.x-replaces
(replaces the old planned.legacy assertion); add TestOpenAPIYAMLServed
to cover the static spec serve; add apispec_test.go for embed/lookup
behaviour.
* refactor(api): move projects contract to internal/project package — #20
Pilots the feature-package layout the backend is migrating toward: a
resource's inbound interface and its DTOs live with the resource, not in
a central ports/ catch-all.
WHY: review flagged ports/ as vague. It conflates three jobs — the
outbound capability seam (legit), single-impl inbound interfaces (Go
idiom wants these consumer-side), and DTOs that aren't ports at all.
This moves the projects contract out as the reference shape #21/#22
follow; the merged session/lifecycle/outbound contracts are left
untouched and migrated separately.
Scope: INTERFACE ONLY. No implementation — handlers still answer via
apispec.NotImplemented and the injected project.Manager stays nil. The
impl lands in a later handler-impl PR.
Changes:
- new internal/project: project.go (Manager interface, 7 endpoints) +
dto.go (AddInput/GetResult/UpdateConfigInput/RemoveResult/ReloadResult,
moved verbatim from ports/projects.go, Project-prefix dropped).
- ports/projects.go deleted; ProjectManager removed from ports/inbound.go.
outbound.go and facts.go untouched.
- controllers/projects.go and httpd/api.go depend on project.Manager.
Domain entities (Project, ProjectSummary, DegradedProject, config types)
stay in domain/ as shared vocabulary.
go build/vet/test/gofmt all clean; no behavioural change.
* refactor(api): consolidate project types into internal/project — #20
Addresses PR review: (1) "why are config_types required at the moment?"
and (2) "project objects already defined in project/ — how do we
differentiate?"
Both had the same root cause: project types were split across domain/
and project/. Fix — keep ALL project types in the project package; only
domain.ProjectID (shared with sessions/lifecycle/workspace) stays in
domain.
- domain/project.go → project/types.go: Project, Summary, Degraded
(renamed from ProjectSummary/DegradedProject; the package name carries
the "Project" prefix now).
- domain/config_types.go deleted. Kept only the 4 shapes the projects
API actually exposes — TrackerConfig, SCMConfig, SCMWebhookConfig,
ReactionConfig — moved into project/types.go. Dropped AgentConfig,
AgentPermission, RoleAgentConfig, LocalProjectConfig (zero references)
and the speculative `Extra map[string]any` passthrough fields (no
marshaller existed, so they silently dropped data — premature).
- project/dto.go + project/project.go reference the local types; ids
stay domain.ProjectID.
Net: one home for project types, no dead code. go build/vet/test/gofmt
clean; no behavioural change (handlers still 501 via apispec).
* feat(api): implement project routes with mock manager/store
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* merge: resolve conflicts with origin/main
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* refactor(httpd): share JSON/API error envelope helpers
* fix(api): align project mock store with sqlite schema
* fix(api): address project API review semantics
* canonicalize both paths with filepath.EvalSymlinks before comparing
* style(project): gofmt git repo validation
---------
Co-authored-by: Aditi Chauhan <aditi1178@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Vaibhaav <user@example.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
If startSession returned an error, run() returned immediately and the
reaper + cdc poller goroutines kept running while defer store.Close()
fired — a data race against the SQLite handle. Mirror the bottom-of-run
shutdown sequence on the error path (cancel ctx, drain reaper, drain
poller) so both goroutines have exited before the store is closed. The
explicit-not-defer ordering is the same the existing
post-srv.Run shutdown block uses; piling on more defers would hit the
LIFO trap the same comment already warns about.
Reported by Greptile on PR #52.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Renames the unused context.Context parameter from `_` to `ctx` so the
parameter name is already in place when a future plugin constructor
needs to honor cancellation (tmux/gitworktree are synchronous today).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Constructs a live *session.Manager in main alongside the LCM, sharing the
exact same SessionStore + LCM dependencies the lifecycle stack already
holds.
Refactor: storeAdapter moves from package main to a new internal
package, wiring.Adapter, so the daemon's composition root and any
in-process integration tests can share a single bridge.
Stubbed for now: ports.Agent has no production adapter on main; a loud
*noopAgent returns sentinel AO_AGENT_HARNESS_NOT_WIRED and logs a
warning once on first call, so a future Spawn through this lane fails
at the runtime layer with a clear breadcrumb rather than starting a
broken session quietly. ports.Notifier and ports.AgentMessenger remain
stubbed alongside the LCM.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A new subscriber could receive the same PTY bytes twice: the ring snapshot
was taken under s.mu but replayed after unlock, while copyOut appended to the
ring and fanned out as two separate lock acquisitions. A chunk appended before
the snapshot could then be fanned out after the replay, delivering it in both.
The same gap let an exited frame overtake the replay.
Make append+fanout one atomic step under s.mu (deliver) and replay the snapshot
before releasing the lock in subscribe, so the two critical sections fully
serialize and each chunk reaches a subscriber exactly once.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Construct the tmux runtime and a terminal.Manager fed by the CDC
broadcaster, and hand it to httpd.New so the /mux WebSocket surface goes
live. httpd.New now runs after the CDC substrate so the broadcaster exists
when the manager subscribes; the listener still binds before running.json
is written, preserving fail-fast on port conflict. The manager is closed on
shutdown alongside the CDC pipeline and lifecycle stack.
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.
Add internal/terminal: a transport-agnostic feature package that attaches
a PTY to a session's tmux pane and multiplexes the byte stream to WebSocket
clients, plus a session-state channel fed by the CDC broadcaster.
The PTY is reached through a small PTYSource interface (satisfied by the
tmux runtime adapter) and spawned via an injectable spawnFunc, so fan-out,
the 50KB replay ring, and re-attach resilience all test without a real
process, tmux, or network. A tmux-guarded integration test exercises the
real creack/pty path end-to-end.
Raw PTY bytes never touch the CDC change_log; only the sessions channel is
CDC-fed. Windows PTY spawning is stubbed pending a ConPTY path.
Greptile P1: the patch-then-Update path in TestRestoreRoundTrip was
discarding GetSession's ok/err. A missed row would have handed UpdateSession
a zero-value SessionRecord (ID==""), which matches zero rows and returns
nil — Phase B then fails with the misleading "agent session id lost across
restart" instead of the real cause.
Fixed at the patch site (the only write path that could swallow the error)
and at the two read-then-assert sites in TestDetectingPersistsAcrossRestart
for consistency. Downstream assertions there would already fail loudly, but
the explicit ok/err check makes the failure mode unambiguous.
All 219 tests still pass under -race.
- Idempotent close + t.Cleanup so a mid-test t.Fatalf can't leak the
SQLite handle for the rest of the binary run. Restart-style tests still
call close() explicitly between phases; the cleanup hook becomes a no-op
once that runs.
- Drop the hardcoded "mer-1" assertion in TestHappyPath; assert the
structural invariant (project-scoped, non-empty id) instead, so the
test does not couple to the {project}-{counter} generation detail.
- Realign the test storeAdapter's PRFactsForSession + WritePR bodies to
be line-for-line identical to backend/lifecycle_wiring.go's production
adapter (extracted prState helper, separated err vs empty-rows checks),
so a future divergence shows up as a diff at review time. The proper
fix (extract to internal/storeutil) remains out of scope per the
brief's "do NOT redesign anything".
All 219 tests still pass under -race.
The Check formatting step failed on the stub method alignment in
stubAgent's three method declarations. Re-run of gofmt aligns the column
gutter on those signatures; no behavioural change.
prateek
Adil Shaikh
whoisasx
itrytoohard
harshitsinghbhandari
Dhruv Sharma
Harshit Singh Bhandari
Pritom Mazumdar
Vaibhaav Tiwari