Add StorageKit: containerized, mode-aware storage by kyleve · Pull Request #47 · kyleve/Stuff

kyleve · 2026-06-29T20:52:29Z

Opened by an AI agent on the repo owner's behalf.

Summary

Adds StorageKit, a new library for containerized, mode-aware storage.

Tree of containers. A StorageSystem (config + vending point) roots a tree of StorageContainers. Each container owns one directory and vends child containers (subdirectories), raw file URLs, a namespaced key-value store, and an isolated SwiftData ModelContainer (its .store + -wal/-shm live in a dedicated child dir, so it's deletable as a unit). Vends are cached and idempotent.
One switch for in-memory vs. persisted. StorageMode (.persistent / .inMemory) makes the same call sites do the right thing — UserDefaults suite vs. in-memory KV, on-disk store vs. isStoredInMemoryOnly with CloudKit forced off and no disk access. App/model code is identical in both modes.
Registration-based, children-first teardown with two paths — reversible deactivate() and destructive deleteContainer() (plus deleteContents() and system-wide deleteAll()) — and three hooks: onDeactivate, prepareForDeletion, afterDeletion. Everything before the irreversible delete is park-safe (a throw aborts with nothing deleted; retry is safe), and a post-commit afterDeletion throw retries only that step.

State is modeled as one enum (active / inactive / deleting(wasActive:) / deleted) rather than loose flags.

Hardening from code review

The module was reviewed for threading and teardown; this branch folds in the fixes (each its own commit):

F1 — invalidate the parent's cached ModelContainer when its backing child is removed (was silent data loss after deleteContents()).
F2 — namespace KV suites by a stable hash of the resolved base directory (was a global suite that clobbered across instances and flaked parallel tests).
F3 — reentrancy-safe teardown: the subtree is frozen deleting up front so a racing vend can't resurrect a directory mid-delete; a pre-commit throw reverts to the exact prior resting state, and onDeactivate fires exactly once across deactivate-then-delete.
F4 — documented (kept) the deliberately non-throwing keyValueStore() trap so callers don't try/catch every access.
F5 — deregister a deleted node from its parent before afterDeletion, so a failed post-commit step still frees the key to re-vend a fresh node.
Minor notes — schema-mismatch detection on model-store re-vend, in-memory stores stay off disk, node-scoped teardown tokens, plus docs for the store/child-key namespace collision and fileURL(_:)'s state-unaware nature.

Test plan

tuist test StorageKitTests on iPhone 17 / iOS 26.2 — full suite green
Concurrency guard (concurrentVendsDoNotResurrectADeletedContainer) run 5× for stability
swiftformat --lint clean
CI green on macos-26

Made with Cursor

Add the Shared/StorageKit library product + target in Package.swift and the StorageKitTests hosted unit-test bundle + scheme in Project.swift, plus the StorageMode enum so the new target builds. Pure groundwork — no behavior yet. (plan step: scaffold) Co-authored-by: Cursor <cursoragent@cursor.com>

Add the mode-aware container tree and its full behavior: - StorageKey (sanitized path component), BaseDirectory (standard / custom roots + subdirectory namespacing), StorageError, and the StorageSystem root actor that vends top-level StorageContainers. - StorageContainer: recursive, cached/idempotent child vending, fileURL, a namespaced KeyValueStore (UserDefaults suite vs. in-memory), and an isolated SwiftData ModelContainer per node (its own child dir; isStoredInMemoryOnly + CloudKit-off in .inMemory). - Two children-first teardown paths: reversible deactivate() and destructive deleteContainer() (prepareForDeletion -> onDeactivate -> delete + suite purge -> afterDeletion), park-safe before the delete, plus deleteContents(). Sources and the StorageKitTests suite land together because StorageContainer depends on every source type and the hosted test bundle needs its sources. 41 Swift Testing cases cover the tree, modes, vends, sanitization, and teardown ordering/park-safety. (plan steps: core-tree, keyvalue, modelcontainer, teardown, tests) Co-authored-by: Cursor <cursoragent@cursor.com>

Document the module: the root/node split, mode-aware vends, isolated SwiftData stores, and the deactivate vs. deleteContainer teardown paths with their park-safe guarantee. AGENTS.md captures the invariants to preserve (cached idempotent vends, in-memory parity, deletion ordering, suites purged on delete) and the test patterns. CLAUDE.md is generated by ./sync-agents and gitignored. (plan step: docs-generate) Co-authored-by: Cursor <cursoragent@cursor.com>

`deleteContents()` (and any direct child deletion) deletes a model store's child directory but left the parent's `modelContainerCache` entry in place, so a re-vend handed back a `ModelContainer` pointing at deleted files — silent data loss. Clear `modelContainerCache[childKey]` in `removeChild`, the single choke point every detach flows through. Fixes review finding F1. Co-authored-by: Cursor <cursoragent@cursor.com>

The root UserDefaults suite name was derived from the system name alone, so two systems with the same name but different base directories shared one global suite — clobbering each other in production and making parallel tests flaky (one test's purge wiping another's data). Fold a stable (FNV-1a, base-36) hash of the resolved root URL into the root suite name; children inherit it. Persistent because Hasher is per-process-seeded and would break persistence across launches. Fixes review finding F2. Co-authored-by: Cursor <cursoragent@cursor.com>

deleteContainer() / deleteContents() suspend at await points between removing a directory and marking the node deleted. A concurrent vend landing in that window would call activate() and recreate the very directory being torn down, leaving an orphaned tree on disk. Add a `deleting` state and mark the whole subtree (self before recursing, so no new child slips past the snapshot) before running any handler. While deleting, activate() throws and keyValueStore() traps, so racing vends are rejected instead of resurrecting the directory. A pre-commit throw reverts the freeze back to active (still park-safe); committing the delete advances to deleted. Fixes review finding F3. Co-authored-by: Cursor <cursoragent@cursor.com>

keyValueStore() intentionally stays non-throwing so reads don't need a try, at the cost of trapping when vended on a deleting/deleted container. That's a deliberate trade-off (F4): rather than make every access try/catch, spell out that the trap signals a lifecycle bug, that callers should hold the store through a live handle and release it on teardown, and that container(_:)/modelContainer(for:) are the throwing alternatives. Doc comment + README + AGENTS only; no behavior change. Addresses review finding F4 by documentation, as agreed. Co-authored-by: Cursor <cursoragent@cursor.com>

If an afterDeletion handler threw, detachFromParent() was skipped, so the node lingered in its parent's children map marked deleted — and a re-vend of that key returned the dead node, which threw on every use. Move the parent deregistration into the delete commit (before afterDeletion, per the documented phase order) and split out releaseChildren() so the post-commit step can still recurse children before they're dropped. A failed afterDeletion now leaves the key free to re-vend a fresh node. Fixes review finding F5. Co-authored-by: Cursor <cursoragent@cursor.com>

The F3 freeze collapsed active and inactive into one deleting state, losing the "already deactivated?" bit. So deleting a node that had been deactivate()d re-fired its onDeactivate handlers (idempotent, but surprising), and a parked delete of an inactive node reverted to active rather than inactive. Carry the pre-freeze state in the case itself — deleting(wasActive:) — so onDeactivate fires exactly once (skipped when the node was already inactive) and a pre-commit throw reverts to the exact prior resting state. prepareForDeletion/afterDeletion are unchanged: they're delete- only and still always run at their points. State gains an explicit Equatable since the associated value drops the implicit conformance. Refines review finding F3. Co-authored-by: Cursor <cursoragent@cursor.com>

modelContainer(for:named:) cached only by name, so a second call under the same store name with a different type set silently returned the first container — a schema mismatch the caller never sees. Cache the type identities alongside the container and throw a new StorageError.modelStoreSchemaMismatch on a mismatched re-vend; an identical type set still returns the cached instance. Addresses review minor note (model cache ignores types). Co-authored-by: Cursor <cursoragent@cursor.com>

modelContainer() vended the store's child container before the mode switch, so even .inMemory mode created an empty store directory on disk. Move the child vend into the .persistent branch; in-memory builds its isStoredInMemoryOnly configuration without touching the filesystem. Test now also asserts the store directory itself is absent. Addresses review minor note (in-memory still creates a store directory). Co-authored-by: Cursor <cursoragent@cursor.com>

Token ids start at 0 per node, so deregister(_:) keyed on id+phase alone could remove an unrelated handler when handed a token from a different container (same id, same phase). Stamp each Token with its issuing node's ObjectIdentifier and ignore a foreign token in deregister(_:). Addresses review minor note (Token is not node-scoped). Co-authored-by: Cursor <cursoragent@cursor.com>

The default model store name "store" lives in the same key namespace as container(_:), so container("store") and the default model store would share a directory. Spell out the collision (and the distinct-named: guidance) in the modelContainer doc comment, README contracts, and AGENTS. Doc only. Addresses review minor note (default "store" name collides with a child). Co-authored-by: Cursor <cursoragent@cursor.com>

fileURL(_:) is nonisolated and can't see the node's state, so on an inactive or deleted node it hands back a URL into a missing directory. Call out the sharp edge in its doc comment and the README contracts so callers only use the URL while the container is live. Doc only. Addresses review minor note (fileURL on inactive/deleted nodes). Co-authored-by: Cursor <cursoragent@cursor.com>

Fill the remaining gaps from the review (the F1 and F2 gaps were already covered by their fixes' tests): - deactivate() drops the vend caches, and reactivation re-vends a fresh ModelContainer over the surviving on-disk store while the KV suite round-trips. - deleteContents() runs each child's full teardown hook sequence while the node stays live. - concurrent vends racing deleteContainer() never resurrect the deleted directory (regression guard for the F3 deleting-state freeze). Co-authored-by: Cursor <cursoragent@cursor.com>

kyleve · 2026-07-01T00:22:17Z

+///
+/// `subdirectory` (optional) namespaces under the standard directory so a system
+/// isn't created at the very top of Application Support / Caches; `custom(_:)` is
+/// for tests and relocations. `resolvedURL(using:)` is public so a caller can


"custom(_:)` is for tests and relocations" feels a bit off; I think there's more directory types than just application support and caches (eg documents), can we include those too so custom is really for exceptional cases?

Posted by an AI agent on the repo owner's behalf.

Agreed — added .documents and .library alongside applicationSupport / caches, and reworded custom(_:) so it's explicitly the escape hatch for exceptional cases (tests, relocations, an App Group / security-scoped URL). (b36a431)

kyleve · 2026-07-01T00:35:36Z

Hmm thinking out loud, this file is getting very long, with a lot of concerns in it (which makes sense, I asked for them)!

I wonder if we should move towards a model where the StorageContainer is more of a generic type, that also is extendable by subtypes that are held by the parent container, similar to how environment works in SwiftUI. Thinking like:

StorageContainer is the generic, general, boring base type

It can have children that conform to something like "StorageContainerExtension"; that is where we'd move SwiftData handling, UserDefaults handling, file storage handling, etc.

So then you'd call container.swiftdata.makeContainer(....) and so on

This would be defined in such a way (extension on StorageContainer) that StorageContainer has opaque storage that holds these types.

Does this make sense?

Posted by an AI agent on the repo owner's behalf.

Split the storage concerns off the base type into typed namespaces, each in its own file: container.files.url(_:) (StorageContainer+Files), container.keyValue.store() (+KeyValue), and container.swiftData.modelContainer(for:) (+SwiftData). The core file now carries only lifecycle, the child registry, and teardown.

One deviation from the "opaque storage holds these types" shape you sketched: a separate extension object can't share the actor's isolation domain to touch its caches synchronously, so instead each namespace is a small Sendable facade that forwards into an actor-isolated make… method, with the caches staying on the actor. The call site is the one you wanted (container.swiftData.modelContainer(…)), and you add your own namespace the same way — extend StorageContainer with a nonisolated accessor returning a facade that composes the public primitives (child containers, files, teardown hooks). Rationale is in AGENTS.md. (dba67ec)

kyleve · 2026-07-01T00:39:23Z

+        name
+    }
+
+    private static func sanitized(_ raw: String) -> String {


Is there no built-in way to do this with iOS apis?

Posted by an AI agent on the repo owner's behalf.

Not that I could find — Foundation has no "make a safe single path component" API, and the near-misses all fall short: addingPercentEncoding is URL-oriented (it leaves ./.. intact and %-mangles otherwise-fine names), URL(fileURLWithPath:).lastPathComponent doesn't neutralize traversal, and FileManager offers no name sanitizer. Kept the hand-rolled scalar pass and added a comment saying why. (69540dc)

kyleve · 2026-07-01T00:41:59Z

+/// The split costs one extra directory level on disk (the layout is an invisible
+/// implementation detail) in exchange for keeping "what do I configure" separate
+/// from "what do I store".
+public actor StorageSystem {


I don't love this name, but something like "storage root" also seems not much clearer; thoughts?

Posted by an AI agent on the repo owner's behalf.

Kept StorageSystem for now. The alternatives weighed: Storage (reads nicely as the entry point — let storage = try Storage(…) — but generic) and StorageRoot (pairs with StorageContainer). Cheap to rename later if one grows on you, since nothing outside tests consumes it yet.

kyleve · 2026-07-01T00:43:02Z

+    public init(
+        _ name: StorageKey,
+        mode: StorageMode,
+        base: BaseDirectory = .applicationSupport(),


Remove default, should need to be specified explicitly

Posted by an AI agent on the repo owner's behalf.

Done — and went a step further and folded the base into the mode: StorageMode.persistent(base:) / .inMemory. On-disk storage now can't be requested without saying where it lives, .inMemory can't carry a base it would ignore, and the base is a required, explicit choice at the call site. (fe0530c)

kyleve · 2026-07-01T00:45:59Z

+    /// A deterministic FNV-1a hash, rendered base-36. `Hasher`/`hashValue` is
+    /// seeded per process, so it can't be used here — the suite name must stay
+    /// stable across launches for persistence to survive.
+    private static func stableHash(_ string: String) -> String {


Is this useful vs just idk, using the URL string itself as the suite name?

Posted by an AI agent on the repo owner's behalf.

The raw URL string can't be the suite name directly: a UserDefaults suite becomes a plist filename (~/Library/Preferences/<suite>.plist), so an absolute path with / is invalid, and even a sanitized full path would be long and would leak the user's home directory into a global preferences filename. The FNV-1a hash is a compact, stable (unlike hashValue, which is per-process seeded) disambiguator that keeps the readable name up front. Expanded the doc comment to spell this out. (69540dc)

Broaden the standard search-path options so `custom(_:)` is reserved for genuinely exceptional bases (tests, relocations, App Group / security-scoped URLs) rather than standing in for common directories like Documents. Addresses PR review: "there's more directory types than just application support and caches (eg documents), can we include those too so custom is really for exceptional cases?" Co-authored-by: Cursor <cursoragent@cursor.com>

Make the persistence base an associated value of `.persistent(base:)` instead of a defaulted `StorageSystem.init` parameter. On-disk storage now can't be requested without saying where it lives, `.inMemory` can't carry a base it would ignore, and the base is a required, explicit choice at the call site. Addresses PR review: "Remove default, should need to be specified explicitly." Co-authored-by: Cursor <cursoragent@cursor.com>

Comment-only: explain that Foundation has no built-in safe-path-component API (so StorageKey sanitizes by hand), and why the root UserDefaults suite name hashes the base URL rather than embedding the raw path (a suite name is a plist filename, so the absolute path can't be used directly). Addresses PR review questions on StorageKey.sanitized and the suite-name hash. Co-authored-by: Cursor <cursoragent@cursor.com>

Move the raw-file, key-value, and SwiftData vends off the StorageContainer base type onto small Sendable facade namespaces — `container.files.url(_:)`, `container.keyValue.store()`, `container.swiftData.modelContainer(for:)` — each in its own `StorageContainer+…` file with its actor-isolated backing. The core file now carries only lifecycle, the child registry, and teardown, and new storage concerns can be added the same way (an extension returning a facade). The facades forward into actor-isolated `make…` methods whose caches stay on the actor (an opaque type-keyed registry would fight actor isolation), so behavior and the cached/idempotent vends are unchanged. Addresses PR review: StorageContainer.swift was long and mixed many concerns. Co-authored-by: Cursor <cursoragent@cursor.com>

kyleve and others added 15 commits June 28, 2026 22:37

kyleve commented Jul 1, 2026

View reviewed changes

kyleve and others added 4 commits June 30, 2026 21:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add StorageKit: containerized, mode-aware storage#47

Add StorageKit: containerized, mode-aware storage#47
kyleve wants to merge 19 commits into
mainfrom
storagekit-module

kyleve commented Jun 29, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

kyleve Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

kyleve commented Jun 29, 2026

Summary

Hardening from code review

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant