feat(sync): local-first multi-machine artifact sync

[maphew]

Implements local-first multi-machine artifact sync from #692. Each machine retains its SQLite archive and exchanges immutable, content-addressed session artifacts plus a hybrid logical clock metadata ledger through shared folders, HTTP peers, or S3-compatible object stores.

The sync path preserves origin-aware identities through SQLite and optional PostgreSQL, treats imported artifacts as untrusted input, regenerates corrupt local objects from SQLite, and safely retries partially published metadata. Lifecycle operations serialize local trash, restore, and purge mutations with peer metadata replay, so pre-publication failures compensate safely and purge artifacts become durable before local deletion.

Transport security matches the project threat model: non-loopback HTTP requires explicit --allow-insecure consent, authenticated redirects are rejected, custom S3 endpoints require TLS outside loopback unless explicitly opted in, and insecure object-store cleanup cannot delete remote objects.

Automatic garbage collection prunes both the local store and target after folder sync. HTTP and S3 targets retain superseded history until transport-aware remote pruning is available, preventing remote-only artifacts from entering a recurring download-and-delete loop.

The branch also adds conflict and peer visibility, conservative manual garbage collection, and operational documentation. Legacy PostgreSQL identities converge without duplicating sessions or losing local curation, relationships, source state, or soft-delete state.

Closes #692.
Closes #1034.
Closes #1035.

[maphew]

Feedback is welcome. Still in draft mode since work and testing to this point has been completely agent driven, a combination of GPT-5.5 and Claude 4.8. Next up is manually trying various distributed machine scenarios and seeing how well any of this works in practice. Assuming the idea eventually proves out, I'm happy to split into smaller manageable PR.

[roborev-ci]

roborev: Combined Review (2e11257)

High-level verdict: two actionable findings remain; no additional security issues were reported.

High

• internal/artifact/metadata.go:145 - Metadata event filenames are built from HLCTimestamp.String(), whose layout contains : characters. Windows is supported, and : is invalid in Windows filenames, so rename/star/pin/delete requests can fail while writing the metadata artifact after the DB mutation has already been applied.
  • Fix: Use a filesystem-safe HLC representation for artifact filenames, or encode/decode the HLC separately from the JSON event value, and add coverage for Windows-safe metadata paths.

Medium

• internal/server/server.go:125 - The server creates a metadata recorder with cfg.ArtifactOriginID, but when that value is empty the recorder falls back to artifact.EnsureOrigin() in SQLite sync state, while CLI folder sync later uses config.EnsureArtifactOriginID() in config.toml. That can create two different origins for the same machine, causing pre-init metadata events to target oldOrigin~session while exported sessions use newOrigin~session, so peers cannot replay those metadata events.
  • Fix: Use one authoritative origin source everywhere: ensure and persist the config origin before enabling metadata recording/import, or make CLI/server/import/conflict lookup all share the same persisted origin.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 9m1s), codex_security (codex/security, done, 7m54s) | Total: 17m4s

[maphew]

Thanks for the review. Both findings were valid and are addressed in 28048707 and f252c35f.

High — Windows-invalid : in metadata filenames. Confirmed: filenames are OrderingKey(hash) = HLCTimestamp.String() + hash, and String() rendered the wall time with 15:04:05 colons. Since the DB mutation runs before the metadata write (e.g. humaStarSession), star/pin/rename/delete would mutate the DB and then fail the file write on Windows, diverging the DB and the ledger. Fix: dropped the : separators from the HLC wall layout (2006-01-02T150405.000000000Z). Keeping a single representation everywhere — content, filename, and the persisted orderKey cursor — preserves lexicographic ordering, parse round-trip, and the filename-equals-content integrity check with no separate encode/decode. Updated the canonical-format goldens, added an assertion that the filename has no Windows-invalid characters, and a parse-rejection case for the old colon form.

Note this changes the canonical on-disk HLC string (event.HLC no longer contains colons). That is fine here since the format is pre-release, but any artifacts written by an earlier draft build would be stale-format.

Medium — divergent origin sources. Confirmed: runServe never populated cfg.ArtifactOriginID before server.New, so an empty config origin made the recorder fall back to a random DB-state origin while CLI folder sync minted a machine-named origin in config.toml. Fix: ensure the config origin (authoritative) at serve startup before the recorder exists, and reconcile the DB sync-state to it via a new idempotent artifact.AdoptOrigin. The recorder, peer import, and folder sync now converge on one persisted origin. Added AdoptOrigin unit tests (persist, idempotent, overwrite-divergent, reject-invalid).

go vet, gofmt, and the artifact/server/config/e2e Go suites pass.

Claude Opus 4.8 reasoning-medium on behalf of maphew

[roborev-ci]

roborev: Combined Review (f252c35)

High-risk issues remain in metadata replay/local state handling; no security-specific findings were reported.

High

