Repository navigation
agenticd architectural-analysis follow-ups (2026-05-21) #34
Description
Tracking issue for the 11 architectural recommendations produced by the 2026-05-21 architectural analysis of crates/agenticd/.
Audit doc: docs/ops/2026-05-21-agenticd-architectural-analysis.md — durable evidence behind each child issue.
Ship order
The audit assigns priority across three tiers; ship in this order:
Must fix before v1.0 (milestone: v1.0, label: must-fix-v1.0)
These three contradict load-bearing ADR claims or break the broken-prompt demo. The audit's executive summary calls all three out by name.
- A1 — Quiesce trigger poller during memory restore — A1: Quiesce trigger poller during memory restore #35. Addresses C8 / R1. Without this, rollback's atomicity claim is silently false.
- A2 — Lifecycle module: SIGTERM drain + startup ref reconciliation — A2: Lifecycle module — SIGTERM drain + startup ref reconciliation #36. Addresses C2, B7 / R2. Without this, process kill mid-2PC leaves refs and Postgres divergent.
- A8 — Reverse-migration outer transaction + restore-guard fix + wire
accept_data_loss— A8: Reverse-migration outer transaction + restore-guard fix + wire accept_data_loss #37. Addresses B8, B9, B10 / R4, R5. Without this, memory restore can be silently skipped and partial migration orphans the schema.
Hardening sprint (label: hardening-sprint, no milestone)
These are real risks but don't block the demo or contradict a load-bearing ADR. Ship in the hardening sprint window between v1.0 ship and v1.1 kickoff.
- A3 — Extract
handle_commitinto acommit.rsorchestrator with named 2PC phases — A3: Extract handle_commit into commit.rs orchestrator with named 2PC phases #38. Addresses S2, B4, B5, B7-partially. Mirrors the shaperollback::executealready has. - A4 — Split
rollback.rsintomod/loaders/writebacksubmodules — A4: Split rollback.rs into mod / loaders / writeback #39. Addresses S3, S5, S7, R6, R11. - A5 — Move GCS blocking I/O off LocalSet via
spawn_blocking(tactical) — A5: Move GCS blocking I/O off LocalSet via spawn_blocking (tactical) #40. Addresses C1, B2, B3 / R3. Full async-trait fix lives in ADR-0011. - A6 — Structured
Response::Errorvariant + framing-error envelope — A6: Structured Response::Error + framing-error envelope (daemon-side; blocked by ADR-0010) #41. Addresses B6, B13, B14 / R7. Wire-shape is defined by ADR-0010. - A7 — Parallelise MCP fingerprinting with
FuturesUnordered— A7: Parallelise MCP fingerprinting with FuturesUnordered #42. Addresses B1, C3 / R9.
v1.1 (label: v1.1, no milestone)
These are blocked by other Proposed ADRs or are aesthetic-only. They land when their unblocker lands.
- A9 — Complete
MemoryAdaptertrait so non-Postgres backends can plug in — A9: Complete MemoryAdapter trait so non-Postgres backends can plug in (v1.1) #43. Addresses S1, S4, S6 / R10. Blocked by ADR-0005 (Proposed). - A10 — Add
SegmentManifest::from_canonical_bytesto mirrorto_canonical_bytes— A10: Add SegmentManifest::from_canonical_bytes to mirror to_canonical_bytes #44. Addresses S4. Aesthetic; reopen on next manifest schema change. May fold into A9: Complete MemoryAdapter trait so non-Postgres backends can plug in (v1.1) #43. - A11 —
Diffatomicity: snapshot both refs under a single read — A11: Diff atomicity — snapshot both refs under a single read (v1.1) #45. Addresses C6. Blocked by ADR-0007 (Proposed) — ephemeral-branch semantics first.
How to read the audit
Each issue body links back to the audit at a specific anchor (#a1, #a2, …) for the full upstream finding chain. The audit captures 7 structural findings (S1–S7), 15 behavioral findings (B1–B15), 13 concurrency findings (C1–C13), 11 risk items (R1–R11), and 12 architectural recommendations (A1–A12, where A12 is intentionally-not-addressed).
What is NOT in this batch
- System-level concerns (wire-protocol redesign, full async-trait redesign, cross-daemon coordination) — these need their own ADRs and live in ADR-0010 + ADR-0011 (both Proposed; ADR-0010 (wire-protocol error model) + ADR-0011 (ObjectStore async-trait shape) #33). The audit's "System-level concerns deferred" section names them.
- Findings the analyst marked correct/benign (lock ordering, advisory-lock cancellation safety, panic isolation, content-addressed TOCTOU) — no issues for these; they document strengths.
Activity
tonibergholm commentedon May 21, 2026
A8 (#37) → PR #46 open. 5 commits on
fix/a8-reverse-migration-tx-and-restore-guard. 17 tests pass (10 unit migrate, 4 unit rollback validation, 3 integration against real Postgres). Audit §A8 marked done.Remaining must-fix-v1.0:
tonibergholm commentedon May 21, 2026
A1 (#35) → PR #48 open. Pre-fix the demo's atomic-rollback claim was silently false; post-fix a write-100-rows-during-restore integration test verifies the streamer's view stays consistent with the table.
Remaining must-fix-v1.0:
tonibergholm commentedon May 21, 2026
A2 (#36) → PR #50 open. Last of the three
must-fix-v1.0items. After merge, the audit's three must-fix items are all closed.Remaining must-fix-v1.0:
Next up: the hardening-sprint tier (A3-A7), then v1.1 (A9-A11). 8 unit tests for the lifecycle module + 1 for the new refs helper; clippy/fmt clean; A1+A8 integration tests still pass.
tonibergholm commentedon Jul 10, 2026
Close-out: all 11 recommendations verified landed on
mainCode-grounded verification at
main=75c29de(2026-07-10): each artifact below was confirmed to exist in code, not just in a merged PR description.Quiesceable/QuiesceTokenagentic-memory/src/triggers.rs:30,40;RestoreGuard+begin_restore/restore_with_guardadapter.rs:54,138,155;TRUNCATE agentic_change_login restore txrestore.rs:57,116; testtests/integration.rs:246Lifecycleagenticd/src/lifecycle.rs:40-92;reconcile_refs_on_startupbefore bindmain.rs:245,269; select!-drainmain.rs:308-376; B7 deferred HEAD publishcommit.rs:238-247;Refs::list_branchesagentic-core/src/refs.rs:137agentic-memory/src/postgres.rs:657-682; restore-guard rejectionagenticd/src/rollback/mod.rs:452-454;accept_data_losswired proto→cli→server→rollback→migrate (agentic-proto/src/lib.rs:82,agenticd/src/migrate.rs:53,106); Postgres integration teststests/reverse_migration.rscommit.rsorchestrator with named 2PC phasesagenticd/src/commit.rs:57-124; D3 order inagentic-core/src/commit.rs:104-144;server.rs:504delegatesagenticd/src/rollback/{mod.rs:207, loaders.rs:16-41, writeback.rs:44-205}spawn_blockingshimagenticd/src/store_async.rs:44-85, call sitescommit.rs:154,server.rs:552— tactical shape as scoped (ADR-0011 successor still Proposed)Response::Erroragentic-proto/src/lib.rs:179-187; error mappingagenticd/src/wire_error.rs:30; framing envelope + v0 shimserver.rs:351,418,441agenticd/src/mcp.rs:147-153viastream::iter(...).buffered(FINGERPRINT_CONCURRENCY=8)— deliberate ordered-stream deviation fromFuturesUnordered, documented in audit §A7apply_reverse_migrationson traitagentic-memory/src/adapter.rs:167;Option<Arc<dyn MemoryAdapter>>no-mutexagenticd/src/server.rs:70;MemoryBackendSpecmembackend.rs:18-27; in-memory rollback e2etests/rollback_in_memory.rs:52-77SegmentManifest::from_canonical_bytesagentic-memory/src/segment.rs:157; loaders route through itagenticd/src/rollback/loaders.rs:41-45Refs::snapshot()/RefsSnapshotagentic-core/src/refs.rs:183,219; diff takes snapshot undercommit_lockthen resolves from frozen mapagenticd/src/server.rs:533-536,616-627One doc nit found during verification: audit §A1 says
RestoreGuardlives inrestore.rs; it is defined inadapter.rs:54(restore.rsconsumes it). Code is correct; noting here rather than churning the doc.Deferred system-level concerns were split out via #33 into ADR-0010 (Accepted; implemented via #74) and ADR-0011 (Proposed; A5's tactical
spawn_blockingshim covers it meanwhile). A12 remains intentionally-not-addressed per the audit (documents strengths / benign findings, not work).Audit doc marker sync: #125 (follow-up table's A9 row lagged its section header).
Closing as completed.
Close-out records for tracking issue #34: spec, plan, audit-table A9 …