feat(postgres): add named push targets

[rodboev]

pg push and pg status still assume one global PostgreSQL destination, which makes multiple remotes clumsy to operate and unsafe to share because one target can advance the same local watermark another target depends on, and the default-target service status path still reports the unscoped legacy watermark.

This change adds named [pg.NAME] targets and default_pg while keeping the existing single [pg] shape working as it does today. pg push and pg status can now use the default target, select one named target, or run --all sequentially. Target selection now resolves only the target being used, so a broken sibling target no longer blocks pg push archive, pg status archive, or the per-target --all loop. pg push --watch follows the default target or one selected named target, and rejects --all so the long-running multi-target watcher problem stays out of scope. The existing AGENTSVIEW_PG_* overrides stay supported, but they apply only to the effective default target instead of trying to rewrite every named entry.

The sync layer now scopes push watermark state by target name in named-target mode and lazily migrates the legacy global watermark into the selected default target on first use. Follow-up review fixes keep pg_push_marker_id local to the SQLite database so pg push --all does not self-conflict across named targets, preserve Last push: never for healthy pg service status output, and document that legacy [pg] field names are unavailable as named target identifiers. pg service status now reads the effective default target scope, and the PostgreSQL docs now describe named targets, default_pg, default-target-only environment overrides, and the default-target-only service and watch behavior. pg serve, service install, and the broader long-running multi-target watch problem still stay out of scope here so the PR stays reviewable.

Fixes #517

[roborev-ci]

roborev: Combined Review (6ea3cc0)

Synthesis unavailable. Showing individual review outputs.

claude-code — default (done)

I've verified the key mechanics: the target fingerprint (pgTargetFingerprint) incorporates database, user, schema, host, and port, so a migrated legacy watermark pointed at a genuinely different default-target DB self-corrects via pushTargetState → clearPushState (full re-push). The env-override relocation to pgEnvOverrides is safe because all runtime readers go through ResolvePG()/ResolvePGTarget(); only tests read c.PG.URL directly. The scoped-state migration is idempotent and crash-safe. Findings below.

Review Findings

Severity: Medium
Location: internal/config/config.go — finalize() (~line 1057) calling validatePGTargets()
Problem: finalize() now unconditionally calls validatePGTargets(), which fails the entire config load whenever default_pg is inconsistent (set without named targets, pointing at a nonexistent target, or multiple named targets with no default_pg). Because every Load* entry point runs finalize(), a single stray/typo'd default_pg line makes purely-local commands like agentsview serve, session list, etc. fail with invalid pg config: ... even though they never touch PostgreSQL. This is inconsistent with how legacy [pg] URL errors are handled — those are resolved lazily and only surface on PG commands. The error-case tests (TestResolvePGTargets_MultipleNamedTargetsRequireDefault) exercise ResolvePGTargets() directly and never go through Load, so this blast radius is untested.
Fix: Defer default_pg/named-target validation to the PG command resolution path (resolvePGTargetSelections/ResolvePGTarget, which already re-validate) instead of failing all config loads in finalize(); or only validate when a PG operation is actually requested.

---

Severity: Low
Location: internal/postgres/sync.go Status() / ReadLastPushAt(); cmd/agentsview/pg_service.go readServiceLastPush()
Problem: For the default named target, status reads carry MigrateLegacySyncState: true, so ensureMigration() fires on a read. Running pg status (default) or pg service status silently rewrites local sync state — moving last_push_at/boundary/fingerprint into the :target scope and clearing the legacy keys — before any push to the new target has happened. A user inspecting status, then reverting to a legacy [pg] config, loses the legacy watermark and triggers a full re-push. No data loss (fingerprint mismatch self-corrects), but a read-only status command mutating persistent state is surprising.
Fix: Gate the legacy migration to the actual push path (or pass migrateLegacy=false from status readers), so status remains side-effect-free.

---

Severity: Low
Location: cmd/agentsview/pg.go — runPGPushTarget() and runPGStatusTarget()
Problem: applyClassifierConfig(appCfg) is already called once in runPGPush/runPGStatus before the target loop, then called again inside runPGPushTarget/runPGStatusTarget per target. With --all over N targets this runs N+1 times with identical appCfg. Redundant.
Fix: Drop the per-target applyClassifierConfig call; the single pre-loop call already covers all targets.

---

Severity: Low
Location: internal/config/config.go — ResolvePGTargets() (~line 1565)
Problem: The exported ResolvePGTargets() is referenced only by config_test.go; the CLI fan-out uses cmd/agentsview/pg.go:resolvePGTargetSelections() instead, which reimplements the same default-first sort.Slice ordering. This is unused production code plus duplicated ordering logic that can drift.
Fix: Either route the CLI selection through ResolvePGTargets() (sharing the ordering), or remove the unused exported method.

Summary

Adds named [pg.NAME] PostgreSQL targets with default_pg selection, per-target scoped sync-state (with legacy-key migration for the default target), and --all/positional-target support for pg push/pg status, while keeping pg serve/pg service on the effective default target.

---

claude-code — security (done)

I've reviewed the diff and verified the security-relevant paths. Here is my assessment.

Summary

This change set adds named PostgreSQL targets ([pg.NAME] blocks plus default_pg) alongside the legacy single [pg] block, scopes per-target push watermark/sync-state keys, and refactors the pg push/pg status CLI from fatal() to returned errors with multi-target failure aggregation. It is entirely local-CLI/config plumbing for a single-user tool; it does not touch the HTTP server, auth model, Caddy proxy, or any network-facing bind.