• internal/artifact/sync.go:523 - Local metadata events are never recorded in metadata_replay_state because the importer skips the local origin entirely and MetadataRecorder.Append only writes the artifact file. A later peer event for the same field is treated as the first winner and can overwrite a newer local rename/star/delete/pin without recording a conflict.
  Fix: Keep skipping local session manifests, but replay local meta/ events into the replay tables, or have local append also persist the local projection/applied-event state.

Medium

• internal/artifact/replay.go:51 - Imported remote HLCs are never observed by the local HLC clock, so after replaying a peer event whose clock is ahead, the next local edit can get an older HLC and lose LWW ordering despite happening after the import.
  Fix: Parse accepted remote event HLCs during replay and advance the persisted metadata clock with HLCClock.Observe.

• internal/artifact/replay.go:53 - A single ErrMetadataTargetUnavailable aborts replay for the rest of that origin, so one missing/excluded session or pin target can block unrelated later metadata events from applying.
  Fix: Defer only that event without marking it applied, then continue replaying later events whose targets are available.

---

Panel: ci_default_security | Synthesis: codex, 11s | Members: codex_default (codex/default, done, 7m18s), codex_security (codex/security, done, 4m50s) | Total: 12m19s

[maphew]

Thanks again. All three findings were valid and are addressed in 055f3b3c.

High — local metadata events missing from the replay register (sync.go:523). Confirmed: import skips the local origin and MetadataRecorder.Append only wrote the artifact file, so locally-originated edits never landed in metadata_replay_state/metadata_applied_events. A later peer event for the same field saw no current winner and could overwrite a newer local rename/star/delete/pin without recording a conflict. Fix: Append now records the event through a new db.RecordLocalMetadataProjection, which runs the same per-field LWW conflict/register/applied bookkeeping as replay but skips re-applying the mutation (the handler already applied it). ApplyMetadataProjection was refactored to share that path via an applyMutation flag. Added a test where a local rename with a higher HLC survives a later-imported lower-HLC peer rename and the peer is recorded as the conflict loser.

Medium — remote HLCs not observed by the local clock (replay.go:51). Confirmed. Replay now tracks the newest accepted remote HLC per origin and calls HLCClock.Observe once at the end, so the next local edit is causally ahead of imported peers. Advancing past the drift bound is best-effort and never fails an otherwise successful import. Import is routed through the recorder (MetadataRecorder.Import) so appends and imports share one clock instance rather than racing two clocks on the same persisted key. Added a test asserting a post-import local edit gets an HLC strictly after a peer HLC that was ahead of the local wall clock.

Medium — one unavailable target aborted the rest of the origin (replay.go:53). Confirmed. ErrMetadataTargetUnavailable now skips only that event (left unapplied, watermark not advanced) and replay continues with later events, which retries the deferred event on a subsequent run once its target exists. Added a test where an event for a missing session no longer blocks a later event for an existing session, and the deferred event applies after the session arrives.

go vet, gofmt, and the artifact/db/server Go suites pass.

Claude Opus 4.8 reasoning-medium on behalf of maphew

[roborev-ci]

roborev: Combined Review (14f32bc)

Artifact sync is not yet complete: two medium-severity data convergence gaps need fixes before merge.

Medium

• internal/artifact/sync.go:414
  Artifact export pages through ListSessions, which applies the normal session-list visibility filter (message_count > 0). Zero-message sessions with usage events are never exported or synced, even though artifact manifests support usage events.
  Fix: Export from a raw DB enumeration for owned, non-deleted sessions instead of the UI list API, and add coverage for zero-message/usage-only sessions.

• internal/server/huma_routes_starred.go:92
  Bulk star updates the DB but does not append metadata events. This path is used for localStorage star migration, so those stars remain local-only and never converge through artifact sync.
  Fix: Have bulk star return or determine the sessions actually starred, then append a MetadataOpStar event for each valid session.

---

Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 10m33s), codex_security (codex/security, done, 5m28s) | Total: 16m9s

[maphew]

Thanks. Both convergence gaps were valid and are fixed in 1d8d24c1 and 8cac9fff.

Medium — usage-only sessions never exported (sync.go:414). Confirmed: export paged through ListSessions, whose sidebar filter (message_count > 0) hid owned sessions that carry only usage events, so they never reached a checkpoint. Fix: export now enumerates owned, non-deleted sessions from a raw query (db.ListOwnedSessionIDsForExport) instead of the list API. Soft-deleted and foreign sessions stay excluded — deletes still propagate via metadata tombstone events, never checkpoint absence. Added a test that a zero-message session with a usage event lands in the checkpoint and imports into a peer with its usage events intact, plus one asserting deleted/foreign sessions stay out.

Medium — bulk star emitted no metadata events (huma_routes_starred.go:92). Confirmed: bulk star wrote the DB but appended nothing, so localStorage-migrated stars stayed local-only. Fix: BulkStarSessions now returns the IDs actually starred (sessions that exist), across the SQLite, PostgreSQL, and DuckDB backends to preserve parity, and the handler appends a star metadata event for each, matching single-session star. Stale IDs are still skipped and produce no event. Added a server test asserting one star event artifact per existing session and none for a missing id.

