diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -6,8 +6,6 @@ See [the architecture reference](docs/architecture.md) for Appa's folder, device, member, peer, node, and capability terminology. -The [code-quality audit disposition](docs/code-quality-audit.md) records the -current design boundaries and hardening decisions. ## Install diff --git a/docs/code-quality-audit.md b/docs/code-quality-audit.md deleted file mode 100644 --- a/docs/code-quality-audit.md +++ /dev/null @@ -1,95 +0,0 @@ -# Code-quality audit disposition - -This document records the disposition of the August 2026 Appa code-quality -review. It is intentionally specific: a review finding is either fixed with a -test or code boundary, or rejected with the reason that it would make the v1 -code less clear without addressing a concrete failure mode. - -## Correctness and failure handling - -The following findings were accepted and fixed. - -- Forgetting a folder removes every folder-scoped table, including its signed - roster. The table list is centralized and covered by a storage test. -- Merkle delta requests treat a missing local child as different, allowing an - empty or small folder to receive a large remote manifest. An Iroh regression - test covers this case. -- Control-stream reads have a shared timeout. A malformed request is logged and - rejected without ending the peer's remaining control streams. -- Conflict-artifact recognition has one canonical implementation and requires - an Appa-shaped conflict name, rather than merely a matching substring. -- Filesystem paths are Unicode-only at the manifest boundary. Appa rejects - non-Unicode names rather than lossy-converting them into a different path. -- SQLite allocates version counters atomically with an UPSERT `RETURNING` - statement. -- Completion generation and systemd commands return before opening state. -- Iroh online waits, peer connects, control requests, blob operations, and - announcements are bounded. -- History reconstruction returns errors instead of assuming a checkpoint is - always present. -- Failed `appa service install` restores the prior unit file and reloads - systemd best-effort. - -## Structure and terminology - -The following findings were accepted and fixed. - -- The stable library surface is only `appa::run`; implementation modules are - private. The crate and architecture reference describe this boundary. -- Sync helpers use `SyncContext` for the folder, node, and state store. -- Merkle construction lives in `domain/merkle.rs`; reconciliation and roster - code have their own focused modules. -- Manifest-summary responses are named structs, not positional tuples. -- The transport implementation aliases the external Iroh crate as - `iroh_network`, avoiding ambiguity with Appa's internal `iroh` module. -- State schema statements are separated and folder-scoped deletion is explicit. -- CLI path helpers borrow `Path`, ticket parsing accepts a test reader, and - confirmation/removal behavior is shared. -- Per-peer synchronization is isolated from the scan/register/announce flow. -- The configuration action name and identity replacement choice are named at - their owning boundaries. - -## Diagnostics, operations, and format versions - -Public boundaries add context where it changes diagnosis: state opening, -filesystem scanning and watching, local process locking, blob-store creation, -and systemd rollback. We intentionally do not wrap every internal `?`: the -operation boundary already identifies the failed resource, while repeated -context inside a small helper would obscure the original error. - -The architecture reference defines Folder, Device, Member, Peer, Node, and -Capability. It also records the independent control-stream, invitation, -folder, and configuration version boundaries. Local state intentionally has no -migration ladder: this pre-release v1 starts from fresh state instead of -shipping compatibility paths for unpublished formats. - -## Evaluated but not adopted - -These review recommendations were considered and intentionally not applied. - -- **Domain newtypes for every string.** Device IDs, blob hashes, capabilities, - and manifest paths cross serde, SQLite, Iroh, and CLI boundaries. A wholesale - wrapper conversion would add pervasive boilerplate without preventing the - two identified bugs; those bugs are fixed directly (character-safe conflict - suffixes and non-Unicode path rejection). Introduce a newtype only when a - new invariant needs one. -- **One duration type everywhere.** `time::Duration` is used only for calendar - expiry arithmetic with `OffsetDateTime`; `std::time::Duration` is used for - I/O limits; Tokio accepts the standard duration for async scheduling. These - types represent distinct APIs, not conflicting units. -- **A storage abstraction over SQLite.** `StateStore` is the storage boundary. - Adding a second repository layer would only forward SQLite queries. Schema - and folder-scoped ownership are already separated; future alternate storage - can justify a trait when it exists. -- **A wire version in every control message.** ALPN selects the compatible - control protocol before decoding a message. Adding redundant per-message - fields would not improve compatibility or authorization. -- **Compatibility migrations.** They are explicitly out of scope while Appa - begins from clean local state. - -## Continuing review rule - -Keep functions focused, use named structs at cross-module boundaries, add -resource context at failure boundaries, and add a regression test for every -sync or state-integrity fix. Do not add abstractions merely to satisfy a style -metric; add them when they make a real invariant or ownership boundary clear.