I checked the most plausible exploit candidates:

• Credential exposure in errors/logs. Connection failures route through postgres.Open, which wraps driver errors with RedactDSN(dsn) (hostname only). CheckSSL reports cfg.Host, not credentials. The multi-target failure aggregation in runPGPush/runPGStatus interpolates target.label() (the target name, e.g. work (default)), not the URL. No DSN/password reaches stderr/logs, and any output stays on the owner's own terminal (no new trust boundary).
• Service-install env-dependency bypass. buildServiceSpec now resolves via RawPGTarget(""), but rejectEnvDependentServicePGURL still hard-rejects a set AGENTSVIEW_PG_URL and ${...}-expanded URLs for the default target, preserving the invariant that the daemon (which doesn't inherit shell env) only installs with a literal URL. The target name is not interpolated into the systemd/launchd unit (the unit runs pg push --watch with no target arg), and validateServiceSpec still rejects control/quote characters in spec values.
• SQL/schema injection via new config surface. Named-target schemas still flow through postgres.Open → quoteIdentifier, which validates against ^[a-zA-Z_][a-zA-Z0-9_]*$. Target names are normalized (TrimSpace+ToLower), used only as map keys and sync-state key suffixes against parameterized Get/SetSyncState helpers, and originate from the user-owned config file (not an attacker boundary in this single-user model).
• Per-target SSL/allow_insecure handling. Each target's AllowInsecure is decoded independently per [pg.NAME]; env overrides apply only to the effective default target. No cross-target contamination that would silently downgrade TLS enforcement.

The env-override refactor (moving AGENTSVIEW_PG_* into pgEnvOverrides applied in resolvePGConfig) preserves the prior env-wins-over-file precedence without weakening any security check.

No issues found.

[rodboev]

Addressed in afc6ea0.

[rodboev]

Follow-up details for afc6ea0:

• named-target validation now stays on PG resolution paths instead of config finalization, so unrelated commands can still load config
• status reads are side-effect free again; they read the scoped watermark first, then fall back to the legacy default watermark without migrating state
• the duplicate per-target classifier setup is gone
• target ordering now comes from shared config helper logic without eagerly resolving sibling targets

[roborev-ci]

roborev: Combined Review (afc6ea0)

No issues found.

---

Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 8m47s), codex_security (codex/security, done, 3m26s) | Total: 12m13s

[mjacobs]

Nice work — the named-target design is clean and the genuinely tricky parts are handled carefully. Things I specifically liked:

• Validation is correctly lazy: finalize() (internal/config/config.go:1057) no longer validates PG targets, so a typo'd default_pg still lets purely-local commands load and only PG resolution errors — TestLoadMinimal_DefersNamedPGValidationForNonPGCommands pins it.
• Per-target watermark scoping is applied uniformly — every last_push_at/boundary/fingerprint access in Push moved from s.local to the scoped store from effectiveSyncState(), with pg_push_marker_id deliberately kept DB-global (push.go) so pg push --all doesn't self-conflict.
• The legacy→default migration (internal/postgres/sync.go:43-94) is idempotent and self-healing: a legacy key is cleared only after its value lands in scope, so a crash leaves the value in both places, never neither.
• The status read path is genuinely read-only: ReadLastPushAt (sync.go:342) builds its scoped store with migrateLegacy=false and only falls back to a plain read of the legacy key.

Two minor, non-blocking items:

1. (low — test placement) The three TestScopedSyncStateStore* tests in internal/postgres/sync_test.go cover the most safety-critical new logic (the copy-then-clear migration), but that file is //go:build pgtest, so the default make test skips them — go test -tags fts5 -run TestScopedSyncStateStore ./internal/postgres/... reports [no tests to run]. They need no PostgreSQL (testDB is just db.Open(tmp)). Moving them — and testDB — into the existing non-pgtest internal/postgres/sync_unit_test.go would let the default suite guard the migration loop. (Non-tagged files compile into the pgtest build too, so the remaining PG tests keep seeing testDB.)
2. (nit) Header output differs between sibling commands for a single named target: pg status work prints Target: work (pg.go:280) but pg push work prints nothing (pg.go:107). Cosmetic; aligning push to the status condition would let pg push work confirm which target it wrote to.

One question: is pg push --all against a legacy single [pg] block (currently accepted as a plain single push) the intended contract, or would you rather it error since there's nothing to fan out over? Harmless either way.

Also checked the earlier automated-review items against the latest commit: the validation-scoping and status-read fixes are both in afc6ea0f and tested, the per-target classifier double-call is gone, and the unused ResolvePGTargets() is at most a minor cleanup (the CLI fan-out shares one ordering source via PGTargetNames()).

[wesm]

rebasing

[roborev-ci]

roborev: Combined Review (45c7b98)

High severity issue found: pg status can now mutate the local SQLite archive.

High

• Location: cmd/agentsview/pg.go:226
• Problem: pg status now opens the local SQLite archive with openDB, causing a read-only status command to run writable initialization and migrations. db.Open can create the DB and, on stale or incomplete schema, remove and recreate the archive, so simply checking PostgreSQL status can mutate or damage local session state.
• Fix: Use openReadOnlyDB here as before, or introduce a read-only sync-state path that tolerates a missing DB without calling db.Open.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 6m22s), codex_security (codex/security, done, 3m14s) | Total: 9m43s

[roborev-ci]

roborev: Combined Review (94357a9)

No issues found.

---

Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 11m26s), codex_security (codex/security, done, 1m3s) | Total: 12m29s