go vet, gofmt, and the db/artifact/server/duckdb Go suites pass; the PG curation pgtest compiles under the pgtest tag.

Claude Opus 4.8 reasoning-medium on behalf of maphew

[roborev-ci]

roborev: Combined Review (97c2128)

Artifact sync has several medium-risk correctness issues that should be fixed before merge.

Medium

• cmd/agentsview/sync.go:118 - agentsview sync --host <host> <artifact-target> is accepted, but the early return after runRemoteSync skips artifact sync entirely, silently ignoring the target. Reject --host with any artifact target, or run runArtifactFolderSync before returning.

• internal/artifact/transport_s3.go:355 - S3 uploads use unconditional PUT, so a stale or concurrent sync can overwrite an existing artifact object. This violates the write-once artifact invariant, especially for sequence-named checkpoints. Use conditional writes such as If-None-Match: *; on “already exists”, fetch and compare content, accepting identical duplicates and rejecting conflicts.

• internal/artifact/sync.go:1415 - CopyUnion defers from.Close() inside the walk loop, keeping every copied source file open until the entire traversal finishes. Large artifact stores can hit the process file descriptor limit and fail mid-sync. Close each source file immediately after io.Copy, handling both copy and close errors.

• internal/artifact/replay.go:86 - Metadata replay applies remote events before advancing the local HLC, then silently ignores Observe failures for future-skewed remote HLCs. A later local edit can get a lower order key, be recorded as the losing value, while the handler has already mutated the local DB to that losing value. Do not apply remote metadata unless the clock can be advanced, or make local event creation consult/advance past the replay-state winner before applying the local mutation.

---

Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 10m13s), codex_security (codex/security, done, 6m59s) | Total: 17m22s

[maphew]

Thanks. Addressed in e77db3a9, 4110dce2, acbb7899, and 6ea6fa89.

Medium — --host + artifact target silently ignored (sync.go:118). Confirmed: --host dispatches to SSH remote sync and returns before artifact sync runs. Fix: validateSyncConfig now rejects the combination outright (ordered after the --init/--watch checks so their specific messages still win). Added accept/reject tests.

Medium — unconditional S3 PUT violates write-once (transport_s3.go:355). Confirmed: unlike the folder and HTTP transports, S3 blind-overwrote. Fix: uploads now send If-None-Match: *; on the 412 precondition failure the object is fetched and compared — identical content is an accepted duplicate, divergent content is a conflict, never a silent merge. The mock honors the conditional and a test covers both outcomes, and the MinIO integration test confirms a real server accepts it.

Medium — CopyUnion deferred close (sync.go:1415). This one is actually a false positive: the defer from.Close() was inside the per-entry WalkDir callback closure, so it ran after each file, not at the end of the traversal — no descriptor accumulation. That said, I removed the ambiguity entirely by switching to os.ReadFile (no manual open/defer), which reads cleaner regardless.

Medium — remote events applied before the HLC advances (replay.go:86). Confirmed and the most important one. Fix: replay now calls Observe on each remote event's HLC before applying it and defers any event whose wall time is beyond the drift bound, leaving it unapplied for a later run to retry once local wall time catches up. A future-skewed peer can no longer drag local state to a value a subsequent local edit couldn't out-order. Added a deferral test; the two-instance harness now orders concurrent edits with within-drift offsets.

go vet, gofmt, and the artifact/db/server/duckdb/cmd Go suites pass; the MinIO integration test (make test-minio) passes against real MinIO.

Claude Opus 4.8 reasoning-medium on behalf of maphew

[roborev-ci]

roborev: Combined Review (6ea6fa8)

Medium severity findings remain.

Medium

• internal/duckdb/store.go:273
  DuckDB clears Cursor and Limit before applying the generic session filter, so the new sidebar endpoint can return the entire corpus instead of the requested first page. In starred-only mode it also filters only starred roots, missing groups where a child is starred but the root is not, unlike SQLite/Postgres sidebar semantics. Preserve pagination inputs and implement the same root-page plus descendant expansion query used by the other backends, including “any starred session in the tree” root eligibility.

• internal/artifact/replay.go:49
  Metadata artifact replay accepts filenames/order keys whose HLC component cannot be parsed, because normalizeMetadataName only checks the hash suffix and replay silently ignores ParseHLCTimestamp failures. A peer that can write to the shared artifact transport could publish an order key like zzzz-<sha256>.json with matching body metadata, bypass drift checks, and win LWW metadata conflicts such as purge, soft delete, rename, star, and pin. Reject metadata artifacts whose HLC component cannot be parsed, ideally in validateMetadataArtifactEvent or normalizeMetadataName, while preserving drift deferral for valid future timestamps.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 6m9s), codex_security (codex/security, done, 7m2s) | Total: 13m20s

[roborev-ci]

roborev: Combined Review (18c0f18)

Medium issues need attention before merge.

Medium

