fix(cli): require daemon transport for read commands#933
Conversation
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
…ly open 0.35 added a strict read-only schema check (#826) alongside the new tool_calls.file_path column (#846). Read-only commands never run migrations, so an archive last written by 0.34.x is missing that column. When the upgrade was done while an older daemon still owned the archive, every read-only open failed with a bare "schema missing tool_calls.file_path" and no way forward (issue #929); reverting to 0.34.5, which predates the check, was the only escape. OpenReadOnly now returns a typed SchemaUpgradeRequiredError, and the CLI maps it to actionable guidance: the pending migration only runs on a writable open, so the user must let the daemon restart to upgrade the archive. The historical error text is preserved so existing diagnostics still match.
Read-oriented CLI commands were still able to bypass daemon discovery and open the SQLite archive directly. That made the daemon-first migration incomplete: upgraded users could keep hitting unmigrated archives or stale daemon state through a compatibility path that is no longer supported. Daemon-capable reads now use the discovered daemon, start or replace it when appropriate, or fail with explicit guidance instead of falling back to local DB access. The only remaining local read paths are the explicit offline/archive cases and raw source-file export behavior that do not go through the daemon transport contract. Validation: go fmt ./...; CGO_ENABLED=1 go test -tags fts5 ./cmd/agentsview/... ./internal/service/... ./internal/server/... ./internal/db/... -count=1; CGO_ENABLED=1 go vet -tags fts5 ./...
Daemon-backed read commands need to keep the same user-facing behavior as the former direct SQLite path while still forcing reads through the daemon. Health output should include the same one-shot and automated sessions it reported before, health detail should accept the short IDs printed by that list, and stats with GitHub outcomes should use the daemon's server-side token instead of losing outcome aggregation. Read intents also need to replace older compatible daemons, because newly added read endpoints can otherwise hit a stale process and fail with missing-route errors instead of launching the upgraded daemon.
The daemon replacement path must preserve an explicit no-sync request from archive reads instead of inheriting only the old daemon runtime flags. Otherwise replacing an older daemon can unexpectedly restart syncing work for a command that deliberately asked not to sync. Health detail also needs exact session lookup before partial list matching, because exact IDs can refer to sessions outside the root health list page, including child or subagent sessions that remain individually addressable.
Background replacement can decide to stop an older daemon while another foreground startup is still publishing its replacement runtime. On slower Windows CI, that startup can finish before the background path observes the start lock, leaving the replacement code to act on the stale original decision. Refresh the current replacement target immediately before stopping a daemon so newly published writable runtimes are reused or reprobed instead of being stopped through an outdated runtime record.
Daemon-backed reads need to preserve health ID semantics and avoid stale daemon endpoints even when autostart is disabled. Exact health lookup now still checks displayed short-ID collisions, and read intent reports restart guidance instead of using an older daemon that may lack new read endpoints. Replacement refresh also keeps a previously selected target when the fresh probe temporarily cannot ping it but the runtime record remains stop-confirmed, avoiding false target changes in the stop/start path.
Session stats still depend on the SQLite-only v1 stats implementation. Routing the stats command through a read-only pg serve daemon therefore regressed local archive users by turning a previously local query into a remote not-implemented/read-only failure. Keep writable daemon routing intact, but bypass read-only daemon transports for stats until PostgreSQL has session-stats parity. The regression test locks in that a pg serve runtime is ignored and the local SQLite archive remains the source of truth.
The Windows Go job failed in the daemon replacement test because the helper subprocess can report that it acquired the kit start lock before the parent-side probe observes that lock. The test then enters the replacement path and trips the stop guard, even though the behavior under test is the wait/reuse branch. Make the helper return only after the parent can observe the external start lock. This keeps the coverage on the same replacement contract while removing the Windows-only synchronization race in the harness.
Read-intent commands can wait for a concurrently starting authenticated background daemon, but they previously kept the stale empty token loaded before that daemon persisted config. That made the freshly started daemon look unreachable and pushed read commands into the direct-read-only failure path. Reload the background launch config before retrying detection when a read intent lands in direct-read-only with no token. Also bound the projects daemon request so a stalled endpoint cannot hang the CLI indefinitely.
Daemon-backed stats now exposes the session-stats API path to CLI users, so invalid filters need to remain user-correctable validation failures instead of being reported as internal server faults. Mark stats filter validation errors explicitly and map them to HTTP 400 while leaving database and aggregation failures on the internal-error path.
Health detail resolution needs to behave like the former direct archive path after read commands move behind daemon transports. Filtering the first health list page misses older substring matches, so partial ID resolution now uses a dedicated service/API lookup with SQLite, DuckDB, and PostgreSQL implementations. The archive-query read-only daemon path also needs command-appropriate guidance: usage daily and statusline do not support --pg, so only session-usage reject paths keep that suggestion.
Health short-ID lookup should preserve the old strings.Contains semantics after moving behind the daemon API. Raw LIKE patterns made underscores and percent signs act as wildcards and let SQLite differ from PostgreSQL and DuckDB on case handling.
Use literal, case-sensitive substring predicates across stores, document the service contract, and move the resolver endpoint outside the /sessions/{id} namespace so it cannot shadow a real session ID.
The daemon-backed stats command depends on /api/v1/session-stats. Without an API version bump, an upgraded CLI can treat an already-running older daemon as compatible and fail against a missing route instead of replacing it. Bump the daemon/server API version so endpoint capability changes participate in the existing compatibility and replacement path.
The partial-ID resolver now promises literal, case-sensitive matching across stores. Add PostgreSQL pgtest coverage for wildcard characters and case variants so PG cannot drift from SQLite and DuckDB semantics.
mariusvniekerk
force-pushed
the
middleman/issue-929-agents-view-fails-to-load-history
branch
from
5c1b53e to
ade0b2a
Compare
roborev: Combined Review (
|
|
For the latest roborev finding on Daemon-backed stats should use daemon-side GitHub credentials. Forwarding a per-invocation CLI token through So the current behavior is acceptable: CLI-local GitHub tokens are not forwarded over the daemon API. Users who need daemon-backed GitHub outcome stats should configure the daemon environment/token. generated by a clanker |
2 participants
Read commands still had compatibility paths that could open SQLite directly when daemon discovery did not yield HTTP. That undermined the daemon-first migration and left upgraded users exposed to old schema/read-only failures instead of letting the writable daemon own migrations.
This makes daemon-capable reads resolve through daemon transport: they use reachable daemons, start or replace the local daemon where appropriate, and fail with restart guidance when the daemon is unreachable or incompatible instead of falling back. The remaining direct DB access is limited to commands whose contract is explicitly local, offline, or writer-owned, such as raw source export and offline archive queries.
generated by a clanker