• internal/postgres/push.go:1094 - resolvePushedSessionIdentity treats any existing row with a different machine as a native ID collision, ignoring owner_marker and legacy marker machine aliases. After a machine rename, a session already owned by the same marker can be pushed as new-machine~id, duplicating the existing PG session instead of updating it.

  • Fix: Resolve identity using owner_marker and legacy marker aliases, matching the ownership rules used later in pushSession; only prefix IDs for true cross-owner collisions.

• internal/artifact/sync.go:561 - Folder import accepts unvalidated origin directory names. A crafted or mistaken origin such as local can be imported and rewritten with Machine = "local", causing foreign sessions to be treated as locally owned and re-exported under this machine’s origin.

  • Fix: Validate folder origin names with validateOriginID before importOrigin, and reject reserved or malformed origins consistently with HTTP/S3 index validation.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 12m20s), codex_security (codex/security, done, 7m46s) | Total: 20m13s

[wesm]

I will rebase this

[roborev-ci]

roborev: Combined Review (b228d18)

High-level verdict: changes need fixes before merge due to artifact import identity/state corruption risks.

High

• internal/artifact/sync.go:738
  Imported artifacts copy the source machine’s FilePath, file hash/mtime/size, incremental ordinals, and inode/device fields into the local SQLite session row. Existing local sync lookups use file_path without filtering by machine, so a peer artifact whose path exists locally can cause local sync to skip the real local file or append local messages into the imported origin~id session.
  Fix: Clear or namespace parser-local file metadata/control fields during artifact import, or make all sync file-path/incremental queries restrict to locally owned sessions.

Medium

• internal/artifact/sync.go:755
  Artifact import prefixes parent_session_id and subagent tool-call session IDs, but leaves source_session_id unchanged. Imported sessions are stored as origin~nativeID, so an unprefixed source_session_id will dangle or point at an unrelated local session.
  Fix: Rewrite non-empty, non-global SourceSessionID the same way as ParentSessionID, and add an import test covering source/parent/subagent relationship rewriting.

• internal/config/config.go:1588
EnsureArtifactOriginID generates and writes a new origin without taking the config lock or re-reading under that lock. Concurrent first starts can generate different origin IDs, causing one process to use an origin that is overwritten by another and splitting local artifact identity.
  Fix: Wrap the ensure path in withConfigLock, re-read the config map while locked, honor any existing artifact_origin_id, and only generate/write when still absent.

---

Panel: ci_default_security | Synthesis: codex, 11s | Members: codex_default (codex/default, done, 12m13s), codex_security (codex/security, done, 7m49s) | Total: 20m13s

[wesm]

I'll continue to work a bit on this to see if I can get it into a state that I'm comfortable with

[roborev-ci]

roborev: Combined Review (16e5a7b)

High-level verdict: changes are not ready as-is due to one high-severity artifact sync bypass and two medium-severity reliability issues.

High

• cmd/agentsview/sync.go:175
  When a writable daemon is running, agentsview sync /artifact-share returns immediately after daemon sync, so the artifact target and --init are silently ignored and no artifact exchange happens.
  Fix: Route artifact exchange through the daemon, or reject artifact sync while a writable daemon owns the DB; add a daemon-backed artifact sync test.

Medium

• internal/artifact/transport_http.go:218
  HTTP artifact uploads do not set an Origin header, so POSTs to a default no-auth loopback peer are rejected by the server’s CORS middleware with 403.
  Fix: Set Origin to the peer origin for mutating peer requests, matching the daemon sync client behavior.

• internal/config/config.go:1914
EnsureArtifactOriginID reads and writes config.toml without withConfigLock or re-reading under the lock, so two first-run processes can generate different origins and fork the machine identity.
  Fix: Wrap the read/generate/write path in withConfigLock and reuse an existing artifact_origin_id found after acquiring the lock.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 13m9s), codex_security (codex/security, done, 11m17s) | Total: 24m35s

[roborev-ci]

roborev: Combined Review (8733aec)

Medium-risk issues remain.

Medium

• internal/artifact/sync.go:803 - Artifact import restores sessions from m.Session, but db.Session.SessionName is json:"-" and is not restored from the manifest. The batch upsert writes session_name from that nil field, causing synced sessions to lose parser-provided titles and potentially clearing titles on existing imported rows. Add an explicit manifest SessionName *string, populate it on export, restore it in rewriteForImport, and add an artifact round-trip test.

• cmd/agentsview/sync.go:291 - When agentsview sync targets an HTTP(S) artifact peer without --token, artifactPeerToken falls back to appCfg.AuthToken, which is then sent as Authorization: Bearer ... to the peer via internal/artifact/transport_http.go:253. A malicious or compromised peer could collect the local AgentsView auth token and use it for API access. Require an explicit peer token for HTTP(S) artifact targets, make local-token reuse explicit opt-in, or avoid sending Authorization when no peer-specific token is configured.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 8m29s), codex_security (codex/security, done, 8m0s) | Total: 16m38s

[roborev-ci]

roborev: Combined Review (7851273)

Medium-risk issues found in metadata replay/artifact publishing; security review found no additional issues.

Medium

• internal/db/metadata_replay.go:184 - Local metadata events are recorded with applyMutation=false after handlers have already mutated the row. If a peer metadata import runs between the handler mutation and this projection write, the new local event can win the LWW register without reapplying its value, leaving metadata_replay_state saying the local edit won while the actual session/star/pin row still contains the peer value. Serialize the user mutation and local projection in one transaction/critical section, or have local projection recording idempotently reapply the local mutation when it wins.

• internal/artifact/metadata.go:159 - Append marks the local metadata event applied/current in SQLite before the immutable event file is written. A crash or writeFileAtomic failure after that point leaves the local mutation committed and the replay state advanced, but no artifact exists for peers; for purge/delete-everywhere this can permanently lose the fleet-wide delete event after the session is gone. Use a durable outbox or transactional publish flow so the event artifact is written or queued for retry before the mutation is considered successful and before replay state is advanced irrecoverably.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 18m9s), codex_security (codex/security, done, 6m21s) | Total: 24m39s

[roborev-ci]

roborev: Combined Review (efb934f)

Summary verdict: Changes are not clean; 2 medium issues need attention before merge.

Medium

• internal/db/orphaned.go:337
  Full resync only copies pg_push_marker_id from pg_sync_state and does not copy the new artifact metadata replay tables. After a temp-DB rebuild, local curation rows may be preserved, but metadata_replay_state, metadata_applied_events, and the artifact HLC/import state are lost. A later artifact import can replay older foreign rename/star/trash events as new while local-origin events are skipped, potentially overwriting preserved local metadata.
  Fix: Preserve artifact sync state and metadata replay tables during resync, or rebuild replay state from local metadata artifacts before importing foreign events.

• internal/artifact/peer.go:288
  Peer/object artifact storage validates artifacts with current-version import decoders before storing them. Future-version checkpoints, segments, and metadata events are rejected by WriteArtifact/ReadArtifact, so an older HTTP peer or S3 client cannot act as a pass-through store during rolling upgrades and sync aborts instead of deferring unknown artifacts.
  Fix: Split storage validation from import validation: verify names, hashes, and basic origin/sequence envelopes where possible, store/serve opaque future-version artifacts, and leave version-specific rejection to the import path.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 12m23s), codex_security (codex/security, done, 7m39s) | Total: 20m11s

[roborev-ci]

roborev: Combined Review (550372b)

Summary verdict: medium-risk sync consistency issues need fixes before merge.

Medium

• internal/server/huma_routes_sessions.go:669
  Batch soft-deletes update SQLite but never append MetadataOpSoftDelete events, so sessions deleted via the multi-select/batch endpoint do not converge through artifact sync while single deletes do.
  Fix: Return or compute the IDs actually newly deleted and append one soft-delete metadata event per changed session.

• internal/artifact/peer.go:510
  Peer/S3 artifact writes reject future-version checkpoints, segments, and metadata as invalid, which makes HTTP/S3 sync fail against a newer peer instead of storing the immutable artifact and letting import defer it.
  Fix: Treat errFutureArtifactVersion as acceptable for transport storage after minimal origin/name/hash validation, and leave deferral to the import path.

• internal/db/metadata_replay.go:419
  Replayed purge events only tombstone the exact session ID, unlike the existing permanent-delete helpers that also exclude parser fallback aliases; delete-everywhere can therefore be resurrected under an alias on a later local sync.
  Fix: Resolve alias IDs before deleting and insert all of them into excluded_sessions in the same transaction.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 12m28s), codex_security (codex/security, done, 5m7s) | Total: 17m44s

[roborev-ci]

roborev: Combined Review (eaf6694)

Medium-risk issues remain in artifact sync and artifact upload handling.

Medium

• internal/artifact/sync.go:1114 - Artifact export/import drops db.ToolCall.FilePath because segmentToolCall never carries it, so synced sessions lose edit/write file paths and disappear from Recent Edits or downstream PG/DuckDB mirrors.
  Fix: Add a file_path field to segmentToolCall, populate it from call.FilePath, restore it in dbMessage(), and add a round-trip test.

• internal/artifact/sync.go:749 - Checkpoint and manifest validation accepts arbitrary referenced manifest/segment strings as long as they are non-empty. Those values are later used in filepath.Join, so malformed artifacts can abort imports or resolve .. path components outside the intended artifact-kind directory.
  Fix: Validate checkpoint manifest hashes and manifest segment/raw hashes with validateHashHex before accepting or importing them, and reject malformed artifacts early.

• internal/server/huma_routes_artifacts.go:236 - humaPostArtifact writes request bodies to $AGENTSVIEW_DATA_DIR/artifacts via artifact.WriteArtifact without checking s.db.ReadOnly() or whether the server has a writable local archive. A client that can reach a read-only pg serve/DuckDB serve instance can post arbitrary SHA-matching content, creating persistent files and consuming disk where other mutating routes are blocked.
  Fix: Reject artifact POST in read-only mode before calling WriteArtifact, or only register the write route when the server has a writable local *db.DB plus sync/metadata support.

---

Panel: ci_default_security | Synthesis: codex, 12s | Members: codex_default (codex/default, done, 15m4s), codex_security (codex/security, done, 5m54s) | Total: 21m10s

[roborev-ci]

roborev: Combined Review (f944578)

Reviewed artifact sync changes: one medium-severity correctness finding remains; no high or critical issues reported.

Medium

• internal/server/huma_routes_starred.go:76 - humaUnstarSession appends an unstar metadata event even when the session ID does not exist or no star row was removed. With artifact sync, that can create a durable last-writer-wins tombstone for arbitrary IDs, so a mistyped or stale request can later suppress a valid peer star for that session. Fix by checking that the session exists, or by having UnstarSession report whether it removed an existing star, before appending metadata. Keep idempotent HTTP behavior if desired, but do not publish metadata for nonexistent/no-op targets.

---

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 7m50s), codex_security (codex/security, done, 5m59s) | Total: 13m55s

[roborev-ci]

roborev: Combined Review (69ce721)

Verdict: One high-severity security vulnerability and two medium-severity reliability/scalability issues require attention.

High

• Destination symlinks permit arbitrary file creation outside the artifact folder — internal/artifact/sync.go:2591

  CopyUnion builds destination paths with filepath.Join and uses os.MkdirAll without rejecting symlinked path components. An attacker who can modify a shared artifact target could replace a copied directory with a symlink, causing a subsequent sync to create attacker-controlled files outside the artifact folder with the agentsview user’s privileges.

  Confine destination operations to an opened filesystem root and reject or safely resolve symlink components. Consider also restricting folder synchronization to valid artifact paths rather than mirroring arbitrary unknown trees.

Medium

• Transport preparation discards cancellation context — internal/artifact/transport_http.go:85, internal/artifact/transport_s3.go:178

  Preparation uses context.Background(), so cancellation or Ctrl-C cannot interrupt HTTP/S3 readiness probes. An unavailable peer may therefore stall shutdown for the full 120-second timeout.

  Pass the caller’s context through Transport.Prepare and use it for readiness requests.

• Unchanged syncs create unbounded immutable checkpoints — internal/artifact/sync.go:599

  Every sync creates a checkpoint even when the published session set and manifest hashes are unchanged. At a 15-minute interval, this produces roughly 35,000 checkpoints per origin annually. HTTP and S3 targets have no automatic garbage collection, while S3 scans the growing prefix on every exchange.

  Skip checkpoint creation when the prospective session map matches the latest valid checkpoint, or implement bounded retention for HTTP and S3 targets.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m48s

[roborev-ci]

roborev: Combined Review (962c765)

Verdict: One high-severity security vulnerability and two medium-severity synchronization issues require fixes.

High

• Source-side symlink race can exfiltrate arbitrary local files — internal/artifact/sync.go:2659

  During inbound folder synchronization, a peer with write access to the shared artifact folder can replace a validated file—or an ancestor directory—with a symlink before os.ReadFile(path) opens it. Because the read follows the symlink and unrecognized paths are mirrored, arbitrary readable local files could enter the artifact store and later be published back to the shared folder.

  Fix: Open and pin the source with os.OpenRoot; use root-relative, confined traversal and reads; and verify the opened file descriptor is regular after opening.

Medium

• Stale replicas may overwrite newer canonical PostgreSQL sessions — internal/postgres/push.go:2199

  Imported replicas use the originating session’s owner marker and may overwrite its PostgreSQL row without any manifest hash, checkpoint sequence, or freshness comparison. A stale replica can replace newer metadata and messages, while the origin’s incremental watermark may prevent restoration.

  Fix: Distinguish source-owned sessions from imported replicas. Make replica pushes insert-only when a canonical row exists, or compare a persisted artifact revision/hash before updating.

• Each artifact upload repeatedly imports the entire store — internal/server/huma_routes_artifacts.go:247

  Every artifact POST synchronously triggers a full-store import. Since artifacts are uploaded individually and checkpoints can arrive before referenced manifests and segments, initial synchronization repeatedly scans incomplete sessions, causing approximately artifact-count × session-count work and potential timeouts on large archives.

  Fix: Stage uploads and import once through a batch/finalize operation or coalesced background job. At minimum, upload dependencies before checkpoints and skip full imports for manifest/segment uploads.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 12m25s

[roborev-ci]

roborev: Combined Review (5ce0710)

Artifact synchronization has four medium-severity issues involving symlink safety, import retries, and corrupted checkpoints.

Medium

• internal/artifact/sync.go:400 — Symlinked roots can cause recursive self-copy. Overlap validation compares only lexical absolute paths. A target symlink may resolve to the local artifact store or an ancestor, allowing the destination-inside-source walk to recursively copy the store into itself and exhaust disk space. Resolve both roots to canonical paths or compare opened directory identities before exchange.

• internal/artifact/gc.go:303 — Symlinked artifact directories allow deletion outside the sync root. os.ReadDir follows symlinked artifact-kind directories, while candidates are later removed without root confinement at internal/artifact/gc.go:116. A writable shared folder could redirect GC into another directory and cause matching files to be deleted. Perform the entire scan-and-delete operation through an os.Root, reject symlinked origins and kind directories, and avoid reopening candidates by absolute path.

• internal/server/huma_routes_artifacts.go:251 — Repaired dependencies do not retry deferred imports. Peer import runs only after checkpoint or metadata uploads. If an import was deferred due to a missing or corrupt manifest or segment, uploading that repaired dependency does not retry it, potentially leaving the session absent indefinitely. Retry import after every successfully stored artifact or track incomplete checkpoints and retry when dependencies arrive.

• internal/artifact/sync.go:1346 — One semantically invalid checkpoint blocks broader synchronization. A JSON-decodable checkpoint that fails semantic or filename-sequence validation aborts the entire import instead of being quarantined and falling back to an older checkpoint. Quarantine invalid current-format checkpoints and continue scanning older ones, while continuing to defer unsupported future versions.

---

Reviewers: 2 done | Synthesis: codex, 15s | Total: 23m53s

[roborev-ci]

roborev: Combined Review (c7f2d4a)

One medium-severity issue prevents HTTP peers from publishing their own current sessions.

Medium

• internal/server/huma_routes_artifacts.go:91 — HTTP artifact routes only expose the existing artifact directory and never export current local database sessions. A running server can receive and redistribute peer artifacts but cannot publish its own new or updated sessions, despite documentation presenting it as a participating HTTP peer. Tests mask this by manually calling artifact.Export before exercising the routes.
  • Fix: Integrate artifact export and initial metadata baselining into the server lifecycle, refreshing artifacts after local sync and before peer enumeration. Alternatively, clearly document and restrict these routes as artifact-store-only endpoints requiring a separate publisher.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 7m18s

[roborev-ci]

roborev: Combined Review (3be2fcd)

Changes need one fix: conflict badges can become stale during an open session.

Medium

• frontend/src/lib/components/layout/SessionBreadcrumb.svelte:147 — Conflict data is permanently cached by session ID. If artifact import adds a conflict while the session remains open, an SSE-driven refresh reruns the effect but returns early, leaving the badge stale until navigation or reload. Invalidate or refetch conflicts on session refresh or relevant SSE events instead of caching solely by session ID.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 8m44s

[roborev-ci]

roborev: Combined Review (34062c6)

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 7m50s

[roborev-ci]

roborev: Combined Review (7fe9180)

Changes requested: two medium-severity synchronization correctness issues were identified.

Medium

• Shutdown can omit pending filesystem changes — cmd/agentsview/sync_watch.go:62
  The final shutdown push no longer performs discovery. Because the watcher drops pending batches during shutdown and stops only after the final push, changes still within its batching window can be missed.
  Fix: Run SyncAll for reasonShutdown as well as reasonInterval, using the shutdown flush context. Add a test covering a pending watcher change during shutdown.

• Peer synchronization status can be inaccurate — internal/server/huma_routes_artifacts.go:197
  Status is based on all non-deleted sessions for a machine instead of the exact sessions and manifest hashes in the peer’s latest checkpoint. Stale or unrelated rows may falsely show “In sync,” while locally trashed but fully imported rows may falsely appear pending.
  Fix: Determine landed state from each checkpoint’s exact GID-to-manifest mapping, preferably by comparing artifact import provenance with checkpoint manifest hashes.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 7m38s

[wesm]

I'm still working on this, there were a bunch of performance concerns. I am going to release 0.38.0 first and then see about this

[wesm]

I am still tracking this and will return to it in the near future

[maphew]

Thanks for the performance work, Wes. Since the 0.38 release is out, I can take the branch-maintenance pass: bring #731 up to current main, resolve the conflicts, address the two latest correctness findings, and post reproducible large-archive benchmark results. After that, would you prefer a staged PR series or to keep this together?

[maphew]

Following up: I kept #731 untouched and split the assist into three draft side PRs, each targeting the feature branch in my fork:

• Current-main catch-up — resolves the current merge conflicts and preserves the artifact-sync behavior across the newer DB/CLI/frontend shapes.
• Correctness fixes — flushes pending watcher changes during shutdown and derives peer status from the exact checkpoint provenance.
• Performance benchmarks — adds reproducible export/import/no-op/incremental workloads plus the measured large-archive baseline.

All three are mergeable and green across CI and the benchmark gate. The documented 200-session × 80-message (~6.1 MB) run on an i9-13900 measured about 0.95s export, 0.87s import, 0.58s converged no-op, and 0.67s incremental sync.

Use, cherry-pick, or choose whatever is helpful. I will not push changes to #731.

[roborev-ci]

roborev: Combined Review (719553a)

Artifact synchronization has five medium-severity issues involving unbounded work and memory, pin correctness, metadata convergence, and inconsistent exports.

Medium

• Unbounded artifact downloads allow peer-triggered memory exhaustion
  Locations: internal/artifact/transport_http.go:250, internal/artifact/transport_s3.go:419
  Both transports use io.ReadAll before validating hashes, compression, or decoded size. A malicious or compromised peer can return an arbitrarily large artifact and exhaust memory. Apply per-kind encoded-size limits, reject oversized Content-Length values, use a limited reader or bounded temporary file, and bound artifact-index and checkpoint response sizes/cardinality.

• Filesystem changes trigger archive-wide artifact exports
  Locations: cmd/agentsview/sync_watch.go:72, internal/artifact/sync.go:642
  Every debounced change enumerates, fingerprints, loads, and materializes checkpoint entries for all owned sessions, making per-event CPU and memory scale with total archive size. Pass changed session IDs into export, update checkpoints incrementally, handle deletions separately, and add a cardinality-scaling regression test.

• Duplicate source UUIDs break pin convergence
  Locations: internal/artifact/replay.go:289, internal/db/metadata_replay.go:670
  Pin convergence uses only source_uuid as its LWW field and resolves the first matching message. Independent pins can collapse, later duplicates can target the wrong message, and unpinning can remove all matches. Add a stable duplicate discriminator—such as an ordinal or UUID occurrence—to pin anchors and target selection, then delete only the resolved pinned message.

• Initial peer import can republish stale foreign metadata as local
  Location: internal/server/huma_routes_artifacts.go:226
  A server without an origin skips its baseline, imports peer artifacts, then snapshots imported rows as newer local metadata during the next discovery request. This can override an intervening source-peer edit. Capture the baseline before the first import, as the CLI path does, or exclude imported sessions from later baseline snapshots.

• Artifact export can combine data from different revisions
  Locations: internal/artifact/sync.go:660, internal/artifact/sync.go:920
  Session rows, messages, usage events, and related data are read through separate queries without a shared snapshot. Concurrent watcher sync can therefore produce an immutable manifest whose metadata and counts do not match its segments. Perform all export reads in one read transaction or serialize the complete export through the sync engine’s coordination primitive.

---

Reviewers: 2 done | Synthesis: codex, 15s | Total: 23m37s

[roborev-ci]

roborev: Combined Review (0082cca)

Medium-severity scalability issues remain in background artifact synchronization and garbage collection.

Medium

• cmd/agentsview/sync_watch.go:72, internal/artifact/sync.go:642 — Every watched filesystem batch enumerates and inspects all locally owned sessions, making a single-session change proportional to the entire archive. Propagate changed session IDs and export only that batch, with periodic full reconciliation as a fallback.

• internal/artifact/sync.go:1166, internal/artifact/replay.go:113 — Every sync materializes all applied metadata identities and scans all append-only metadata artifacts, causing time and memory use to grow indefinitely with history. Maintain an incremental per-origin inventory or cursor and query identities only for candidate artifacts while supporting late and out-of-order events.

• internal/artifact/sync.go:1269 — Manifests for locally trashed or excluded foreign sessions are repeatedly reread and decompressed because suppression is not recorded. Track a separate suppressed/pending manifest watermark, then clear or retry it when the session is restored or included.

• internal/artifact/gc.go:361 — Any artifact-directory change invalidates the GC cache, causing background GC to validate every live manifest and decompress every referenced segment. Persist and incrementally update live-reference accounting from changed checkpoints and manifests.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 16m43s

[wesm]

We've built a CAS system for msgvault that is in kenn-io/kit, I am working on generalizing that so that it can serve as the CAS layer for this PR, but it is going to take me some time, maybe another week. That will prevent creating an independent content-addressed storage system for this feature alone. Stay tuned

[roborev-ci]

roborev: Combined Review (cc88a44)

Artifact synchronization has two high-severity correctness/security issues and two medium-severity reliability issues.

High

• Artifact exports omit normal local sessions — internal/db/sessions.go:2318
  The export query hardcodes machine = 'local', but production sync stores the configured local machine name, typically the OS hostname. Normal artifact syncs therefore export no sessions; tests conceal this by seeding "local". Pass the configured machine name into the query and add an integration test using a non-"local" identifier.

• Unbounded artifact downloads can exhaust client memory — internal/artifact/transport_http.go:250, internal/artifact/transport_s3.go:395, internal/artifact/transport_s3.go:419, internal/artifact/sync.go:2962
  The transports buffer complete remote files with io.ReadAll before validating size, structure, or hash. A compromised peer, bucket, or shared folder can repeatedly supply an arbitrarily large artifact and cause memory exhaustion. Enforce per-kind size and checkpoint-cardinality limits, reject oversized Content-Length or file sizes, use io.LimitReader(limit+1), and stream legitimately large artifacts to temporary files while hashing.

Medium

• Standalone test targets fail in clean checkouts — Makefile:356, Makefile:380
  The test-minio and e2e targets compile Go code without first generating the ignored file required by the pricing package’s go:embed directive. Add pricing-snapshot as a prerequisite for both targets.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 14m30s

[wesm]

https://github.com/kenn-io/docbank is now public so I'm looking at how we could base the artifact management on docbank to reduce code ownership for CAS in agentsview