From 2462ed7fe025f9e1c4bbdd06894c82c6ba9272f3 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:02:39 -0400 Subject: [PATCH 01/19] chore(plans): add marshal-drop-nullish plan (#232, #233) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- plans/marshal-drop-nullish.md | 132 ++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 plans/marshal-drop-nullish.md diff --git a/plans/marshal-drop-nullish.md b/plans/marshal-drop-nullish.md new file mode 100644 index 0000000..8de7748 --- /dev/null +++ b/plans/marshal-drop-nullish.md @@ -0,0 +1,132 @@ +--- +status: in-progress +depends: [] +specs: + - specs/behaviors/normalization.md + - specs/rust-core.md +issues: [232, 233] +--- + +# Plan: drop null/undefined-valued keys at the marshal boundary + +## Scope + +Restore the 1.x `@iarna/toml` semantics the Rust-core cutover silently broke +([#232](https://github.com/JarvusInnovations/gitsheets/issues/232)): a +`null`/`undefined`-valued key in a record must be **dropped** on write, not +rejected with `cannot marshal JS value of type Null/Undefined`. The drop is +recursive (nested tables, and tables inside arrays) and applies identically in +every binding. A null **array element** stays an error — but with a targeted, +actionable message (see Approach for the rationale). + +Riding along ([#233](https://github.com/JarvusInnovations/gitsheets/issues/233)): +the missing 1.x → 2.x breaking-changes section in `docs/migration-guide.md` +(hologit drop, null handling, canonical byte re-baseline). + +Out of scope: + +- Merge-patch marshalling (`applyMergePatch` / `MergePatch`), where `null` is + the RFC 7396 delete sentinel — a separate, already-correct code path. +- Any `Value::Null` variant in the core value type. The core stays + TOML-faithful; nulls are a host-language concept resolved at the marshal + boundary (see Approach). +- Null-aware query-filter semantics (e.g. `{ field: null }` matching absent + fields) — a filter leaf of `null` keeps erroring, as it does today. + +## Implements + +- `specs/behaviors/normalization.md` — the (already-specced, newly-explicit) + rule that null values are omitted from output: "Null / undefined handling" + subsection under TOML serialization details. +- `specs/rust-core.md` — the new "Nulls" type-fidelity rule for the marshal + boundary every binding implements. + +## Approach + +1. **Spec first**: make the existing one-liner ("Null values are *omitted* from + the output") explicit and complete in `specs/behaviors/normalization.md` — + recursive drop for table keys, error for array elements, the absent-key == + cleared-optional equivalence — and add the matching "Nulls" bullet to + `specs/rust-core.md`'s type-fidelity rules. +2. **Where the fix lives**: the host→core marshal in the two Rust binding + crates — `JsValue::from_napi_value` in `rust/gitsheets-napi/src/lib.rs` and + `py_to_value` in `rust/gitsheets-py/src/lib.rs`. The core `Value` enum is + deliberately TOML-faithful (no null variant), so the core never sees a null + by construction; the marshal recursion is the single place each binding can + see one. The rule itself is core-owned: `gitsheets-core` exports the shared + error-message helper so the rejection reads identically from Node and + Python, and the spec is the cross-binding contract, enforced by the existing + cross-binding byte-parity CI job. +3. **Table entries**: a `null`/`undefined` (JS) or `None` (Python) property + value drops the key, recursively — the record serializes as if the key were + never set, matching 1.x bytes exactly. +4. **Array elements**: reject with a dedicated error naming the index. TOML + arrays cannot contain nulls, and silently *dropping an element* — unlike + dropping a key — shifts sibling indices and changes data. This is a + **deliberate divergence from 1.x**: `@iarna/toml@2.2.5` silently dropped + null array elements (`[1, null, 2]` → `[1, 2]`), which is exactly the kind + of silent data mutation the bytes-authority refuses. Spec + migration guide + call the divergence out explicitly. +5. **Top-level / scalar-position nulls** (a whole record, a filter leaf): keep + erroring, with the clarified message. +6. **Tests at every layer**: core unit test for the shared message helper; napi + boundary tests (roundtrip drop, byte-equality with the pre-stripped record, + array-element error, upsert-through-`record_write`); Python mirror tests + plus a cross-binding byte-parity case with nullish keys; package-layer + vitest covering the real consumer pattern (Standard Schema validator + normalizing cleared optionals to `null` → upsert succeeds → serialized + record has no such keys). +7. **Migration guide (#233)**: new "v1.x → v2.0 (Rust core)" section — hologit + dependency drop (`BlobObject.write`/`hologitRepo` → `repo.writeBlob` + + `setAttachments` before/after), null handling (initially threw, fixed in + this release; array-element edge case), canonical byte re-baseline per #196 + with the one-time re-serialize commit recipe. + +## Validation + +- [ ] `cargo test` green across the workspace (core + bindings build, clippy + `-D warnings` clean) +- [ ] napi suite (`npm test` in `rust/gitsheets-napi`) green, including new + cases: null/undefined table keys dropped recursively (nested tables, tables + inside arrays), serialized bytes identical to the pre-stripped record, null + array element rejected with an error naming the index, top-level null still + rejected +- [ ] Python suite (`pytest` in `rust/gitsheets-py`) green, including the + mirrored `None` cases and a cross-binding byte-parity case for a record + with nullish keys +- [ ] Package vitest suite (`npm test`) green, including the end-to-end + consumer pattern: `.nullable().optional()`-style Standard Schema validator + emitting `null` for cleared optionals → `upsert` succeeds → read-back has no + such keys → on-disk TOML bytes contain no trace of them +- [ ] `docs/migration-guide.md` gains the 1.x → 2.x section enumerating the + three breaks from #233 with before/after for each + +## Risks / unknowns + +- **Blanket drop in the generic record marshal** also affects `roundtrip`, + `serializeRecords`, comparator args, `createPatch` src/dst, and + `recordQueryCandidates` partial records. This is intended: all of those + surfaces were plain JS in 1.x where a null-valued key was either dropped at + serialize time or behaved as absent. Filter leaves are unaffected (filters + recurse host-side and only marshal scalar/array leaves). +- **Merge-patch adjacency** — the null-drop must not leak into + `MergePatch` marshalling where null means delete. Covered by existing + `apply_merge_patch` tests in both bindings. + +## Notes + +- 1.x array-element behavior verified against `@iarna/toml@2.2.5`: + `stringify({ a: [1, null, 2] })` silently emits `a = [ 1, 2 ]` — a silent + element drop that shifts indices. 2.x deliberately rejects instead; only the + key-drop is restored as 1.x-compatible. +- JS sparse-array holes (`[1, , 2]`) read back as `undefined` and hit the same + array-element rejection — deliberate, since dropping them would also shift + indices. +- The shared rejection-message helpers live in `gitsheets-core::value` + (`null_array_element_msg` / `null_scalar_msg`) so both bindings and any + future one emit identical diagnostics; the drop recursion itself is + ~6 lines per binding inside each marshal. + +## Follow-ups + +None. From 7197a16b75c291cf75527aa80ea42cd8f480e130 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:02:39 -0400 Subject: [PATCH 02/19] docs(specs): spec null/undefined handling at the marshal boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the existing one-liner ('Null values are omitted from the output') explicit and complete: null/undefined-valued table keys are dropped recursively (nested tables, and tables inside objects within arrays) before validation and serialization, in every binding — restoring the 1.x @iarna/toml drop semantics that the Rust-core cutover regressed (#232). A null array *element* is a typed error naming the index: TOML arrays cannot hold nulls, and silently dropping an element (as 1.x did) shifts sibling indices — a silent data mutation the bytes-authority refuses. Absent key == cleared optional field is the specced equivalence; required-field nulls fail validation as missing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- specs/behaviors/normalization.md | 39 +++++++++++++++++++++++++++++++- specs/rust-core.md | 11 +++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/specs/behaviors/normalization.md b/specs/behaviors/normalization.md index 837c6ad..0007371 100644 --- a/specs/behaviors/normalization.md +++ b/specs/behaviors/normalization.md @@ -103,9 +103,46 @@ Sorts an array of strings using locale-aware comparison (`localeCompare` with `s - The **canonical serializer** is the Rust core's `gitsheets-core::canonical::serialize` — the [`toml` crate](https://docs.rs/toml)'s default formatting applied to a deep-key-sorted value. It defines the canonical bytes: a value is lowered to `toml::Value` (whose `Table` is a `BTreeMap`, so the deep key sort happens structurally) and emitted with the crate's default rendering (triple-quoted multiline strings, literal-quoted strings where possible). Parsing is the same crate's parser. See [architecture.md](../architecture.md) and [rust-core.md](../rust-core.md#canonical-form-rebaseline). - Dates, datetimes, and date-times round-trip by native TOML datetime type — all four kinds (offset date-time, local date-time, local date, local time) parse to the core `Datetime` and serialize back to the matching TOML type byte-faithfully. - Multi-line string formatting follows the `toml` crate's defaults — strings with newlines emit as triple-quoted (`"""…"""`); the library imposes no length threshold of its own. -- Null values are *omitted* from the output. TOML can't represent `null`; absent fields read back as `undefined` and are treated as null by validation. +- Null values are *omitted* from the output — see [Null / undefined handling](#null--undefined-handling) below for the full rule. - Empty arrays and empty objects are preserved (they have semantic meaning distinct from absent). +### Null / undefined handling + +TOML has no `null`. A cleared optional field and an absent key are the **same +state** — there is exactly one on-disk representation for "this field has no +value", and that is the key not existing. The rules, applied at the host-value +marshal boundary in **every** binding (JS `null`/`undefined`, Python `None`), +before the record reaches validation or serialization: + +1. **Table keys — dropped, recursively.** A key whose value is null/undefined + is dropped as if it were never set. This applies at every depth: top-level + fields, keys inside nested tables, and keys inside objects that live inside + arrays. Consequently the standard consumer pattern — `.nullable().optional()` + schemas with `?? null` normalization on write — serializes identically to + never setting the field, and a record round-trips as: write `{ bio: null }`, + read back `{}` (the field is `undefined`). +2. **Array elements — rejected.** A null/undefined **element** of an array is + an error (a typed marshal error naming the element index). TOML arrays + cannot contain nulls, and silently dropping an element — unlike dropping a + key — shifts sibling indices and changes data. Consumers must remove the + element host-side if that is what they mean. *(Deliberate divergence from + 1.x: `@iarna/toml` silently dropped null array elements. That was a silent + data mutation, not a semantics-preserving omission, and is not carried + forward.)* A JS sparse-array hole (`[1, , 2]`) reads as `undefined` and is + rejected the same way. +3. **A null where a value itself is required** — a whole record, a scalar in + any other value position — is an error, as before. + +Because keys are dropped *before* validation, a required field set to `null` +fails JSON-Schema validation as **missing** (exactly as if it were never set), +and `null` for an optional field validates as absent. This preserves the +equivalence: absent key == cleared optional field. + +These rules are part of the cross-binding bytes-authority contract: dropping +the same keys in every binding is what keeps a record with cleared optionals +byte-identical whether written from Node or Python (and byte-identical to what +1.x `@iarna/toml` produced, which dropped null keys at serialize time). + > **Substrate note.** The canonical form above is now in effect on the Node binding: TOML serialization (and the whole tree/commit substrate) runs through `gitsheets-core`, and the `@iarna/toml` / `smol-toml` dependencies are dropped (`node-binding-thin`). Existing repos re-normalize once via `git sheet normalize` per the [Canonical-form re-baseline](#canonical-form-re-baseline-the-rust-serializer). ### File bytes diff --git a/specs/rust-core.md b/specs/rust-core.md index 5c5096d..2a4b925 100644 --- a/specs/rust-core.md +++ b/specs/rust-core.md @@ -104,6 +104,17 @@ kind for faithful re-serialization. re-serialization. - **Strings, booleans, arrays, tables** map to their obvious idiomatic counterparts (table ↔ plain object / dict). +- **Nulls — no core representation; resolved at the marshal boundary.** The + core value type is TOML-faithful and TOML has no `null`, so a host null + (JS `null`/`undefined`, Python `None`) never crosses the FFI. Each binding's + host→core marshal applies the same rule, specced in + [behaviors/normalization.md](behaviors/normalization.md#null--undefined-handling): + a null-valued **table key** is dropped (recursively — nested tables, and + tables inside arrays), restoring the 1.x `@iarna/toml` drop semantics; a + null **array element** is a typed marshal error naming the index (dropping + an element would silently shift data); a null in any other value position is + an error. The shared rejection messages come from `gitsheets-core` so + diagnostics read identically across bindings. ## Embedded code execution From 02c2ce84efbf58c1bfa05bfccd8a75d1f93adf64 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:10:38 -0400 Subject: [PATCH 03/19] fix(marshal): drop null/undefined-valued keys instead of throwing (#232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores the 1.x @iarna/toml drop semantics the Rust-core cutover regressed: a null/undefined (JS) or None (Python) valued table key is dropped at the host->core marshal boundary, recursively — top-level fields, nested tables, and objects inside arrays — so the standard .nullable().optional() + '?? null' consumer pattern serializes byte-identically to never setting the field. A null array ELEMENT is still rejected, now with a typed error naming the index: TOML arrays cannot hold nulls, and dropping an element (as 1.x silently did) shifts sibling indices — a silent data mutation, not a semantics-preserving omission. A null value itself (a whole record, a scalar position) also stays an error. The rejection messages are minted in gitsheets-core (null_array_element_msg / null_value_msg) so the boundary's diagnostics read identically from every binding; each binding's marshal recursion applies the same specced rules (specs/behaviors/normalization.md 'Null / undefined handling'). Merge-patch marshalling, where null is the RFC 7396 delete sentinel, is deliberately untouched. Tests: core unit (message contract), napi boundary (recursive drop, byte-parity with the pre-stripped record, element/scalar rejections, sparse-array holes), Python mirrors, and a new 'nullish' cross-binding byte-parity fixture proving Node and Python land on identical bytes. Closes #232 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- rust/gitsheets-core/src/lib.rs | 2 +- rust/gitsheets-core/src/value.rs | 55 +++++++++++++++++++ rust/gitsheets-napi/src/lib.rs | 49 ++++++++++++++++- rust/gitsheets-napi/test/canonical.mjs | 26 +++++++++ rust/gitsheets-napi/test/roundtrip.mjs | 49 +++++++++++++++++ rust/gitsheets-py/src/lib.rs | 34 +++++++++++- rust/gitsheets-py/tests/_node_writer.mjs | 12 ++++ rust/gitsheets-py/tests/test_cross_binding.py | 11 +++- rust/gitsheets-py/tests/test_smoke.py | 40 ++++++++++++++ 9 files changed, 270 insertions(+), 8 deletions(-) diff --git a/rust/gitsheets-core/src/lib.rs b/rust/gitsheets-core/src/lib.rs index dd012ac..ac88f31 100644 --- a/rust/gitsheets-core/src/lib.rs +++ b/rust/gitsheets-core/src/lib.rs @@ -47,7 +47,7 @@ pub use record::{ }; pub use sheet::{Sheet, StageOutcome, UpsertCandidate, WillChange}; pub use transaction::{Author, Transaction, TransactionOptions, TransactionResult}; -pub use value::{Datetime, DatetimeKind, Value}; +pub use value::{null_array_element_msg, null_value_msg, Datetime, DatetimeKind, Value}; /// Identity over a batch of records — the minimal exercise of the value type /// across a bulk boundary. Bindings call this to prove a whole array of records diff --git a/rust/gitsheets-core/src/value.rs b/rust/gitsheets-core/src/value.rs index 5eb5c9b..293aa46 100644 --- a/rust/gitsheets-core/src/value.rs +++ b/rust/gitsheets-core/src/value.rs @@ -51,6 +51,43 @@ impl Value { } } +// ── marshal-boundary null diagnostics ──────────────────────────────────────── +// +// TOML has no `null`, so a host-language null (JS `null`/`undefined`, Python +// `None`) never crosses the FFI — each binding resolves it during its +// host→core marshal, per the contract in +// `specs/behaviors/normalization.md` § "Null / undefined handling": +// +// 1. a null-valued **table key** is dropped, recursively (absent key == +// cleared optional field — the 1.x `@iarna/toml` semantics); +// 2. a null **array element** is an error (dropping an element would silently +// shift the remaining elements — a data mutation, not an omission); +// 3. a null in any other value position (a whole record, a scalar) is an +// error. +// +// The drop/reject *recursion* necessarily lives in each binding (only the +// binding can see host values), but the messages are minted here so the +// contract's diagnostics read identically from every host language. + +/// The rejection message for rule 2 — a null array element. `null_name` is the +/// host language's spelling ("null/undefined", "None"). +pub fn null_array_element_msg(null_name: &str, index: usize) -> String { + format!( + "cannot marshal {null_name} as an array element (index {index}): TOML arrays cannot \ + contain nulls, and dropping the element would silently shift the rest of the array — \ + remove the element instead (null-valued keys are dropped; array elements are not)" + ) +} + +/// The rejection message for rule 3 — a null where a value itself is required +/// (a whole record, a scalar position). `null_name` as above. +pub fn null_value_msg(null_name: &str) -> String { + format!( + "cannot marshal {null_name} to a TOML value: TOML has no null — null-valued keys are \ + dropped from records, but a value itself cannot be {null_name}" + ) +} + /// The four distinct TOML datetime kinds. They serialize to different bytes, so /// the core keeps them distinguishable even though a binding (e.g. Node) may /// surface them all as one host type (a JS `Date`). @@ -158,6 +195,24 @@ mod tests { } } + #[test] + fn null_diagnostics_name_the_host_null_and_the_index() { + let msg = null_array_element_msg("null/undefined", 3); + assert!(msg.contains("index 3"), "must name the offending index"); + assert!(msg.contains("null/undefined"), "must use the host spelling"); + assert!( + msg.contains("remove the element"), + "must tell the consumer the actionable fix" + ); + + let msg = null_value_msg("None"); + assert!(msg.contains("None"), "must use the host spelling"); + assert!( + msg.contains("null-valued keys are dropped"), + "must state the key-drop rule so the boundary reads consistently" + ); + } + #[test] fn nested_tables_compare_structurally() { let mut inner = IndexMap::new(); diff --git a/rust/gitsheets-napi/src/lib.rs b/rust/gitsheets-napi/src/lib.rs index 9c7afb5..72fc680 100644 --- a/rust/gitsheets-napi/src/lib.rs +++ b/rust/gitsheets-napi/src/lib.rs @@ -13,6 +13,10 @@ //! them as JS `Date` (matching `@iarna` v1.x), with the precise kind retained //! core-side. A `Date` round-trips to a `Date`. //! - **Tables ↔ plain objects, arrays ↔ arrays, strings/bools** — obvious. +//! - **Nulls** — a `null`/`undefined`-valued key is dropped (recursively; the +//! 1.x `@iarna` semantics), a `null`/`undefined` array element or value +//! itself is rejected. See `specs/behaviors/normalization.md` +//! ("Null / undefined handling"). //! //! Errors cross as **structured, matchable** JS errors (a `code`/`status`/class //! discriminant, plus `issues`/`conflictingPaths` payloads), never an opaque @@ -97,8 +101,29 @@ impl FromNapiValue for JsValue { ValueType::String => Value::String(String::from_napi_value(env, napi_val)?), ValueType::Object => { if unknown.is_array()? { - let items = Vec::::from_napi_value(env, napi_val)?; - Value::Array(items.into_iter().map(|j| j.0).collect()) + let obj = JsObject::from_napi_value(env, napi_val)?; + let len = obj.get_array_length()?; + let mut items = Vec::with_capacity(len as usize); + for i in 0..len { + let el: JsUnknown = obj.get_element(i)?; + // A null/undefined *element* is rejected — dropping it + // (as @iarna v1.x silently did) would shift the rest of + // the array. specs/behaviors/normalization.md, rule 2. + if matches!( + el.get_type()?, + ValueType::Null | ValueType::Undefined + ) { + return Err(Error::new( + Status::InvalidArg, + gitsheets_core::null_array_element_msg( + "null/undefined", + i as usize, + ), + )); + } + items.push(JsValue::from_napi_value(env, el.raw())?.0); + } + Value::Array(items) } else if unknown.is_date()? { let date = JsDate::from_napi_value(env, napi_val)?; let ms = date.value_of()?; @@ -112,6 +137,16 @@ impl FromNapiValue for JsValue { let key_js: JsString = names.get_element(i)?; let key = key_js.into_utf8()?.as_str()?.to_owned(); let child: JsUnknown = obj.get_named_property(&key)?; + // A null/undefined-valued *key* is dropped, recursively + // — TOML has no null, so a cleared optional field IS an + // absent key (the 1.x @iarna drop semantics). + // specs/behaviors/normalization.md, rule 1. + if matches!( + child.get_type()?, + ValueType::Null | ValueType::Undefined + ) { + continue; + } let child = JsValue::from_napi_value(env, child.raw())?; map.insert(key, child.0); } @@ -119,9 +154,17 @@ impl FromNapiValue for JsValue { } } other => { + // A null/undefined *value itself* (a whole record, a scalar + // position) — specs/behaviors/normalization.md, rule 3. + if matches!(other, ValueType::Null | ValueType::Undefined) { + return Err(Error::new( + Status::InvalidArg, + gitsheets_core::null_value_msg("null/undefined"), + )); + } return Err(Error::new( Status::InvalidArg, - format!("cannot marshal JS value of type {other:?} to a TOML value (null/undefined have no TOML representation)"), + format!("cannot marshal JS value of type {other:?} to a TOML value"), )); } }; diff --git a/rust/gitsheets-napi/test/canonical.mjs b/rust/gitsheets-napi/test/canonical.mjs index d193578..92a510e 100644 --- a/rust/gitsheets-napi/test/canonical.mjs +++ b/rust/gitsheets-napi/test/canonical.mjs @@ -80,3 +80,29 @@ test('a malformed document throws a typed ConfigError', () => { }, ); }); + +test('nullish keys serialize byte-identically to the record without them', () => { + // The consumer pattern from #232: `.nullable().optional()` schemas with + // `?? null` normalization on write. A cleared optional field must produce + // the SAME bytes as never setting it — the 1.x @iarna drop semantics. + const [withNulls] = serializeRecords([ + { + slug: 'jane', + middleName: null, + bio: undefined, + contact: { email: 'jane@x.org', phone: null }, + roles: [{ title: 'chair', until: null }], + }, + ]); + const [stripped] = serializeRecords([ + { + slug: 'jane', + contact: { email: 'jane@x.org' }, + roles: [{ title: 'chair' }], + }, + ]); + assert.equal(withNulls, stripped, 'byte-identical to the pre-stripped record'); + assert.ok(!withNulls.includes('middleName'), 'no trace of the cleared field'); + assert.ok(!withNulls.includes('phone')); + assert.ok(!withNulls.includes('until')); +}); diff --git a/rust/gitsheets-napi/test/roundtrip.mjs b/rust/gitsheets-napi/test/roundtrip.mjs index ea4f854..d601a08 100644 --- a/rust/gitsheets-napi/test/roundtrip.mjs +++ b/rust/gitsheets-napi/test/roundtrip.mjs @@ -119,3 +119,52 @@ test('a top-level array of scalars round-trips', () => { const out = roundtrip([[1, 'two', true]]); assert.deepEqual(out[0], [1, 'two', true]); }); + +// ── null / undefined handling (specs/behaviors/normalization.md) ───────────── +// +// TOML has no null: a null/undefined-valued KEY is dropped (recursively — the +// 1.x @iarna semantics restored by #232), a null/undefined array ELEMENT or a +// null/undefined value itself is rejected. + +test('null- and undefined-valued keys are dropped from a record', () => { + const out = trip({ keep: 'v', gone: null, alsoGone: undefined }); + assert.deepEqual(out, { keep: 'v' }); + assert.equal(out.gone, undefined, 'dropped key reads back as undefined'); +}); + +test('the key drop is recursive: nested tables and objects inside arrays', () => { + const out = trip({ + nested: { x: null, y: 2, deeper: { z: undefined, k: 'v' } }, + arr: [{ a: null, b: 1 }, { c: 2 }], + }); + assert.deepEqual(out, { + nested: { y: 2, deeper: { k: 'v' } }, + arr: [{ b: 1 }, { c: 2 }], + }); +}); + +test('a record of ONLY nullish keys becomes an empty table, not an error', () => { + const out = trip({ a: null, b: undefined }); + assert.deepEqual(out, {}); +}); + +test('a null array element is rejected with the index named', () => { + assert.throws( + () => roundtrip([{ tags: ['a', null, 'c'] }]), + (err) => { + assert.match(err.message, /array element \(index 1\)/); + assert.match(err.message, /remove the element/); + return true; + }, + ); +}); + +test('an undefined array element (and a sparse hole) is rejected the same way', () => { + assert.throws(() => roundtrip([{ tags: ['a', undefined] }]), /array element \(index 1\)/); + // eslint-disable-next-line no-sparse-arrays + assert.throws(() => roundtrip([{ tags: [1, , 3] }]), /array element \(index 1\)/); +}); + +test('a null record itself is still rejected', () => { + assert.throws(() => roundtrip([null]), /cannot marshal null\/undefined to a TOML value/); +}); diff --git a/rust/gitsheets-py/src/lib.rs b/rust/gitsheets-py/src/lib.rs index 205463b..6d1756b 100644 --- a/rust/gitsheets-py/src/lib.rs +++ b/rust/gitsheets-py/src/lib.rs @@ -17,6 +17,9 @@ //! them as an aware UTC `datetime.datetime`, mirroring the Node `Date` mapping //! (an absolute instant at UTC), with the precise kind retained core-side. //! - **Tables ↔ `dict`, arrays ↔ `list`, strings/bools** — obvious. +//! - **`None`** — a `None`-valued key is dropped (recursively; the 1.x +//! `@iarna` semantics), a `None` array element or value itself is rejected. +//! See `specs/behaviors/normalization.md` ("Null / undefined handling"). //! //! Errors cross as typed Python exceptions (a `GitsheetsError` hierarchy mirroring //! the Node `binding.cjs` classes), each carrying `code`/`status`/ @@ -145,6 +148,12 @@ fn py_to_value(obj: &Bound<'_, PyAny>) -> PyResult { if let Ok(d) = obj.cast::() { let mut map = IndexMap::new(); for (k, v) in d.iter() { + // A None-valued *key* is dropped, recursively — TOML has no null, + // so a cleared optional field IS an absent key (the 1.x @iarna + // drop semantics). specs/behaviors/normalization.md, rule 1. + if v.is_none() { + continue; + } let key: String = k .extract() .map_err(|_| PyTypeError::new_err("table (dict) keys must be strings"))?; @@ -154,21 +163,40 @@ fn py_to_value(obj: &Bound<'_, PyAny>) -> PyResult { } if let Ok(l) = obj.cast::() { let mut items = Vec::with_capacity(l.len()); - for item in l.iter() { + for (i, item) in l.iter().enumerate() { + // A None *element* is rejected — dropping it would shift the rest + // of the array. specs/behaviors/normalization.md, rule 2. + if item.is_none() { + return Err(PyValueError::new_err( + gitsheets_core::null_array_element_msg("None", i), + )); + } items.push(py_to_value(&item)?); } return Ok(Value::Array(items)); } if let Ok(t) = obj.cast::() { let mut items = Vec::with_capacity(t.len()); - for item in t.iter() { + for (i, item) in t.iter().enumerate() { + if item.is_none() { + return Err(PyValueError::new_err( + gitsheets_core::null_array_element_msg("None", i), + )); + } items.push(py_to_value(&item)?); } return Ok(Value::Array(items)); } + // A None *value itself* (a whole record, a scalar position) — + // specs/behaviors/normalization.md, rule 3. + if obj.is_none() { + return Err(PyValueError::new_err(gitsheets_core::null_value_msg( + "None", + ))); + } let type_name = obj.get_type().name()?.to_string(); Err(PyTypeError::new_err(format!( - "cannot marshal Python value of type {type_name} to a TOML value (None has no TOML representation)" + "cannot marshal Python value of type {type_name} to a TOML value" ))) } diff --git a/rust/gitsheets-py/tests/_node_writer.mjs b/rust/gitsheets-py/tests/_node_writer.mjs index 5d9c17f..7c1fc28 100644 --- a/rust/gitsheets-py/tests/_node_writer.mjs +++ b/rust/gitsheets-py/tests/_node_writer.mjs @@ -29,6 +29,18 @@ const FIXTURES = { ratio: 1.5, // → core Float when: new Date(Date.UTC(2026, 5, 26, 12, 0, 0)), }, + // Nullish keys (cleared optionals) are DROPPED at the marshal boundary in + // every binding — the record serializes as if the keys were never set + // (specs/behaviors/normalization.md "Null / undefined handling", #232). The + // Python side mirrors this with None values (and simply never sets the + // `undefined` one); both must land on the same bytes. + nullish: { + slug: 'jane', + middleName: null, + bio: undefined, + contact: { email: 'jane@x.org', phone: null }, + roles: [{ title: 'chair', until: null }], + }, }; function out(obj) { diff --git a/rust/gitsheets-py/tests/test_cross_binding.py b/rust/gitsheets-py/tests/test_cross_binding.py index a6b681e..9e971d9 100644 --- a/rust/gitsheets-py/tests/test_cross_binding.py +++ b/rust/gitsheets-py/tests/test_cross_binding.py @@ -67,10 +67,19 @@ def _git(args, cwd=None): "ratio": 1.5, "when": dt.datetime(2026, 6, 26, 12, 0, 0, tzinfo=dt.timezone.utc), }, + # Nullish keys are dropped at the marshal boundary in every binding (the + # 1.x drop semantics, #232). The JS side also carries `bio: undefined`, + # which Python expresses by never setting the key — same bytes either way. + "nullish": { + "slug": "jane", + "middleName": None, + "contact": {"email": "jane@x.org", "phone": None}, + "roles": [{"title": "chair", "until": None}], + }, } -@pytest.mark.parametrize("fixture", ["basic", "typed"]) +@pytest.mark.parametrize("fixture", ["basic", "typed", "nullish"]) def test_tree_and_blob_bytes_identical_across_bindings(fixture): """record_write from Python and Node yields identical tree + blob hashes.""" py_dir = tempfile.mkdtemp(prefix="gs-py-") diff --git a/rust/gitsheets-py/tests/test_smoke.py b/rust/gitsheets-py/tests/test_smoke.py index b1e7442..61bd57d 100644 --- a/rust/gitsheets-py/tests/test_smoke.py +++ b/rust/gitsheets-py/tests/test_smoke.py @@ -113,6 +113,46 @@ def test_naive_datetime_is_treated_as_utc(): assert "when = 2026-06-26T12:00:00Z\n" in text +# ── None handling at the marshal boundary ───────────────────────────────────── +# specs/behaviors/normalization.md "Null / undefined handling": None-valued +# keys are dropped recursively (the 1.x drop semantics, #232); a None array +# element or a None value itself is an error. + + +def test_none_valued_keys_are_dropped_recursively(): + [out] = gitsheets.roundtrip( + [ + { + "keep": "v", + "gone": None, + "nested": {"x": None, "y": 2}, + "arr": [{"a": None, "b": 1}], + } + ] + ) + assert out == {"keep": "v", "nested": {"y": 2}, "arr": [{"b": 1}]} + + +def test_none_keys_serialize_byte_identically_to_the_stripped_record(): + [with_nones] = gitsheets.serialize_records( + [{"slug": "jane", "middleName": None, "contact": {"email": "j@x.org", "phone": None}}] + ) + [stripped] = gitsheets.serialize_records([{"slug": "jane", "contact": {"email": "j@x.org"}}]) + assert with_nones == stripped + assert "middleName" not in with_nones + assert "phone" not in with_nones + + +def test_none_array_element_raises_with_the_index_named(): + with pytest.raises(ValueError, match=r"array element \(index 1\)"): + gitsheets.roundtrip([{"tags": ["a", None, "c"]}]) + + +def test_none_record_itself_raises(): + with pytest.raises(ValueError, match="cannot marshal None to a TOML value"): + gitsheets.roundtrip([None]) + + # ── record CRUD over holo-tree ────────────────────────────────────────────────── From a25aba91d751d5d0a854f6a0d6ab1f1268dc12dd Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:05:38 -0400 Subject: [PATCH 04/19] docs(specs): freshness model + streaming blob reads (#184, #235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add specs/behaviors/freshness.md: non-transaction Sheet reads resolve through a rebindable read snapshot — rebound automatically after each commit-producing repo.transact (read-your-writes) and explicitly via sheet.refresh() / store.refresh() / repo.refresh(). Spec the streaming read surface: repo.readBlobStream(ref, path) resolved at call time, and sheet.getAttachmentStream(recordOrPath, name) resolved through the snapshot. Align indexing/transactions wording to the rebind model. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- specs/README.md | 1 + specs/api/repository.md | 33 ++++++++- specs/api/sheet.md | 28 +++++++- specs/api/store.md | 16 +++++ specs/behaviors/attachments.md | 23 ++++++- specs/behaviors/freshness.md | 118 ++++++++++++++++++++++++++++++++ specs/behaviors/indexing.md | 6 +- specs/behaviors/transactions.md | 2 +- 8 files changed, 220 insertions(+), 7 deletions(-) create mode 100644 specs/behaviors/freshness.md diff --git a/specs/README.md b/specs/README.md index 955809d..83b39c1 100644 --- a/specs/README.md +++ b/specs/README.md @@ -44,6 +44,7 @@ specs/ ├── validation.md ├── normalization.md ├── transactions.md + ├── freshness.md ├── indexing.md ├── push-sync.md ├── attachments.md diff --git a/specs/api/repository.md b/specs/api/repository.md index 4ed7ebe..d36fb5e 100644 --- a/specs/api/repository.md +++ b/specs/api/repository.md @@ -78,6 +78,35 @@ const result = await repo.transact( See [behaviors/transactions.md](../behaviors/transactions.md) for semantics (mutex, commit-on-success, trailer formatting). +**Auto-refresh on commit.** When the transaction produces a commit, `repo.transact` rebinds every live `Sheet` this repository has issued to the current `HEAD` tree before resolving — reads through standing sheets (including `store.`) immediately reflect the committed state. A no-op or discarded transaction rebinds nothing. See [behaviors/freshness.md](../behaviors/freshness.md). + +### `repo.refresh()` + +Rebind every live `Sheet` this `Repository` has issued (via `openSheet`, `openSheets`, `openStore`, or `Sheet.clone()`) to the repository's current `HEAD` tree. Returns `Promise`. + +```typescript +// after an out-of-band ref movement (external process commit, fetch+reset): +await repo.refresh(); +``` + +Cheap: one ref resolution, then a lazy rebind per sheet — records, attachments, config, and indexes re-derive on their next read. Consumers do **not** need to call this after their own `repo.transact` — a successful commit auto-refreshes (see below). See [behaviors/freshness.md](../behaviors/freshness.md). + +### `repo.readBlobStream(ref, path)` + +Stream a blob's bytes by `:`, resolved **at call time** — independent of any sheet's read snapshot. Returns `Promise` (a Node stream); bytes are piped from the object store without materializing the whole blob in memory. + +```typescript +const stream = await repo.readBlobStream('HEAD', 'people/janedoe/avatar.jpg'); +stream.pipe(httpResponse); +``` + +- `ref` — any tree-ish: ref name, commit hash, or tree hash. +- `path` — tree path under the ref (e.g. an attachment's key: `//`). +- Throws `RefError` (`ref_not_found`) when `ref` doesn't resolve to a tree-ish. +- Throws `NotFoundError` (`record_not_found`) when `path` is absent under the ref's tree, or names a non-blob (a directory). + +The typical consumer is an HTTP handler serving attachment bytes by key (see [behaviors/attachments.md](../behaviors/attachments.md#streaming-reads-by-keypath)); `Sheet.getAttachmentStream` is the sheet-scoped sibling. + ### `repo.requireExplicitTransactions()` Opt into strict mode. After this is called on a `Repository`, calling `Sheet.upsert` / `delete` / `patch` outside a transaction throws `TransactionError` with `code: 'transaction_required'`. @@ -131,7 +160,8 @@ When the handler stages no mutations, the transaction does **not** commit — `c | Class | Code | When | | --- | --- | --- | -| `RefError` | `ref_not_found` | `parent` ref doesn't exist | +| `RefError` | `ref_not_found` | `parent` ref doesn't exist; `readBlobStream` ref doesn't resolve | +| `NotFoundError` | `record_not_found` | `readBlobStream` path absent under the ref, or not a blob | | `TransactionError` | `transaction_in_progress` | Another transaction is open on this repo | | `TransactionError` | `commit_failed` | The underlying `git commit-tree` or `update-ref` failed | | `TransactionError` | `parent_moved` | Optimistic concurrency: parent ref moved between transaction start and commit | @@ -168,4 +198,5 @@ await daemon.stop(); - [api/store.md](store.md) - [api/errors.md](errors.md) - [behaviors/transactions.md](../behaviors/transactions.md) +- [behaviors/freshness.md](../behaviors/freshness.md) - [behaviors/push-sync.md](../behaviors/push-sync.md) diff --git a/specs/api/sheet.md b/specs/api/sheet.md index 12da5ed..a4fc881 100644 --- a/specs/api/sheet.md +++ b/specs/api/sheet.md @@ -12,6 +12,22 @@ A `Sheet` represents one declared sheet in `.gitsheets/.toml`. It owns ## Reading +Non-transaction reads resolve against the sheet's **read snapshot** — rebound automatically after each commit by the owning `Repository`, and explicitly via `refresh()`. See [behaviors/freshness.md](../behaviors/freshness.md). + +### `sheet.refresh()` + +Rebind this sheet's read snapshot to the repository's current `HEAD` tree. Returns `Promise`. + +```typescript +await sheet.refresh(); // pick up out-of-band ref movement +await sheet.queryAll(); // now reflects the current HEAD tree +``` + +- Rebinds **only this sheet** — the "did my row land?" primitive. For every open sheet at once, use `repo.refresh()` / `store.refresh()`. +- Not needed after this repository's own `repo.transact` — a successful commit auto-refreshes every live sheet. +- Lazily re-derives records, attachments, config, and index builds from the new tree ([behaviors/freshness.md](../behaviors/freshness.md#what-a-rebind-refreshes)). +- Throws `TypeError` on a transaction-bound sheet (`tx.sheet(name)`) — those read the transaction's private tree. + ### `sheet.query(filter?, opts?)` Async iterator yielding records matching `filter`. @@ -146,7 +162,7 @@ Pairs naturally with `Transaction#finalize`'s no-op detection (see [transaction. ### `sheet.clone()` -Returns a deep clone of the `Sheet` instance with a cloned data tree. Used to stage a tentative state for diffing / proposing without mutating the original. +Returns a deep clone of the `Sheet` instance with a cloned data tree. Used to stage a tentative state for diffing / proposing without mutating the original. A clone is a Repository-issued sheet like any other: it participates in the freshness model (auto-refresh on commit, `refresh()`), and is **not** a pinned snapshot — see [behaviors/freshness.md](../behaviors/freshness.md#pinned--historical-reads). ## Indexing @@ -196,6 +212,15 @@ await sheet.deleteAttachment(record, 'avatar.jpg'); // throws NotFoundError await sheet.deleteAttachments(record); // no-op if record has no attachment dir ``` +Streaming read without materializing the record: + +```typescript +const stream = await sheet.getAttachmentStream('janedoe', 'avatar.jpg'); // Readable | null +stream?.pipe(httpResponse); +``` + +`getAttachmentStream(recordOrPath, name)` accepts a record object or a rendered record path (like `getAttachment`), returns a Node `Readable` over the attachment's bytes, or `null` when the attachment is absent — resolved through the sheet's read snapshot. See [behaviors/attachments.md](../behaviors/attachments.md#streaming-reads-by-keypath). + Iterator surface: ```typescript @@ -247,6 +272,7 @@ See [api/errors.md](errors.md). Common: `ValidationError`, `NotFoundError`, `Pat - [behaviors/validation.md](../behaviors/validation.md) - [behaviors/normalization.md](../behaviors/normalization.md) - [behaviors/transactions.md](../behaviors/transactions.md) +- [behaviors/freshness.md](../behaviors/freshness.md) - [behaviors/indexing.md](../behaviors/indexing.md) - [behaviors/patch-semantics.md](../behaviors/patch-semantics.md) - [behaviors/attachments.md](../behaviors/attachments.md) diff --git a/specs/api/store.md b/specs/api/store.md index ef0a1b9..030c88e 100644 --- a/specs/api/store.md +++ b/specs/api/store.md @@ -90,6 +90,21 @@ await store.transact({ message: '...' }, async (tx) => { The transaction's commit message, trailers, author, parent, etc. follow the same options as `repo.transact` (see [api/transaction.md](transaction.md)). +## Freshness + +`store.` reads follow the standard freshness model ([behaviors/freshness.md](../behaviors/freshness.md)): a successful `store.transact` / `repo.transact` auto-refreshes every sheet, so post-commit reads through `store.` reflect the committed state. + +### `store.refresh()` + +Rebind every sheet to the repository's current `HEAD` tree — delegates to `repo.refresh()` (a Store's sheets are Repository-issued sheets; partial-store freshness is not a meaningful state). Returns `Promise`. Use after out-of-band ref movement. + +```typescript +await store.refresh(); +const fresh = await store.users.queryAll(); +``` + +Like `transact`, `refresh` is a reserved property name on the `Store` surface — a sheet named `refresh` or `transact` is shadowed by the method and must be reached via `repo.openSheet(name)`. + ## Why `Store` and not just `Repository`? `Repository.openSheets()` returns `{ [name]: Sheet }` — a flat dictionary with no type-level connection between sheet names and shapes. `Store` adds: @@ -114,3 +129,4 @@ Both APIs are public. Use `Repository` for one-off scripts and dynamic discovery - [api/sheet.md](sheet.md) - [api/transaction.md](transaction.md) - [behaviors/validation.md](../behaviors/validation.md) +- [behaviors/freshness.md](../behaviors/freshness.md) diff --git a/specs/behaviors/attachments.md b/specs/behaviors/attachments.md index 98cdba3..6a744a4 100644 --- a/specs/behaviors/attachments.md +++ b/specs/behaviors/attachments.md @@ -6,7 +6,8 @@ A record may have any number of **attachments** — binary blobs colocated with ## Applies To -- [api/sheet.md](../api/sheet.md) — `getAttachment`, `getAttachments`, `setAttachment`, `setAttachments`, `deleteAttachment`, `deleteAttachments`, `attachments` (iterator) +- [api/sheet.md](../api/sheet.md) — `getAttachment`, `getAttachments`, `getAttachmentStream`, `setAttachment`, `setAttachments`, `deleteAttachment`, `deleteAttachments`, `attachments` (iterator) +- [api/repository.md](../api/repository.md) — `readBlobStream` - [behaviors/path-templates.md](path-templates.md) — query traversal skips attachment subtrees ## Storage layout @@ -94,6 +95,26 @@ await sheet.deleteAttachments(record); **No-op** when the record has no attachment directory — idempotent, mirroring the cascade behavior on `Sheet.delete(record)`. The transaction is not marked mutated in the no-op case (so a transaction that does nothing else still completes without a commit). +## Streaming reads by key/path + +Two surfaces stream an attachment's bytes **without materializing its record** — for HTTP handlers serving blobs (avatars, uploads) where "load the record, then get the attachment" is pure overhead: + +### `sheet.getAttachmentStream(recordOrPath, name)` + +```typescript +const stream = await sheet.getAttachmentStream('janedoe', 'avatar.jpg'); +if (stream === null) reply.code(404); +else stream.pipe(reply.raw); +``` + +Accepts a record object or a rendered record path (like `getAttachment`); returns `Promise` — `null` when the attachment is absent, mirroring `getAttachment`. Resolves through the sheet's read snapshot ([freshness.md](freshness.md)), so it reflects this repository's commits without re-opening the sheet. + +### `repo.readBlobStream(ref, path)` + +The repository-level primitive for consumers whose attachment key **is** the tree path (`//`): resolves `:` at call time — no sheet, no snapshot — and returns `Promise`. Throws `RefError` (`ref_not_found`) / `NotFoundError` (`record_not_found`) rather than returning `null`, since the caller named an explicit ref. See [api/repository.md](../api/repository.md#reporeadblobstreamref-path). + +Both are backed by a streamed `git cat-file blob` read — bytes are piped, never fully buffered by gitsheets. + ## Iterator API A higher-level iterator surface — sugar over `getAttachments`: diff --git a/specs/behaviors/freshness.md b/specs/behaviors/freshness.md new file mode 100644 index 0000000..76806ed --- /dev/null +++ b/specs/behaviors/freshness.md @@ -0,0 +1,118 @@ +# Behavior: Read Freshness + +## Rule + +A non-transaction `Sheet` reads through a **read snapshot** — the repository's +`HEAD` tree hash, captured when the sheet was opened. The snapshot is **rebound +to the current `HEAD` tree**: + +1. **Automatically**, after every transaction on the owning `Repository` + instance that produces a commit (explicit `repo.transact` / `store.transact` + *and* permissive-mode auto-transactions). This gives **read-your-writes**: a + record committed through any surface of a `Repository` is immediately + visible to every standing `Sheet` that repository has issued. +2. **Explicitly**, via `sheet.refresh()`, `store.refresh()`, or + `repo.refresh()` — the consumer's tool after **out-of-band ref movement** + (another process committed, a `git fetch` + reset, a hot-reload merge). + +gitsheets never watches the ref: there is no polling and no automatic detection +of commits made outside the `Repository` instance. That is deliberate — the +single-writer model ([push-sync.md](push-sync.md)) makes the owning process the +source of truth for when the ref can move underneath it. + +## Applies To + +- [api/sheet.md](../api/sheet.md) — `query`/`queryFirst`/`queryAll`, `count`, + `loadBody`, `findByIndex`, `getAttachment(s)`, `attachments()`, + `getAttachmentStream`, `diffFrom` (dst side), `readConfig` +- [api/repository.md](../api/repository.md) — `repo.refresh()`, and the + auto-refresh performed by `repo.transact` +- [api/store.md](../api/store.md) — `store.refresh()` +- [behaviors/indexing.md](indexing.md) — index invalidation on rebind + +## Details + +### What a rebind refreshes + +Rebinding swaps the sheet's snapshot tree and lazily re-derives everything +downstream of it: + +- **Record reads** — `query`/`queryFirst`/`queryAll`/`count`/`loadBody` + resolve against the new tree on their next call. +- **Attachment reads** — `getAttachment`/`getAttachments`/`attachments()`/ + `getAttachmentStream` resolve against the new tree. +- **`diffFrom`** — the dst side of the diff is the new tree. +- **Sheet config** — the memoized `.gitsheets/.toml` is dropped and + re-read lazily from the new tree, so a *committed* config change becomes + visible without re-opening the sheet. +- **Indexes** — every index build is invalidated by the snapshot-hash + comparison and lazily rebuilt on its next `findByIndex` (the "out-of-band + ref movement" path in [indexing.md](indexing.md#invalidation)). Note the + cost: a rebuild is a full body-less scan of the sheet, per index, per + rebind that actually changed the tree. + +### Rebind semantics + +- The snapshot always comes from **`HEAD` at rebind time** — not from the + transaction's result tree. A transaction that commits onto a non-`HEAD` + branch (`opts.parent: 'feature-x'`) leaves `HEAD`'s tree unchanged, so + standing sheets do **not** shift onto the other branch's state. +- A rebind to a tree hash identical to the current snapshot is a no-op + (memoized config and index builds are kept). +- Rebinding applies to every live `Sheet` the `Repository` instance has + issued — via `openSheet`, `openSheets`, `openStore`, or `Sheet.clone()`. + Sheets are tracked weakly; holding a `Sheet` does not leak, and dropping one + requires no lifecycle call. +- `repo.refresh()` re-resolves `HEAD^{tree}` once and rebinds every live + sheet. `sheet.refresh()` rebinds only that sheet (the "did my row land?" + primitive). `store.refresh()` delegates to `repo.refresh()` — a Store's + sheets are Repository-issued sheets, and partial-store freshness is not a + meaningful state. + +### Interaction with transactions + +- **Transaction-bound sheets** (`tx.sheet(name)`, `tx.` in + `store.transact`) always read the transaction's private in-progress tree. + They are never rebound; calling `refresh()` on one throws `TypeError`. +- Reads through standing sheets **while a transaction is open** still see the + pre-commit snapshot ([transactions.md](transactions.md#single-writer-model) + — reads don't take the mutex). The rebind happens only after the commit + succeeds; a discarded transaction rebinds nothing. +- A no-op transaction (no commit produced) does not rebind — the tree didn't + move. + +### Iteration stability + +A `query()` async iterator captures the snapshot when iteration starts and +continues against it even if a rebind happens mid-iteration. The rebind +affects subsequent calls, never an in-flight traversal. + +### Pinned / historical reads + +There is no API to pin a `Sheet` to an old tree. Point-in-time reads go +through explicit refs: `sheet.diffFrom(srcRef)` for record-level change +feeds, `repo.readBlobStream(ref, path)` for raw blob bytes at any ref. + +## Principles + +**Local:** + +- **Read-your-writes beats frozen snapshots.** A standing `Sheet` is a live + view of the repository's committed state *as this process advances it* — + not an immutable snapshot pinned at open time. When snapshot stability and + post-write visibility conflict, gitsheets picks visibility: a consumer that + just committed a record must be able to read it back through the handles it + already holds. (First production consumers universally worked around the + pinned-snapshot behavior rather than relying on it — see + [#184](https://github.com/JarvusInnovations/gitsheets/issues/184).) Pinned + historical reads go through explicit refs, not through withholding refresh. + +## Coordinates with + +- [api/repository.md](../api/repository.md) +- [api/sheet.md](../api/sheet.md) +- [api/store.md](../api/store.md) +- [behaviors/transactions.md](transactions.md) +- [behaviors/indexing.md](indexing.md) +- [behaviors/push-sync.md](push-sync.md) +- [GitHub #184](https://github.com/JarvusInnovations/gitsheets/issues/184) — motivating issue diff --git a/specs/behaviors/indexing.md b/specs/behaviors/indexing.md index 2bad676..7502c5d 100644 --- a/specs/behaviors/indexing.md +++ b/specs/behaviors/indexing.md @@ -78,13 +78,13 @@ Without a pre-mutation read (e.g., a brand-new upsert), there's no old key to re For updates where the record changed identity-fields (the key from `keyFn` differs from before), the old key is removed and the new key is added in one synchronous step. -### On out-of-band ref movement +### On read-snapshot rebind (ref movement) -If the sheet's underlying ref moves due to a change made outside this `Sheet` instance — another process committed, the data repo was re-clone, etc. — every index on the sheet is marked dirty. +If the sheet's read snapshot is rebound — automatically after this repository's own commit, or explicitly via `refresh()` after out-of-band movement (see [freshness.md](freshness.md)) — every index on the sheet is marked dirty. Next `findByIndex(name, ...)` call: -1. Detects the dirty state (by comparing the current ref's commit hash against the hash captured at last build) +1. Detects the dirty state (by comparing the current snapshot's tree hash against the hash captured at last build) 2. Discards the existing index for that name 3. Re-runs the build pipeline 4. Serves the lookup diff --git a/specs/behaviors/transactions.md b/specs/behaviors/transactions.md index ed4752d..97568c7 100644 --- a/specs/behaviors/transactions.md +++ b/specs/behaviors/transactions.md @@ -20,7 +20,7 @@ One open transaction per `Repository` at a time, serialized by an in-process mut | Two concurrent `repo.transact` calls from independent async contexts | Second waits on the mutex; runs after the first commits or releases. | | **Nested** `repo.transact` — opening one inside another's handler (same async context) | Throws `TransactionError` (`transaction_in_progress`) immediately; it does not queue. Use `tx.sheet(name)` inside the handler instead. | | Concurrent `repo.transact` *and* a permissive-mode `Sheet.upsert` outside a transaction | The permissive `upsert` opens its own transaction, contends for the same mutex. | -| Same-process concurrent reads | Reads don't take the mutex. They see the *committed* state (pre-transaction tree). | +| Same-process concurrent reads | Reads don't take the mutex. They see the *committed* state (pre-transaction tree). After a successful commit, standing sheets rebind to the new tree — see [freshness.md](freshness.md). | Multi-process / multi-host writers are explicitly out of scope. If another process commits to the same ref while a transaction is open, the transaction throws `TransactionError` (`parent_moved`) when it tries to commit. Detection is via comparing the parent ref's commit hash at transaction open vs. at commit. From e0ef1ca48d2b4df23a96c7208fa41e72eb591028 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:05:38 -0400 Subject: [PATCH 05/19] chore(plans): add sheet-freshness-streaming plan (#184, #235) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- plans/sheet-freshness-streaming.md | 121 +++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 plans/sheet-freshness-streaming.md diff --git a/plans/sheet-freshness-streaming.md b/plans/sheet-freshness-streaming.md new file mode 100644 index 0000000..fdbcd19 --- /dev/null +++ b/plans/sheet-freshness-streaming.md @@ -0,0 +1,121 @@ +--- +status: in-progress +depends: [] +specs: + - specs/behaviors/freshness.md + - specs/api/repository.md + - specs/api/sheet.md + - specs/api/store.md + - specs/behaviors/attachments.md + - specs/behaviors/indexing.md + - specs/behaviors/transactions.md +issues: [184, 235] +--- + +# Plan: Read freshness (refresh + auto-refresh) and streaming blob reads + +## Scope + +Consumer-hardening from the first production 2.x migration +([#184](https://github.com/JarvusInnovations/gitsheets/issues/184), +[#235](https://github.com/JarvusInnovations/gitsheets/issues/235)): + +**In:** + +- The freshness model ([`specs/behaviors/freshness.md`](../specs/behaviors/freshness.md)): + non-transaction `Sheet` reads resolve through a rebindable read snapshot. + `sheet.refresh()`, `store.refresh()`, `repo.refresh()`, and **auto-refresh of + every live sheet after a successful `repo.transact` commit** (read-your-writes). +- Streaming blob reads by key/path without materializing the record: + `repo.readBlobStream(ref, path)` (call-time ref resolution) and + `sheet.getAttachmentStream(recordOrPath, name)` (snapshot-resolved). + +**Out:** + +- Python-binding freshness parity (`rust/gitsheets-py` has its own host shell) — + follow-up issue at closeout. +- Pinning a `Sheet` to an old tree (explicitly rejected by the freshness spec's + read-your-writes principle; historical reads go through `diffFrom(srcRef)` / + `readBlobStream(ref, path)`). +- #234/#236/#237 — the parallel `consumer-api-ergonomics` plan. + +## Implements + +- [`specs/behaviors/freshness.md`](../specs/behaviors/freshness.md) — whole spec (new). +- [`specs/api/repository.md`](../specs/api/repository.md) — `refresh`, + `readBlobStream`, transact auto-refresh note. +- [`specs/api/sheet.md`](../specs/api/sheet.md) — `refresh`, + `getAttachmentStream`, clone-freshness note. +- [`specs/api/store.md`](../specs/api/store.md) — `store.refresh()`. +- [`specs/behaviors/attachments.md`](../specs/behaviors/attachments.md) — + "Streaming reads by key/path". +- [`specs/behaviors/indexing.md`](../specs/behaviors/indexing.md) / + [`specs/behaviors/transactions.md`](../specs/behaviors/transactions.md) — + invalidation/visibility wording aligned to the rebind model. + +## Approach + +All host-shell (TypeScript) work — verified the Rust core is stateless for +non-transaction reads (`recordQuery`/`recordQueryCandidates`/`coreDiscoverSheets` +take `gitDir` + `treeRef` per call; the core `Sheet`/`Store` state machine only +lives inside a `CoreTransaction`). No core or napi changes. + +- **`Sheet`** — make `#readRef` mutable; `_rebindReadTree(tree)` (internal) + swaps the snapshot and drops the memoized config promise; index builds + invalidate automatically via the existing `treeHashAtBuild` comparison. + Public `refresh()` re-resolves via the repo; throws `TypeError` when + transaction-bound. +- **`Repository`** — weak registry (`Set>`) populated from the + `Sheet` constructor for non-tx sheets (covers `openSheet`, `openSheets`, + `openStore`, `clone`); `refresh()` resolves `HEAD^{tree}` once and rebinds + live sheets, pruning dead refs; `transact` calls it after a commit-producing + finalize (before post-commit hooks). +- **`Store`** — `refresh` property delegating to `repo.refresh()`; `Store` + type gains `readonly refresh: () => Promise`. +- **`repo.readBlobStream(ref, path)`** — `git cat-file -t :` type + probe → typed `RefError`/`NotFoundError`, then a spawned + `git cat-file blob` stdout as the `Readable` (consistent with the existing + attachment-handle streaming; genuine git porcelain). +- **`sheet.getAttachmentStream`** — `getAttachment` (snapshot-resolved hash) + → stream by hash; `null` passthrough. +- Docs: `docs/api.md` Repository/Sheet/Store sections. + +## Validation + +- [ ] Standing `Sheet`/`Store` reads reflect a `repo.transact` commit + immediately after it resolves (no re-open) — vitest. +- [ ] External commit (second `Repository` instance) is invisible until + `sheet.refresh()` / `repo.refresh()` / `store.refresh()`, visible after — + vitest. +- [ ] A commit onto a non-HEAD branch does **not** shift standing sheets — vitest. +- [ ] `findByIndex` after a commit reflects the new tree (lazy rebuild), and a + committed sheet-config change is visible after rebind — vitest. +- [ ] `refresh()` on a tx-bound sheet throws `TypeError` — vitest. +- [ ] `repo.readBlobStream` streams byte-identical content for a committed + attachment; missing path / non-blob → `NotFoundError(record_not_found)`; + bad ref → `RefError(ref_not_found)` — vitest. +- [ ] `sheet.getAttachmentStream` streams current bytes through the snapshot + (fresh after auto-refresh) and returns `null` when absent — vitest. +- [ ] Full suites green: package vitest + type-check, `cargo test`, napi + `node --test` (core untouched — suites prove no regression). + +## Risks / unknowns + +- **Behavior change for snapshot-reliant consumers** — auto-refresh replaces + the accidental pinned-at-open behavior. Mitigated: the old behavior was + never specified as a snapshot guarantee (repository.md said "bound to this + repository's current state"), and the known consumers all worked around + staleness rather than relying on it. Spec'd as the read-your-writes + principle. +- **WeakRef registry growth** — long-lived repos opening many short-lived + sheets. Mitigated: dead refs pruned on every refresh. +- **In-flight iteration during rebind** — `query()` captures its tree ref at + call start; spec'd as iteration stability, no code change needed. + +## Notes + +(Populated at closeout.) + +## Follow-ups + +(Populated at closeout.) From 44184494a0c2f9e7214e65968258a15198296acf Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:10:04 -0400 Subject: [PATCH 06/19] feat(api): read freshness (refresh + auto-refresh) and streaming blob reads (#184, #235) Implement specs/behaviors/freshness.md host-side (the Rust core is stateless for non-transaction reads, so no core changes): - Sheet: mutable read snapshot + rebindReadTree (drops the memoized config; index builds invalidate via the existing treeHashAtBuild comparison); public sheet.refresh(); TypeError when tx-bound. - Repository: weak sheet registry populated from the Sheet constructor; repo.refresh(); transact rebinds every live sheet to the post-commit HEAD tree before post-commit hooks (read-your-writes). - Store: store.refresh() delegating to repo.refresh(). - repo.readBlobStream(ref, path): call-time-resolved streamed blob read with typed RefError/NotFoundError; sheet.getAttachmentStream(): the snapshot-resolved sibling, null when absent. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- .../gitsheets/src/repo-blob-stream.test.ts | 138 ++++++++++ packages/gitsheets/src/repository.ts | 107 +++++++- packages/gitsheets/src/sheet-refresh.test.ts | 242 ++++++++++++++++++ packages/gitsheets/src/sheet.ts | 66 ++++- packages/gitsheets/src/store.ts | 11 +- 5 files changed, 557 insertions(+), 7 deletions(-) create mode 100644 packages/gitsheets/src/repo-blob-stream.test.ts create mode 100644 packages/gitsheets/src/sheet-refresh.test.ts diff --git a/packages/gitsheets/src/repo-blob-stream.test.ts b/packages/gitsheets/src/repo-blob-stream.test.ts new file mode 100644 index 0000000..d21b2d1 --- /dev/null +++ b/packages/gitsheets/src/repo-blob-stream.test.ts @@ -0,0 +1,138 @@ +// Streaming blob reads by key/path — repo.readBlobStream + sheet.getAttachmentStream. +// See specs/behaviors/attachments.md#streaming-reads-by-keypath. + +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import type { Readable } from 'node:stream'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { NotFoundError, RefError } from './errors.js'; +import { openRepo, type Repository } from './repository.js'; +import { testRepo, type TestRepoHandle } from './test-helpers/test-repo.js'; + +const handles: TestRepoHandle[] = []; +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.cleanup(); + } +}); + +const USERS = `[gitsheet] +root = 'users' +path = '\${{ slug }}' +`; + +async function seededRepo(): Promise<{ fixture: TestRepoHandle; repo: Repository }> { + const fixture = await testRepo({ withInitialCommit: true }); + handles.push(fixture); + await mkdir(join(fixture.path, '.gitsheets'), { recursive: true }); + await writeFile(join(fixture.path, '.gitsheets', 'users.toml'), USERS); + await fixture.git('add', '.gitsheets/'); + await fixture.git('commit', '-m', 'add users sheet'); + const repo = await openRepo({ gitDir: fixture.gitDir }); + return { fixture, repo }; +} + +async function collect(stream: Readable): Promise { + const chunks: Buffer[] = []; + for await (const chunk of stream) { + chunks.push(chunk as Buffer); + } + return Buffer.concat(chunks); +} + +/** Binary bytes (not valid UTF-8) to prove byte fidelity end-to-end. */ +const BINARY = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0xff, 0xfe, 0x0a, 0x1b]); + +async function commitAvatar(repo: Repository, bytes: Buffer): Promise { + await repo.transact({ message: 'avatar' }, async (tx) => { + const sheet = tx.sheet('users'); + await sheet.upsert({ slug: 'jane' }); + const blob = await repo.writeBlob(bytes); + await sheet.setAttachment('jane', 'avatar.png', blob); + }); +} + +describe('repo.readBlobStream', () => { + it('streams byte-identical content for a committed attachment', async () => { + const { repo } = await seededRepo(); + await commitAvatar(repo, BINARY); + + const stream = await repo.readBlobStream('HEAD', 'users/jane/avatar.png'); + expect(Buffer.compare(await collect(stream), BINARY)).toBe(0); + }); + + it('resolves the ref at call time — a later commit is immediately visible', async () => { + const { repo } = await seededRepo(); + await commitAvatar(repo, Buffer.from('v1')); + await commitAvatar(repo, Buffer.from('v2')); + + const stream = await repo.readBlobStream('HEAD', 'users/jane/avatar.png'); + expect((await collect(stream)).toString()).toBe('v2'); + }); + + it('throws NotFoundError(record_not_found) for a missing path', async () => { + const { repo } = await seededRepo(); + const err = await repo.readBlobStream('HEAD', 'users/nope/avatar.png').catch((e: unknown) => e); + expect(err).toBeInstanceOf(NotFoundError); + expect((err as NotFoundError).code).toBe('record_not_found'); + }); + + it('throws NotFoundError(record_not_found) for a directory path', async () => { + const { repo } = await seededRepo(); + await commitAvatar(repo, BINARY); + const err = await repo.readBlobStream('HEAD', 'users/jane').catch((e: unknown) => e); + expect(err).toBeInstanceOf(NotFoundError); + }); + + it('throws RefError(ref_not_found) for a ref that does not resolve', async () => { + const { repo } = await seededRepo(); + const err = await repo.readBlobStream('no-such-ref', 'users/jane/avatar.png').catch((e: unknown) => e); + expect(err).toBeInstanceOf(RefError); + expect((err as RefError).code).toBe('ref_not_found'); + }); +}); + +describe('sheet.getAttachmentStream', () => { + it('streams the attachment bytes by record path without materializing the record', async () => { + const { repo } = await seededRepo(); + await commitAvatar(repo, BINARY); + const users = await repo.openSheet('users'); + + const stream = await users.getAttachmentStream('jane', 'avatar.png'); + expect(stream).not.toBeNull(); + expect(Buffer.compare(await collect(stream!), BINARY)).toBe(0); + }); + + it('accepts a record object too', async () => { + const { repo } = await seededRepo(); + await commitAvatar(repo, BINARY); + const users = await repo.openSheet('users'); + + const stream = await users.getAttachmentStream({ slug: 'jane' }, 'avatar.png'); + expect(Buffer.compare(await collect(stream!), BINARY)).toBe(0); + }); + + it('returns null when the attachment is absent', async () => { + const { repo } = await seededRepo(); + await commitAvatar(repo, BINARY); + const users = await repo.openSheet('users'); + + expect(await users.getAttachmentStream('jane', 'missing.png')).toBeNull(); + }); + + it('reads through the fresh snapshot after this repository commits (auto-refresh)', async () => { + const { repo } = await seededRepo(); + const users = await repo.openSheet('users'); + + await commitAvatar(repo, Buffer.from('v1')); + let stream = await users.getAttachmentStream('jane', 'avatar.png'); + expect((await collect(stream!)).toString()).toBe('v1'); + + await commitAvatar(repo, Buffer.from('v2')); + stream = await users.getAttachmentStream('jane', 'avatar.png'); + expect((await collect(stream!)).toString()).toBe('v2'); + }); +}); diff --git a/packages/gitsheets/src/repository.ts b/packages/gitsheets/src/repository.ts index 86e7359..55949ad 100644 --- a/packages/gitsheets/src/repository.ts +++ b/packages/gitsheets/src/repository.ts @@ -4,11 +4,12 @@ // remaining git shell-outs are genuine porcelain (ref resolution, sheet // discovery, author config). See specs/api/repository.md. -import { execFile } from 'node:child_process'; +import { execFile, spawn } from 'node:child_process'; +import type { Readable } from 'node:stream'; import { promisify } from 'node:util'; import { addon, callCore, CoreTransaction } from './core.js'; -import { ConfigError, RefError, TransactionError } from './errors.js'; +import { ConfigError, NotFoundError, RefError, TransactionError } from './errors.js'; import type { RecordLike } from './path-template/index.js'; import { PushDaemon, @@ -79,6 +80,13 @@ export class Repository { readonly #gitDir: string; readonly #mutex = new Mutex(); readonly #postCommitHooks: Array<(commitHash: string) => void> = []; + /** + * Every live non-transaction Sheet this Repository has issued, held weakly + * so registration imposes no lifecycle obligation on consumers. Rebound to + * the current HEAD tree by {@link refresh} (and the transact auto-refresh). + * See specs/behaviors/freshness.md. + */ + readonly #sheetRegistry = new Set>(); #strictMode = false; #pushDaemon: PushDaemon | null = null; @@ -111,6 +119,84 @@ export class Repository { this.#strictMode = true; } + /** + * @internal — called from the Sheet constructor so every non-transaction + * Sheet this Repository issues (openSheet / openSheets / openStore / + * Sheet.clone) participates in the freshness model. Weakly held. + */ + registerSheet(sheet: Sheet): void { + this.#sheetRegistry.add(new WeakRef(sheet)); + } + + /** + * @internal — the tree hash non-transaction reads currently resolve + * against: HEAD's tree, or the empty tree on a fresh repo. Used by + * Sheet.refresh to rebind a single sheet. + */ + async currentReadTree(): Promise { + return this.#resolveReadTree(); + } + + /** + * Rebind every live Sheet this Repository has issued to the current HEAD + * tree. The consumer's tool after out-of-band ref movement; not needed + * after this repository's own transact (a successful commit auto-refreshes). + * See specs/behaviors/freshness.md. + */ + async refresh(): Promise { + const tree = await this.#resolveReadTree(); + this.#rebindLiveSheets(tree); + } + + #rebindLiveSheets(tree: string): void { + for (const ref of this.#sheetRegistry) { + const sheet = ref.deref(); + if (sheet === undefined) { + this.#sheetRegistry.delete(ref); + continue; + } + sheet.rebindReadTree(tree); + } + } + + /** + * Stream a blob's bytes by `:`, resolved at call time — + * independent of any Sheet's read snapshot. Returns a Node Readable piped + * from `git cat-file blob`; the blob is never fully buffered by gitsheets. + * + * Throws RefError(ref_not_found) when `ref` doesn't resolve to a tree-ish, + * NotFoundError(record_not_found) when `path` is absent under the ref's + * tree or names a non-blob. See specs/api/repository.md and + * specs/behaviors/attachments.md#streaming-reads-by-keypath. + */ + async readBlobStream(ref: string, path: string): Promise { + const spec = `${ref}:${path}`; + let type: string | null = null; + try { + const { stdout } = await exec('git', ['cat-file', '-t', spec], { cwd: this.#gitDir }); + type = stdout.trim() || null; + } catch { + type = null; + } + if (type === null) { + // Distinguish a bad ref from a missing path for typed errors. + const refResolves = await revParseVerify(this.#gitDir, `${ref}^{tree}`); + if (!refResolves) { + throw new RefError('ref_not_found', `readBlobStream: ref does not resolve: ${ref}`); + } + throw new NotFoundError('record_not_found', `readBlobStream: no object at ${spec}`); + } + if (type !== 'blob') { + throw new NotFoundError( + 'record_not_found', + `readBlobStream: object at ${spec} is a ${type}, not a blob`, + ); + } + const child = spawn('git', ['cat-file', 'blob', spec], { cwd: this.#gitDir }); + child.stdin.end(); + return child.stdout; + } + /** Resolve a ref or commit hash. Returns the full commit hash or null. */ async resolveRef(ref: string): Promise { return resolveCommit(this.#gitDir, ref); @@ -240,6 +326,11 @@ export class Repository { } const result = await tx.finalize(value); if (result.commitHash !== null) { + // Auto-refresh: rebind every live sheet to the post-commit HEAD tree + // (read-your-writes — specs/behaviors/freshness.md). Resolved from + // HEAD rather than the result tree so a commit onto a non-HEAD branch + // doesn't shift HEAD-bound sheets. + this.#rebindLiveSheets(await this.#resolveReadTree()); for (const hook of this.#postCommitHooks) { try { hook(result.commitHash); @@ -352,6 +443,18 @@ export async function openRepo(opts: OpenRepoOptions = {}): Promise return Repository.open(opts); } +/** True when `git rev-parse --verify --quiet ` resolves. */ +async function revParseVerify(gitDir: string, rev: string): Promise { + try { + const { stdout } = await exec('git', ['rev-parse', '--verify', '--quiet', rev], { + cwd: gitDir, + }); + return stdout.trim().length > 0; + } catch { + return false; + } +} + /** Resolve a ref/commit-ish to its full commit hash via git rev-parse; null on failure. */ async function resolveCommit(gitDir: string, ref: string): Promise { try { diff --git a/packages/gitsheets/src/sheet-refresh.test.ts b/packages/gitsheets/src/sheet-refresh.test.ts new file mode 100644 index 0000000..9a05ec5 --- /dev/null +++ b/packages/gitsheets/src/sheet-refresh.test.ts @@ -0,0 +1,242 @@ +// Read freshness — refresh + transact auto-refresh. +// See specs/behaviors/freshness.md. + +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { openRepo } from './repository.js'; +import { openStore } from './store.js'; +import { testRepo, type TestRepoHandle } from './test-helpers/test-repo.js'; + +const handles: TestRepoHandle[] = []; +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.cleanup(); + } +}); + +async function makeRepo(): Promise { + const h = await testRepo({ withInitialCommit: true }); + handles.push(h); + return h; +} + +const USERS = `[gitsheet] +root = 'users' +path = '\${{ slug }}' +`; + +async function seedUsers(fixture: TestRepoHandle): Promise { + await mkdir(join(fixture.path, '.gitsheets'), { recursive: true }); + await writeFile(join(fixture.path, '.gitsheets', 'users.toml'), USERS); + await fixture.git('add', '.gitsheets/'); + await fixture.git('commit', '-m', 'add users sheet'); +} + +describe('transact auto-refresh (read-your-writes)', () => { + it('standing Sheet reads reflect a repo.transact commit without re-opening', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const users = await repo.openSheet('users'); + + expect(await users.queryAll()).toEqual([]); + + await repo.transact({ message: 'add jane' }, async (tx) => { + await tx.sheet('users').upsert({ slug: 'jane', email: 'jane@x.org' }); + }); + + const rows = await users.queryAll(); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ slug: 'jane', email: 'jane@x.org' }); + }); + + it('store sheets read post-commit state after store.transact', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const store = await openStore(repo); + + await store.transact({ message: 'add jane' }, async (tx) => { + await (tx as Record)['users']!.upsert({ + slug: 'jane', + email: 'jane@x.org', + }); + }); + + const users = (store as unknown as Record)['users']!; + expect(await users.queryAll()).toHaveLength(1); + }); + + it('permissive-mode auto-transactions refresh sibling sheets too', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const writer = await repo.openSheet('users'); + const reader = await repo.openSheet('users'); + + await writer.upsert({ slug: 'jane' }); + + expect(await reader.queryAll()).toHaveLength(1); + }); + + it('attachment reads through a standing sheet see post-commit state', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const users = await repo.openSheet('users'); + + await repo.transact({ message: 'add jane + avatar' }, async (tx) => { + const sheet = tx.sheet('users'); + await sheet.upsert({ slug: 'jane' }); + const blob = await repo.writeBlob(Buffer.from('png-bytes')); + await sheet.setAttachment('jane', 'avatar.png', blob); + }); + + const att = await users.getAttachment('jane', 'avatar.png'); + expect(att).not.toBeNull(); + expect((await att!.read()).toString()).toBe('png-bytes'); + }); + + it('a commit onto a non-HEAD branch does not shift standing sheets', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + await fixture.git('branch', 'side'); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const users = await repo.openSheet('users'); + + const result = await repo.transact( + { message: 'add jane on side', parent: 'side' }, + async (tx) => { + await tx.sheet('users').upsert({ slug: 'jane' }); + }, + ); + expect(result.commitHash).not.toBeNull(); + expect(result.ref).toBe('refs/heads/side'); + + // HEAD (main) is unchanged, so the standing sheet stays empty. + expect(await users.queryAll()).toEqual([]); + }); + + it('a no-op transaction leaves the snapshot untouched', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const users = await repo.openSheet('users'); + + const result = await repo.transact({ message: 'noop' }, async () => 'nothing'); + expect(result.commitHash).toBeNull(); + expect(await users.queryAll()).toEqual([]); + }); +}); + +describe('explicit refresh after out-of-band movement', () => { + it('an external commit is invisible until sheet.refresh()', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const users = await repo.openSheet('users'); + expect(await users.queryAll()).toEqual([]); + + // Out-of-band writer: a second Repository instance over the same git dir. + const external = await openRepo({ gitDir: fixture.gitDir }); + await external.transact({ message: 'external add' }, async (tx) => { + await tx.sheet('users').upsert({ slug: 'ext' }); + }); + + // The first repo instance did not commit; its sheets are still pinned. + expect(await users.queryAll()).toEqual([]); + + await users.refresh(); + expect(await users.queryAll()).toHaveLength(1); + }); + + it('repo.refresh() rebinds every open sheet at once', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const a = await repo.openSheet('users'); + const b = await repo.openSheet('users'); + + const external = await openRepo({ gitDir: fixture.gitDir }); + await external.transact({ message: 'external add' }, async (tx) => { + await tx.sheet('users').upsert({ slug: 'ext' }); + }); + + await repo.refresh(); + expect(await a.queryAll()).toHaveLength(1); + expect(await b.queryAll()).toHaveLength(1); + }); + + it('store.refresh() delegates to repo.refresh()', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const store = await openStore(repo); + const users = (store as unknown as Record)['users']!; + + const external = await openRepo({ gitDir: fixture.gitDir }); + await external.transact({ message: 'external add' }, async (tx) => { + await tx.sheet('users').upsert({ slug: 'ext' }); + }); + + expect(await users.queryAll()).toEqual([]); + await store.refresh(); + expect(await users.queryAll()).toHaveLength(1); + }); + + it('refresh() throws TypeError on a transaction-bound sheet', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + + await repo.transact({ message: 'probe' }, async (tx) => { + const sheet = tx.sheet('users'); + await expect(sheet.refresh()).rejects.toBeInstanceOf(TypeError); + await sheet.upsert({ slug: 'jane' }); + }); + }); +}); + +describe('rebind re-derivations', () => { + it('findByIndex reflects the post-commit tree via lazy rebuild', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const users = await repo.openSheet('users'); + users.defineIndex('byEmail', { unique: true }, (r) => (r['email'] as string) ?? undefined); + + expect(await users.findByIndex('byEmail', 'jane@x.org')).toBeUndefined(); + + await repo.transact({ message: 'add jane' }, async (tx) => { + await tx.sheet('users').upsert({ slug: 'jane', email: 'jane@x.org' }); + }); + + const hit = await users.findByIndex('byEmail', 'jane@x.org'); + expect(hit).toMatchObject({ slug: 'jane' }); + }); + + it('a committed sheet-config change becomes visible after rebind', async () => { + const fixture = await makeRepo(); + await seedUsers(fixture); + const repo = await openRepo({ gitDir: fixture.gitDir }); + const users = await repo.openSheet('users'); + expect((await users.readConfig()).schema).toBeNull(); + + const withSchema = `${USERS} +[gitsheet.schema] +type = 'object' + +[gitsheet.schema.properties.slug] +type = 'string' +`; + await repo.transact({ message: 'tighten config' }, async (tx) => { + tx.writeFile('.gitsheets/users.toml', withSchema); + }); + + expect((await users.readConfig()).schema).not.toBeNull(); + }); +}); diff --git a/packages/gitsheets/src/sheet.ts b/packages/gitsheets/src/sheet.ts index fe8e4fd..60d3930 100644 --- a/packages/gitsheets/src/sheet.ts +++ b/packages/gitsheets/src/sheet.ts @@ -741,7 +741,12 @@ export class Sheet { readonly #name: string; readonly #configPath: string; readonly #transaction: Transaction | undefined; - readonly #readRef: string | undefined; + /** + * Non-transactional read snapshot — mutable: rebound to the current HEAD + * tree by the owning Repository's refresh/auto-refresh, or by + * {@link refresh}. See specs/behaviors/freshness.md. + */ + #readRef: string | undefined; readonly #dataBase: string; readonly #validator: StandardSchemaV1 | undefined; readonly #prefix: string; @@ -757,6 +762,11 @@ export class Sheet { this.#dataBase = (opts.dataBase ?? '').replace(/^\/+|\/+$/g, ''); this.#validator = opts.validator; this.#prefix = (opts.prefix ?? '').replace(/^\/+|\/+$/g, ''); + if (this.#transaction === undefined) { + // Participate in the freshness model: the Repository rebinds every live + // non-tx sheet after each of its own commits, and on repo.refresh(). + this.#repo.registerSheet(this as unknown as Sheet); + } } /** The git dir this sheet reads/writes through. */ @@ -811,10 +821,43 @@ export class Sheet { return this.#transaction !== undefined; } + /** + * Rebind this sheet's read snapshot to the repository's current HEAD tree. + * Only this sheet — the "did my row land?" primitive; use `repo.refresh()` + * or `store.refresh()` to rebind every open sheet at once. Not needed after + * this repository's own `repo.transact` (a successful commit + * auto-refreshes). See specs/behaviors/freshness.md. + * + * Throws TypeError on a transaction-bound sheet — those read the + * transaction's private in-progress tree and are never rebound. + */ + async refresh(): Promise { + if (this.#transaction !== undefined) { + throw new TypeError( + 'Sheet.refresh() is not available on a transaction-bound Sheet — it reads the transaction’s private tree', + ); + } + this.rebindReadTree(await this.#repo.currentReadTree()); + } + + /** + * @internal — swap the read snapshot to `tree` and lazily re-derive + * everything downstream: the memoized config drops (re-read from the new + * tree on next access) and index builds invalidate via the existing + * `treeHashAtBuild` comparison in `#ensureIndexBuilt`. No-op when the tree + * hasn't moved, or on a transaction-bound sheet. + */ + rebindReadTree(tree: string): void { + if (this.#transaction !== undefined) return; + if (this.#readRef === tree) return; + this.#readRef = tree; + this.#configPromise = undefined; + } + async readConfig(): Promise { - // A Sheet reads a single, immutable tree ref (the open snapshot, or the tx - // parent), so its config never changes over the instance's lifetime. - // Memoize to avoid a `git rev-parse` per call on hot write/query loops. + // A Sheet reads a stable snapshot tree ref (rebound only via + // rebindReadTree, which resets this memo), so config is memoized to avoid + // a `git rev-parse` per call on hot write/query loops. if (this.#configPromise === undefined) { this.#configPromise = loadConfig(this.#gitDir, this.#configTreeRef(), this.#configPath); } @@ -1442,6 +1485,21 @@ export class Sheet { return out; } + /** + * Stream an attachment's bytes without materializing the record. Accepts a + * record object or a rendered record path (like `getAttachment`); returns a + * Node Readable over the blob's bytes, or `null` when the attachment is + * absent. Resolved through the sheet's read snapshot — fresh after this + * repository's own commits (auto-refresh) or an explicit `refresh()`. + * + * See specs/behaviors/attachments.md#streaming-reads-by-keypath. + */ + async getAttachmentStream(record: T | string, name: string): Promise { + const blob = await this.getAttachment(record, name); + if (blob === null) return null; + return makeAttachmentBlobHandle(this.#gitDir, blob.hash).stream(); + } + /** * Async iterator over a record's attachments. Each yielded item carries * `name`, an extension-inferred `mimeType`, and a `blob` handle with diff --git a/packages/gitsheets/src/store.ts b/packages/gitsheets/src/store.ts index 8f5ac1b..642f9e6 100644 --- a/packages/gitsheets/src/store.ts +++ b/packages/gitsheets/src/store.ts @@ -39,6 +39,13 @@ export type Store = { readonly [K in keyof V]: Sheet>; } & { readonly transact: StoreTransactFn; + /** + * Rebind every sheet to the repository's current HEAD tree — delegates to + * `repo.refresh()`. Use after out-of-band ref movement; not needed after + * this store's own transact (a successful commit auto-refreshes). + * See specs/behaviors/freshness.md. + */ + readonly refresh: () => Promise; }; /** tx object passed into Store.transact's handler. */ @@ -109,5 +116,7 @@ export async function openStore( return repo.transact(txOpts, innerHandler); }; - return Object.assign(Object.create(null), sheets, { transact }) as Store; + const refresh = (): Promise => repo.refresh(); + + return Object.assign(Object.create(null), sheets, { transact, refresh }) as Store; } From 5108edeaf7814b482ca6b50c2eebe0c33dce34a8 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:10:04 -0400 Subject: [PATCH 07/19] docs(api): document refresh, auto-refresh freshness, and streaming blob reads Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- docs/api.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index 50930a3..6adcca3 100644 --- a/docs/api.md +++ b/docs/api.md @@ -83,6 +83,8 @@ const store = await openStore(repo, { | `repo.requireExplicitTransactions()` | Switch to strict mode (one-way) | | `repo.startPushDaemon(opts)` | Start async push to a configured remote | | `repo.resolveRef(ref)` | Resolve a ref or commit hash; returns `string \| null` | +| `repo.refresh()` | Rebind every live Sheet to the current HEAD tree — for out-of-band ref movement (a successful `transact` auto-refreshes) | +| `repo.readBlobStream(ref, path)` | `Promise` — stream a blob's bytes at `:`, resolved at call time. Throws `RefError` / `NotFoundError` | `opts.prefix` scopes records to a sub-tree under each sheet's configured root — useful for multi-tenant deployments where one git repo holds many tenants under `//...`. Mirrors the CLI `--prefix` flag (env `GITSHEETS_PREFIX`). The sheet's `.gitsheets/.toml` config file is unaffected — only the record data tree is scoped. @@ -100,6 +102,7 @@ const store = await openStore(repo, { | `sheet.queryFirst(filter?, opts?)` | `Promise` — honors `opts.signal`, `opts.withBody` | | `sheet.queryAll(filter?, opts?)` | `Promise` — honors `opts.signal`, `opts.withBody` | | `sheet.loadBody(record)` | `Promise` — hydrate a body-less record (content-typed sheets) | +| `sheet.refresh()` | Rebind this sheet's read snapshot to the current HEAD tree (out-of-band movement; own commits auto-refresh) | | `sheet.pathForRecord(record)` | `Promise` — rendered path, no write | | `sheet.normalizeRecord(record)` | `Promise` — canonical form, no write | @@ -124,6 +127,7 @@ All write methods route through a transaction — permissive mode auto-opens one | --- | --- | | `sheet.getAttachment(record, name)` | One attachment's BlobObject or null | | `sheet.getAttachments(record)` | Map of name → BlobObject | +| `sheet.getAttachmentStream(recordOrPath, name)` | `Promise` — stream an attachment's bytes without materializing the record | | `sheet.setAttachment(record, name, blob)` | Add or replace | | `sheet.setAttachments(record, map)` | Bulk variant | | `sheet.deleteAttachment(record, name)` | Remove a single attachment; throws `NotFoundError` if missing | @@ -177,10 +181,13 @@ type Store = { readonly [K in keyof V]: Sheet>; } & { readonly transact: StoreTransactFn; + readonly refresh: () => Promise; }; ``` -`store.` for each sheet declared in `validators`. `store.transact(opts, async tx => ...)` mirrors `repo.transact` with `tx.` aliases that thread validators through. +`store.` for each sheet declared in `validators`. `store.transact(opts, async tx => ...)` mirrors `repo.transact` with `tx.` aliases that thread validators through. `store.refresh()` rebinds every sheet to the current HEAD tree (delegates to `repo.refresh()`). + +Reads through `store.` follow the freshness model: a successful `store.transact` / `repo.transact` auto-refreshes, so post-commit reads reflect the committed state — see [`specs/behaviors/freshness.md`](https://github.com/JarvusInnovations/gitsheets/blob/develop/specs/behaviors/freshness.md). Sheets not in `validators` are accessible via `repo.openSheet(name)` for one-off un-typed access. From d298fb56f3893981239af6862773e23a3e0b704d Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:22:50 -0400 Subject: [PATCH 08/19] test(sheet): cover the cleared-optional consumer pattern end-to-end (#232) The real-world shape that surfaced the regression: a Standard Schema validator normalizing cleared optionals to explicit nulls ('?? null'). Verifies the write succeeds, the read-back has absent keys (not nulls), the on-disk bytes carry no trace of the cleared fields, re-writing the null-cleared record over the stripped one is a byte-level no-op (no new commit), the drop recurses through nested tables and objects inside arrays, a null-valued required field fails validation as missing, and a null array element is rejected with the index named and no tree mutation. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- packages/gitsheets/src/sheet-nullish.test.ts | 180 +++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 packages/gitsheets/src/sheet-nullish.test.ts diff --git a/packages/gitsheets/src/sheet-nullish.test.ts b/packages/gitsheets/src/sheet-nullish.test.ts new file mode 100644 index 0000000..c3d53b5 --- /dev/null +++ b/packages/gitsheets/src/sheet-nullish.test.ts @@ -0,0 +1,180 @@ +// End-to-end tests for null/undefined handling on write (#232) — the +// specs/behaviors/normalization.md "Null / undefined handling" contract at the +// package layer. +// +// The real-world consumer shape this protects: optional fields modeled as +// `.nullable().optional()` (Zod/Valibot/…) with a `?? null` normalization at +// the write boundary. 1.x (@iarna/toml) silently dropped null-valued keys at +// serialize time; the Rust-core cutover initially threw on them; the specced +// behavior is the 1.x drop — an absent optional field IS an absent key. + +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { ValidationError } from './errors.js'; +import { openRepo } from './repository.js'; +import { testRepo, type TestRepoHandle } from './test-helpers/test-repo.js'; +import { type StandardSchemaV1 } from './validation.js'; + +const handles: TestRepoHandle[] = []; +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.cleanup(); + } +}); + +const PEOPLE_CONFIG = `[gitsheet] +root = 'people' +path = '\${{ slug }}' + +[gitsheet.schema] +type = 'object' +required = ['slug'] + +[gitsheet.schema.properties.slug] +type = 'string' + +[gitsheet.schema.properties.middleName] +type = ['string', 'null'] + +[gitsheet.schema.properties.bio] +type = ['string', 'null'] +`; + +async function seededRepo(): Promise { + const fixture = await testRepo({ withInitialCommit: true }); + handles.push(fixture); + await mkdir(join(fixture.path, '.gitsheets'), { recursive: true }); + await writeFile(join(fixture.path, '.gitsheets', 'people.toml'), PEOPLE_CONFIG); + await fixture.git('add', '.gitsheets/'); + await fixture.git('commit', '-m', 'add people sheet'); + return fixture; +} + +/** + * A Standard Schema validator mimicking the `.nullable().optional()` + + * `?? null` pattern: every cleared optional field is normalized to an explicit + * `null` on the validated output — exactly what Zod's + * `z.string().nullable().optional().transform((v) => v ?? null)` (or a manual + * write-boundary `?? null`) produces. + */ +const nullNormalizingValidator: StandardSchemaV1> = { + '~standard': { + version: 1, + vendor: 'test', + validate(value: unknown) { + const v = value as Record; + return { + value: { + ...v, + middleName: v['middleName'] ?? null, + bio: v['bio'] ?? null, + }, + }; + }, + }, +}; + +describe('null/undefined-valued keys on write (#232)', () => { + it('a Standard Schema validator emitting nulls for cleared optionals writes cleanly, with no trace in the bytes', async () => { + const fixture = await seededRepo(); + const repo = await openRepo({ gitDir: fixture.gitDir }); + + // 2.x initially threw here ("cannot marshal JS value of type Null"). + const sheet = await repo.openSheet('people', { validator: nullNormalizingValidator }); + await sheet.upsert({ slug: 'jane', middleName: undefined }); + + // Read back: the cleared optionals are absent keys, not nulls. (Records + // carry non-enumerable-ish symbol metadata; compare string keys.) + const fresh = await repo.openSheet('people'); + const found = await fresh.queryFirst({ slug: 'jane' }); + expect(found).toMatchObject({ slug: 'jane' }); + expect(Object.keys(found ?? {})).toEqual(['slug']); + expect(found?.['middleName']).toBeUndefined(); + + // The on-disk bytes carry no trace — what 1.x @iarna/toml produced. + const { stdout: bytes } = await fixture.git('cat-file', 'blob', 'HEAD:people/jane.toml'); + expect(bytes).toBe('slug = "jane"\n'); + }); + + it('writing the null-cleared record over the stripped record is a byte-level no-op (no new commit)', async () => { + const fixture = await seededRepo(); + const repo = await openRepo({ gitDir: fixture.gitDir }); + + // First write: the record with the optionals simply never set. + const first = await repo.transact({ message: 'stripped' }, async (tx) => + tx.sheet('people').upsert({ slug: 'jane', bio: 'hi' }), + ); + expect(first.commitHash).toMatch(/^[0-9a-f]{40}$/); + + // Second write: logically the same record, but with cleared optionals as + // explicit nulls. Byte-identical canonical output ⇒ no-op transaction. + const second = await repo.transact({ message: 'null-cleared' }, async (tx) => + tx.sheet('people').upsert({ slug: 'jane', bio: 'hi', middleName: null }), + ); + expect(second.commitHash).toBeNull(); + }); + + it('drops nullish keys recursively: nested tables and objects inside arrays', async () => { + const fixture = await seededRepo(); + const repo = await openRepo({ gitDir: fixture.gitDir }); + + await repo.transact({ message: 'nested' }, async (tx) => + tx.sheet('people').upsert({ + slug: 'nested', + contact: { email: 'n@x.org', phone: null }, + roles: [{ title: 'chair', until: null }, { title: 'member' }], + }), + ); + + const sheet = await repo.openSheet('people'); + const found = await sheet.queryFirst({ slug: 'nested' }); + expect(found).toMatchObject({ + slug: 'nested', + contact: { email: 'n@x.org' }, + roles: [{ title: 'chair' }, { title: 'member' }], + }); + expect(Object.keys(found?.['contact'] as object)).toEqual(['email']); + expect(Object.keys((found?.['roles'] as object[])[0]!)).toEqual(['title']); + + const { stdout: bytes } = await fixture.git('cat-file', 'blob', 'HEAD:people/nested.toml'); + expect(bytes).not.toContain('phone'); + expect(bytes).not.toContain('until'); + }); + + it('a required field set to null fails validation as missing (absent == null)', async () => { + const fixture = await seededRepo(); + const repo = await openRepo({ gitDir: fixture.gitDir }); + + try { + await repo.transact({ message: 'bad' }, async (tx) => + tx.sheet('people').upsert({ slug: null, bio: 'x' } as never), + ); + throw new Error('should have thrown'); + } catch (err) { + expect(err).toBeInstanceOf(ValidationError); + const ve = err as ValidationError; + // The null-valued required key was dropped before validation, so the + // failure is "missing required property" — not a type error on null. + expect(ve.issues.some((i) => /required/i.test(i.message))).toBe(true); + } + }); + + it('a null array ELEMENT is rejected with the index named (not silently dropped)', async () => { + const fixture = await seededRepo(); + const repo = await openRepo({ gitDir: fixture.gitDir }); + + const headBefore = await repo.resolveRef('HEAD'); + await expect( + repo.transact({ message: 'bad' }, async (tx) => + tx.sheet('people').upsert({ slug: 'holes', tags: ['a', null, 'c'] }), + ), + ).rejects.toThrow(/array element \(index 1\)/); + + // No tree mutation happened. + expect(await repo.resolveRef('HEAD')).toBe(headBefore); + }); +}); From e6d4bf19fe50135f67ab9d82424a1851d68f6a91 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:24:13 -0400 Subject: [PATCH 09/19] docs(migration): add the 1.x -> 2.x (Rust core) breaking-changes section (#233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enumerates the three breaks that surfaced in the first production 1.x -> 2.x upgrade, each with before/after and the migration step: 1. hologit dropped — BlobObject.write + repo.hologitRepo replaced by repo.writeBlob(buf) + Sheet.setAttachments. 2. null/undefined-valued fields — the initial 2.x releases threw on marshal; fixed in this release to restore the specced 1.x drop semantics (recursive key drop), with the array-element edge case (error naming the index; 1.x silently dropped elements) called out. 3. Canonical byte re-baseline per #196 — the three value-preserving reformat classes and the one-time re-serialize commit recipe. Closes #233 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- docs/migration-guide.md | 101 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/docs/migration-guide.md b/docs/migration-guide.md index f14f8e7..db514d8 100644 --- a/docs/migration-guide.md +++ b/docs/migration-guide.md @@ -216,6 +216,107 @@ Patch release. Two fixes in service of snapshot-importer-style workflows. - `hologit` bumped from `^0.49.1` to `^0.50.2` to pick up `TreeObject.clearChildren()` (the O(1) primitive backing the new `Sheet#clear()`). +## v1.x → v2.0 (the Rust core) + +v2.0 replaces the Node.js engine with the Rust `gitsheets-core` crate: TOML +parse/serialize, normalization, validation, path templates, record CRUD, and +the tree/blob/commit substrate all run natively, and the npm package becomes a +thin marshalling shell over the `@gitsheets/core-napi` addon. The API surface +is intentionally unchanged — but three behavior changes bite real consumers. +All three surfaced during the first production 1.4.1 → 2.x upgrade; this +section is what turns "16 mystery test failures" into "read the section, apply +3 changes." + +### 1. The `hologit` dependency is gone + +2.x drops `hologit` entirely (tree/blob/commit ops run on `holo-tree`/gitoxide +*inside* the core). Anything that reached into it breaks: + +- `repo.hologitRepo` no longer exists. +- `BlobObject` (e.g. `BlobObject.write`) is no longer importable via gitsheets. + +The replacement for the common pattern — hashing binary content and attaching +it to a record — is the built-in blob primitive plus the attachment API: + +```typescript +// 1.x +import { BlobObject } from 'hologit'; + +const blob = await BlobObject.write(repo.hologitRepo, buffer); +await sheet.setAttachments(record, { 'avatar.jpg': blob }); +``` + +```typescript +// 2.x +const blob = await repo.writeBlob(buffer); // Promise +await sheet.setAttachments(record, { 'avatar.jpg': blob }); +``` + +`repo.writeBlob(buf)` hashes the bytes into the object database and returns a +`BlobHandle` accepted everywhere a hologit `BlobObject` used to be +(`setAttachment`, `setAttachments`). `getAttachment`/`getAttachments`/ +`sheet.attachments()` likewise return `BlobHandle`s with `.read()`/`.stream()`. + +### 2. `null`/`undefined`-valued fields + +TOML has no `null`, so a cleared optional field and a never-set field are the +same on-disk state: an absent key. 1.x (`@iarna/toml`) enforced this by +silently dropping null-valued keys at serialize time. **The initial 2.x +releases instead threw on marshal** (`cannot marshal JS value of type +Null/Undefined to a TOML value`) — breaking the standard consumer pattern of +`.nullable().optional()` schemas with `?? null` normalization on write. + +**Fixed in this release**: the 1.x drop semantics are restored and now +specced ([`specs/behaviors/normalization.md`](https://github.com/JarvusInnovations/gitsheets/blob/develop/specs/behaviors/normalization.md#null--undefined-handling)). +A `null`/`undefined`-valued key is dropped, recursively (top-level fields, +nested tables, and objects inside arrays), before validation and +serialization — byte-identical to 1.x output, in every binding (Node and +Python). If you shimmed this with a `stripNullish` helper at your write +boundary, you can delete it. + +One deliberate edge-case divergence from 1.x: a `null`/`undefined` **array +element** is an error naming the index (1.x silently dropped elements — +`[1, null, 2]` → `[1, 2]` — which shifts sibling indices and silently changes +data). Remove the element yourself if that's what you mean. A required field +set to `null` fails JSON-Schema validation as *missing*, same as 1.x. + +### 3. Canonical bytes re-baseline (one-time) + +The canonical serializer is now the core's `gitsheets-core::canonical` (the +Rust `toml` crate's formatting over a deep key sort), replacing `@iarna/toml`. +A record that was already canonical under 1.x may re-serialize to different — +but *value-identical* — bytes, once ([#196](https://github.com/JarvusInnovations/gitsheets/issues/196)). +Three reformat classes, all proven data-lossless and idempotent over a +~29.5k-record corpus ([PR #205](https://github.com/JarvusInnovations/gitsheets/pull/205)): + +1. **Integer digit-group underscores drop** — `legacyId = 31_618` → + `legacyId = 31618` (the dominant class). +2. **String requote** — strings containing both `"` and `'` move from escaped + single-line form to readable triple-quoted `"""…"""` form. +3. **Multiline trailing-quote layout** — a multiline string ending in `"` + loses `@iarna`'s line-continuation dance (same value, fewer lines). + +Until you re-baseline, the first 2.x write of a record whose 1.x bytes fall in +one of these classes shows a spurious-looking (but value-neutral) diff. The +recommended migration is a **one-time re-serialize commit** over each existing +repo, which is idempotent (a second run produces zero diff — that's the +adoption check): + +```bash +# Re-normalize every *.toml record under a directory, in place. +cargo run -p gitsheets-core --example normalize_tree -- path/to/records + +git add path/to/records +git commit -m "chore: re-baseline records to the Rust canonical form" + +# Verify idempotence — a second pass must report 0 files re-normalized: +cargo run -p gitsheets-core --example normalize_tree -- path/to/records +``` + +(Or use `git sheet normalize ` per sheet from the CLI.) See the +[canonical-form re-baseline notes](https://github.com/JarvusInnovations/gitsheets/blob/develop/specs/behaviors/normalization.md#canonical-form-re-baseline-the-rust-serializer) +for the full contract. + ## Going forward Once migrated, the recipes are the fastest path to common patterns: From 0d5db1c2c2168032dffea43c7611b072854b71b7 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:25:19 -0400 Subject: [PATCH 10/19] chore(plans): mark sheet-freshness-streaming done (PR #239) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- plans/sheet-freshness-streaming.md | 37 +++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/plans/sheet-freshness-streaming.md b/plans/sheet-freshness-streaming.md index fdbcd19..a38943d 100644 --- a/plans/sheet-freshness-streaming.md +++ b/plans/sheet-freshness-streaming.md @@ -1,5 +1,6 @@ --- -status: in-progress +status: done +pr: 239 depends: [] specs: - specs/behaviors/freshness.md @@ -82,21 +83,21 @@ lives inside a `CoreTransaction`). No core or napi changes. ## Validation -- [ ] Standing `Sheet`/`Store` reads reflect a `repo.transact` commit +- [x] Standing `Sheet`/`Store` reads reflect a `repo.transact` commit immediately after it resolves (no re-open) — vitest. -- [ ] External commit (second `Repository` instance) is invisible until +- [x] External commit (second `Repository` instance) is invisible until `sheet.refresh()` / `repo.refresh()` / `store.refresh()`, visible after — vitest. -- [ ] A commit onto a non-HEAD branch does **not** shift standing sheets — vitest. -- [ ] `findByIndex` after a commit reflects the new tree (lazy rebuild), and a +- [x] A commit onto a non-HEAD branch does **not** shift standing sheets — vitest. +- [x] `findByIndex` after a commit reflects the new tree (lazy rebuild), and a committed sheet-config change is visible after rebind — vitest. -- [ ] `refresh()` on a tx-bound sheet throws `TypeError` — vitest. -- [ ] `repo.readBlobStream` streams byte-identical content for a committed +- [x] `refresh()` on a tx-bound sheet throws `TypeError` — vitest. +- [x] `repo.readBlobStream` streams byte-identical content for a committed attachment; missing path / non-blob → `NotFoundError(record_not_found)`; bad ref → `RefError(ref_not_found)` — vitest. -- [ ] `sheet.getAttachmentStream` streams current bytes through the snapshot +- [x] `sheet.getAttachmentStream` streams current bytes through the snapshot (fresh after auto-refresh) and returns `null` when absent — vitest. -- [ ] Full suites green: package vitest + type-check, `cargo test`, napi +- [x] Full suites green: package vitest + type-check, `cargo test`, napi `node --test` (core untouched — suites prove no regression). ## Risks / unknowns @@ -114,8 +115,22 @@ lives inside a `CoreTransaction`). No core or napi changes. ## Notes -(Populated at closeout.) +- **Core statelessness confirmed** — the entire freshness model landed in the + Node host shell; `recordQuery`/`recordQueryCandidates`/`coreDiscoverSheets` + take `gitDir` + `treeRef` per call, so no core/napi change was needed and + the napi suite (101) + `cargo test` pass untouched. +- **Auto-refresh resolves HEAD, not the result tree** — keeps non-HEAD-branch + commits from shifting HEAD-bound sheets; covered by a dedicated test. +- **Indexes went fresh, not stale-pinned** — the existing `treeHashAtBuild` + comparison made #184's "indices stay at the snapshot" carve-out unnecessary; + rebinding invalidates builds and `findByIndex` lazily rebuilds. +- **Branch/PR churn** — the original branch (`feat/sheet-freshness-streaming`, + PR #238) picked up two unrelated commits from the parallel #232 work via a + shared working tree; rebased clean onto develop as + `feat/read-freshness-streaming` (PR #239). #238 closed as superseded. ## Follow-ups -(Populated at closeout.) +- Issue [#240](https://github.com/JarvusInnovations/gitsheets/issues/240) — + Python-binding parity: rebindable snapshot + `refresh()` + auto-refresh + + streaming blob read in `rust/gitsheets-py` (spec is binding-agnostic). From cbbb87e4bccfcb5bd31c52a5c299281090ffff28 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:27:58 -0400 Subject: [PATCH 11/19] chore(plans): mark marshal-drop-nullish done (PR #241) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- plans/marshal-drop-nullish.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/plans/marshal-drop-nullish.md b/plans/marshal-drop-nullish.md index 8de7748..01f04b6 100644 --- a/plans/marshal-drop-nullish.md +++ b/plans/marshal-drop-nullish.md @@ -1,10 +1,11 @@ --- -status: in-progress +status: done depends: [] specs: - specs/behaviors/normalization.md - specs/rust-core.md issues: [232, 233] +pr: 241 --- # Plan: drop null/undefined-valued keys at the marshal boundary @@ -84,21 +85,21 @@ Out of scope: ## Validation -- [ ] `cargo test` green across the workspace (core + bindings build, clippy +- [x] `cargo test` green across the workspace (core + bindings build, clippy `-D warnings` clean) -- [ ] napi suite (`npm test` in `rust/gitsheets-napi`) green, including new +- [x] napi suite (`npm test` in `rust/gitsheets-napi`) green, including new cases: null/undefined table keys dropped recursively (nested tables, tables inside arrays), serialized bytes identical to the pre-stripped record, null array element rejected with an error naming the index, top-level null still rejected -- [ ] Python suite (`pytest` in `rust/gitsheets-py`) green, including the +- [x] Python suite (`pytest` in `rust/gitsheets-py`) green, including the mirrored `None` cases and a cross-binding byte-parity case for a record with nullish keys -- [ ] Package vitest suite (`npm test`) green, including the end-to-end +- [x] Package vitest suite (`npm test`) green, including the end-to-end consumer pattern: `.nullable().optional()`-style Standard Schema validator emitting `null` for cleared optionals → `upsert` succeeds → read-back has no such keys → on-disk TOML bytes contain no trace of them -- [ ] `docs/migration-guide.md` gains the 1.x → 2.x section enumerating the +- [x] `docs/migration-guide.md` gains the 1.x → 2.x section enumerating the three breaks from #233 with before/after for each ## Risks / unknowns @@ -126,6 +127,14 @@ Out of scope: (`null_array_element_msg` / `null_scalar_msg`) so both bindings and any future one emit identical diagnostics; the drop recursion itself is ~6 lines per binding inside each marshal. +- Considered and rejected a `Value::Null` core variant: it would break the + value type's "mirrors TOML's type set exactly" invariant and force a Null + arm on every exhaustive match across ~12 core modules, for no gain — the + marshal recursion is the only place a host null is visible, and the + cross-binding byte-parity suite pins the contract. +- Verified totals at closeout: core 169, clippy `-D warnings` clean, napi 108, + pytest 31 (incl. the new `nullish` cross-binding parity fixture), vitest 298 + (gitsheets) + 118 (gitsheets-axi), type-check clean. ## Follow-ups From a9e7e2297d1f398ec40ffc7fed0be201283a6124 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:17:36 -0400 Subject: [PATCH 12/19] docs(specs): bytes attachments, repo.withLock, Standard Schema typing contract (#234, #236, #237) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - setAttachment(s) values accept raw Buffer/Uint8Array bytes (and UTF-8 strings) alongside BlobHandle — one-call attachment writes. - repo.withLock(fn): expose the transact write mutex for non-transact git ops; non-reentrant with lock_held deadlock guards (new TransactionError code). - Standard Schema type-level contract: gitsheets' declared StandardSchemaV1 mirrors the published interface exactly, so compliant Zod v4 / Valibot / ArkType schemas assign without casts, regression-guarded by a compile-time test. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- specs/api/errors.md | 1 + specs/api/repository.md | 20 ++++++++++++++++++++ specs/api/sheet.md | 6 ++++-- specs/api/store.md | 2 ++ specs/behaviors/attachments.md | 22 ++++++++++++++++------ specs/behaviors/transactions.md | 4 ++++ specs/behaviors/validation.md | 9 +++++++++ 7 files changed, 56 insertions(+), 8 deletions(-) diff --git a/specs/api/errors.md b/specs/api/errors.md index 6f83dc0..d811aa4 100644 --- a/specs/api/errors.md +++ b/specs/api/errors.md @@ -54,6 +54,7 @@ try { | `TransactionError` | `commit_failed` | 500 | `git commit-tree` / `update-ref` non-zero | | `TransactionError` | `push_daemon_running` | 409 | `repo.startPushDaemon` while one is already active | | `TransactionError` | `transaction_closed` | 409 | `tx.sheet(...)` after the transaction has been finalized or discarded | +| `TransactionError` | `lock_held` | 409 | `repo.withLock` / `repo.transact` attempted while the caller's own async context already holds the write lock — the lock is not reentrant | | `IndexError` | `index_unique_conflict` | 409 | Unique index would be violated | | `IndexError` | `index_not_defined` | 500 | `findByIndex` for an undeclared index | | `RefError` | `ref_not_found` | 404 | Resolution of a ref / commit-hash failed | diff --git a/specs/api/repository.md b/specs/api/repository.md index d36fb5e..e10adc3 100644 --- a/specs/api/repository.md +++ b/specs/api/repository.md @@ -107,6 +107,25 @@ stream.pipe(httpResponse); The typical consumer is an HTTP handler serving attachment bytes by key (see [behaviors/attachments.md](../behaviors/attachments.md#streaming-reads-by-keypath)); `Sheet.getAttachmentStream` is the sheet-scoped sibling. +### `repo.withLock(fn)` + +Run `fn` while holding the repository's **write lock** — the same in-process mutex that serializes `repo.transact`. For consumers coordinating non-transact git operations against the same repo (an external fetch + ref reset, a hot-reload that re-opens the store, raw plumbing reads that must not interleave with a commit), so they don't have to maintain a parallel lock that shadows gitsheets' own. + +```typescript +const result = await repo.withLock(async () => { + // no gitsheets transaction can start or commit while this runs + await execFile('git', ['fetch', 'origin'], { cwd: repo.gitDir }); + await execFile('git', ['update-ref', 'refs/heads/main', 'origin/main'], { cwd: repo.gitDir }); + return 'synced'; +}); +``` + +- Returns `Promise` where `T` is `fn`'s return type. `fn` may be sync or async. +- **Queueing**: contends FIFO with `repo.transact` calls (and permissive-mode auto-transactions) from other async contexts — whoever holds the lock runs alone; the lock is released when `fn` settles (resolve or throw). A throw from `fn` propagates after release. +- **Not reentrant — deliberately.** The lock has no hold-count. Calling `repo.withLock` inside a `withLock` callback, calling `repo.withLock` inside a `repo.transact` handler (the transaction already holds the lock), or calling `repo.transact` (or any permissive-mode mutation, which auto-opens a transaction) inside a `withLock` callback would self-deadlock — each is detected via async-context tracking and throws `TransactionError` (`lock_held`) instead of hanging. +- The lock is **in-process, per-`Repository`-instance** — the same scope as the transaction mutex ([behaviors/transactions.md](../behaviors/transactions.md#single-writer-model)). It does not coordinate across processes or across two `Repository` instances opened on the same git dir. + + ### `repo.requireExplicitTransactions()` Opt into strict mode. After this is called on a `Repository`, calling `Sheet.upsert` / `delete` / `patch` outside a transaction throws `TransactionError` with `code: 'transaction_required'`. @@ -163,6 +182,7 @@ When the handler stages no mutations, the transaction does **not** commit — `c | `RefError` | `ref_not_found` | `parent` ref doesn't exist; `readBlobStream` ref doesn't resolve | | `NotFoundError` | `record_not_found` | `readBlobStream` path absent under the ref, or not a blob | | `TransactionError` | `transaction_in_progress` | Another transaction is open on this repo | +| `TransactionError` | `lock_held` | `withLock` / `transact` attempted while the caller's own async context already holds the write lock (not reentrant) | | `TransactionError` | `commit_failed` | The underlying `git commit-tree` or `update-ref` failed | | `TransactionError` | `parent_moved` | Optimistic concurrency: parent ref moved between transaction start and commit | | `ConfigError` | `config_missing` | `.gitsheets/.toml` not found in `openSheet` | diff --git a/specs/api/sheet.md b/specs/api/sheet.md index a4fc881..3cf3311 100644 --- a/specs/api/sheet.md +++ b/specs/api/sheet.md @@ -202,8 +202,8 @@ Throws `IndexError` (`index_unique_conflict`) during a lazy build if uniqueness Binary blobs colocated with a record. ```typescript -await sheet.setAttachment(record, 'avatar.jpg', blob); -await sheet.setAttachments(record, { 'avatar.jpg': blob1, 'avatar-128.jpg': blob2 }); +await sheet.setAttachment(record, 'avatar.jpg', bufferOrBlob); +await sheet.setAttachments(record, { 'avatar.jpg': buffer1, 'avatar-128.jpg': blob2 }); const attachments = await sheet.getAttachments(record); // current low-level surface const avatar = await sheet.getAttachment(record, 'avatar.jpg'); @@ -212,6 +212,8 @@ await sheet.deleteAttachment(record, 'avatar.jpg'); // throws NotFoundError await sheet.deleteAttachments(record); // no-op if record has no attachment dir ``` +`setAttachment` / `setAttachments` values accept raw bytes (`Buffer` / `Uint8Array`), UTF-8 `string` content, or a `BlobHandle` from `repo.writeBlob` — the raw-bytes form makes the common "here are the bytes, attach them" case one call. See [behaviors/attachments.md](../behaviors/attachments.md#sheetsetattachmentrecord-name-content). + Streaming read without materializing the record: ```typescript diff --git a/specs/api/store.md b/specs/api/store.md index 030c88e..369c4a4 100644 --- a/specs/api/store.md +++ b/specs/api/store.md @@ -55,6 +55,8 @@ When a sheet has an entry in `validators`, its type flows through: - `store.` is `Sheet>` - `tx.` inside `store.transact` is the same Sheet type, scoped to the transaction's tree +A spec-compliant Standard Schema validator (Zod v4, Valibot, ArkType, …) assigns to a `validators` entry **directly — no `as` cast** ([behaviors/validation.md](../behaviors/validation.md#type-level-contract-no-casts-required), [#237](https://github.com/JarvusInnovations/gitsheets/issues/237)). + When a sheet does not have an entry in `validators`: - It is **not part of the typed surface** — the `Store` type includes only sheets named in `validators`, so property access on a non-validator sheet is a compile-time error and there's no autocomplete for it. diff --git a/specs/behaviors/attachments.md b/specs/behaviors/attachments.md index 6a744a4..8b6e393 100644 --- a/specs/behaviors/attachments.md +++ b/specs/behaviors/attachments.md @@ -31,25 +31,35 @@ The attachment name typically encodes the type via extension (`.jpg`, `.pdf`, `. ## API -### `sheet.setAttachment(record, name, blob)` +### `sheet.setAttachment(record, name, content)` Stage a single attachment. ```typescript -const blob = await repo.writeBlobFromFile('/path/to/avatar.jpg'); +// one call from raw bytes — no repo.writeBlob pre-step: +await sheet.setAttachment(record, 'avatar.jpg', uploadedBuffer); + +// or from an already-written blob handle: +const blob = await repo.writeBlob(bytes); await sheet.setAttachment(record, 'avatar.jpg', blob); ``` -`blob` is a hologit `BlobObject` (or whatever the new substrate provides — see the implementation note below). Helper methods exist for creating blobs from files, buffers, or streams. +`content` accepts any of: + +| Value type | Interpretation | +| --- | --- | +| `Buffer` / `Uint8Array` | Raw bytes — written to the object store as part of staging ([#234](https://github.com/JarvusInnovations/gitsheets/issues/234)) | +| `string` | UTF-8 text — encoded and written as bytes | +| `BlobHandle` | An already-written object-store blob (from `repo.writeBlob` or a diff) — reused by hash, no re-write | ### `sheet.setAttachments(record, map)` -Stage multiple attachments at once. +Stage multiple attachments at once. Values accept the same types as `setAttachment` — mixing is fine: ```typescript await sheet.setAttachments(record, { - 'avatar.jpg': avatarBlob, - 'avatar-128.jpg': thumbnailBlob, + 'avatar.jpg': avatarBuffer, // raw bytes + 'avatar-128.jpg': thumbnailBlob, // BlobHandle }); ``` diff --git a/specs/behaviors/transactions.md b/specs/behaviors/transactions.md index 97568c7..68967ad 100644 --- a/specs/behaviors/transactions.md +++ b/specs/behaviors/transactions.md @@ -21,6 +21,10 @@ One open transaction per `Repository` at a time, serialized by an in-process mut | **Nested** `repo.transact` — opening one inside another's handler (same async context) | Throws `TransactionError` (`transaction_in_progress`) immediately; it does not queue. Use `tx.sheet(name)` inside the handler instead. | | Concurrent `repo.transact` *and* a permissive-mode `Sheet.upsert` outside a transaction | The permissive `upsert` opens its own transaction, contends for the same mutex. | | Same-process concurrent reads | Reads don't take the mutex. They see the *committed* state (pre-transaction tree). After a successful commit, standing sheets rebind to the new tree — see [freshness.md](freshness.md). | +| `repo.withLock(fn)` from an independent async context | Contends FIFO for the same mutex as transactions; `fn` runs alone. See [api/repository.md](../api/repository.md#repowithlockfn). | +| `repo.withLock` inside a transaction handler, `repo.transact` (or a permissive-mode mutation) inside a `withLock` callback, or `withLock` inside `withLock` | Throws `TransactionError` (`lock_held`) immediately — the lock is not reentrant, and queueing would self-deadlock. | + +**The write lock is exposed.** `repo.withLock(fn)` runs consumer code under the same mutex, so out-of-band git operations (external fetch + ref reset, hot-reload re-opens, raw plumbing) serialize against transactions without a parallel consumer-maintained lock ([#236](https://github.com/JarvusInnovations/gitsheets/issues/236)). Multi-process / multi-host writers are explicitly out of scope. If another process commits to the same ref while a transaction is open, the transaction throws `TransactionError` (`parent_moved`) when it tries to commit. Detection is via comparing the parent ref's commit hash at transaction open vs. at commit. diff --git a/specs/behaviors/validation.md b/specs/behaviors/validation.md index c93c538..d24db3c 100644 --- a/specs/behaviors/validation.md +++ b/specs/behaviors/validation.md @@ -101,6 +101,15 @@ The transformed `result.value` (if the validator does coercion / transforms) bec The Standard Schema layer is optional. Without it, only JSON Schema runs. +### Type-level contract: no casts required + +gitsheets' exported `StandardSchemaV1` type (and every result/issue type it references) mirrors the [published Standard Schema v1 interface](https://standardschema.dev) **exactly** — including the `types` input/output metadata carrier on `~standard` and issue paths typed as `ReadonlyArray`. Since Standard Schema is the advertised integration point, the contract is: + +- A spec-compliant validator type (Zod v4, Valibot, ArkType, Effect Schema) **assigns directly** to `openSheet({ validator })` and to a `ValidatorMap` entry in `openStore({ validators })` — no `as` cast, no consumer-side wrapper. +- This is regression-guarded by a compile-time test that assigns real Zod v4 schemas without casts (any narrowing of the declared types that breaks spec-compliant assignability fails `type-check`). + +([#237](https://github.com/JarvusInnovations/gitsheets/issues/237) — the previous hand-rolled subset declared issue path keys as `string | number`, which rejected spec-compliant validators whose declared paths use `PropertyKey`.) + ## Order 1. Record arrives at `Sheet.upsert(record)` (or `Sheet.patch(query, partial)` after the merge step). From 0ff0a8694525381316bfd6a60881d1ea69b409bb Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:17:36 -0400 Subject: [PATCH 13/19] chore(plans): add consumer-api-ergonomics plan (#234, #236, #237) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- plans/consumer-api-ergonomics.md | 114 +++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 plans/consumer-api-ergonomics.md diff --git a/plans/consumer-api-ergonomics.md b/plans/consumer-api-ergonomics.md new file mode 100644 index 0000000..0e055da --- /dev/null +++ b/plans/consumer-api-ergonomics.md @@ -0,0 +1,114 @@ +--- +status: in-progress +depends: [] +specs: + - specs/behaviors/attachments.md + - specs/api/sheet.md + - specs/api/repository.md + - specs/api/errors.md + - specs/behaviors/transactions.md + - specs/behaviors/validation.md + - specs/api/store.md +issues: [234, 236, 237] +--- + +# Plan: Consumer API ergonomics — bytes attachments, withLock, Standard Schema typing + +## Scope + +The ergonomics batch from the first production 2.x migration +([#234](https://github.com/JarvusInnovations/gitsheets/issues/234), +[#236](https://github.com/JarvusInnovations/gitsheets/issues/236), +[#237](https://github.com/JarvusInnovations/gitsheets/issues/237)): + +**In:** + +- `Sheet.setAttachment` / `setAttachments` accept raw bytes + (`Buffer` / `Uint8Array`) alongside `string` and `BlobHandle` — one-call + attachment writes. +- `repo.withLock(fn)` — expose transact's write mutex for coordinating + non-transact git ops; **non-reentrant**, with async-context deadlock guards + throwing the new `TransactionError` code `lock_held`. +- Align the exported `StandardSchemaV1` types with the published Standard + Schema v1 interface so compliant Zod v4 schemas assign without `as` casts; + compile-time regression test against real Zod v4 (dev dependency). + +**Out:** + +- A `setAttachmentBytes` alias (redundant once `setAttachment` takes bytes; + the issue offered either). +- Cross-process locking (out of scope like the transaction mutex itself). +- #184/#235 — the parallel `sheet-freshness-streaming` plan (PR #238). + +## Implements + +- [`specs/behaviors/attachments.md`](../specs/behaviors/attachments.md) — + `setAttachment(record, name, content)` value-type table. +- [`specs/api/sheet.md`](../specs/api/sheet.md) — attachments signature note. +- [`specs/api/repository.md`](../specs/api/repository.md) — `repo.withLock(fn)`. +- [`specs/api/errors.md`](../specs/api/errors.md) — `lock_held` code row. +- [`specs/behaviors/transactions.md`](../specs/behaviors/transactions.md) — + single-writer table rows for `withLock`. +- [`specs/behaviors/validation.md`](../specs/behaviors/validation.md) — + "Type-level contract: no casts required". +- [`specs/api/store.md`](../specs/api/store.md) — validators assignability note. + +## Approach + +All host-shell (TypeScript); no Rust core / napi changes (`addon.writeBlob` +already takes a `Buffer`; the mutex and the type declarations are host +concerns). + +- **#234** — widen the attachment content type to + `string | Buffer | Uint8Array | BlobHandle`; in the tx-bound branch, hash + `Uint8Array` bytes through `addon.writeBlob` (Buffer passthrough, + `Buffer.from` copy for plain `Uint8Array`). +- **#236** — `withLock` acquires the existing `Mutex`; a module-level + `AsyncLocalStorage` lock context marks the callback's async scope. Guards: + `withLock` throws `lock_held` when called inside a lock context *or* a + transaction handler; `transact` throws `lock_held` when called inside a + lock context (permissive mutations auto-open transactions, so they're + covered by the same guard). +- **#237** — replace the hand-rolled `StandardSchemaV1` subset in + `validation.ts` with interfaces structurally identical to the published + spec (`Props.types` carrier, `PropertyKey` issue paths, `vendor`/`version`), + dropping the fake `__types__` prop; keep exported names. Host issue mapping + learns `symbol` path segments. Add `zod` (v4) as a dev dependency of the + `gitsheets` workspace and a vitest file whose *compilation* asserts direct + assignability (`satisfies ValidatorMap`, `InferRecord` inference) and whose + runtime asserts validate/transform/reject behavior through `openStore`. + +## Validation + +- [ ] `setAttachment(record, name, buffer)` and mixed-value `setAttachments` + write attachments byte-identical to the `repo.writeBlob` two-step — + vitest. +- [ ] `repo.withLock` serializes against `repo.transact` FIFO (observable + ordering), returns the callback's value, releases on throw — vitest. +- [ ] All three self-deadlock shapes throw `TransactionError('lock_held')` + immediately: `withLock` in `withLock`, `withLock` in a transact + handler, `transact` (and a permissive mutation) in `withLock` — vitest. +- [ ] Real Zod v4 object schemas assign to `openStore({ validators })` and + `openSheet({ validator })` with no casts; `InferRecord` yields the Zod + output type; suite compiles under `tsc --noEmit` — compile-time test. +- [ ] Zod validate/transform/reject flows work end-to-end through the typed + Store (transform reflected in written bytes; failure carries + `source: 'standard-schema'` issues) — vitest runtime. +- [ ] Full suites green: package vitest + type-check, `cargo test`, napi + `node --test` (core untouched). + +## Risks / unknowns + +- **Structural-typing drift vs future Zod majors** — the compile-time test + pins against whatever Zod v4 minor is installed; a future Zod type change + surfaces as a type-check failure here rather than in consumers. +- **`lock_held` is a new public error code** — additive; codes are stable + and new scenarios get new codes per specs/api/errors.md. + +## Notes + +(Populated at closeout.) + +## Follow-ups + +(Populated at closeout.) From 2a45eeee6d87c0ecada7ed5341c82700317aed8a Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:24:22 -0400 Subject: [PATCH 14/19] chore(deps): add zod v4 as gitsheets dev dependency (#237) Generated by: npm install -w gitsheets --save-dev zod Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- package-lock.json | 13 ++++++++++++- packages/gitsheets/package.json | 3 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1c15bcd..7d66c46 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1790,6 +1790,16 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/zod": { + "version": "4.4.3", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", + "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==", + "dev": true, + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } + }, "packages/gitsheets": { "version": "0.0.0-dev", "license": "Apache-2.0", @@ -1810,7 +1820,8 @@ "@types/yargs": "^17.0.35", "tmp": "^0.2.5", "typescript": "^6.0.3", - "vitest": "^4.1.6" + "vitest": "^4.1.6", + "zod": "^4.4.3" }, "engines": { "node": ">=20" diff --git a/packages/gitsheets/package.json b/packages/gitsheets/package.json index d7cebe5..4a62f06 100644 --- a/packages/gitsheets/package.json +++ b/packages/gitsheets/package.json @@ -53,6 +53,7 @@ "@types/yargs": "^17.0.35", "tmp": "^0.2.5", "typescript": "^6.0.3", - "vitest": "^4.1.6" + "vitest": "^4.1.6", + "zod": "^4.4.3" } } From 2c5b386375ae80eabd2e548c292b7b2a5b37896d Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:28:49 -0400 Subject: [PATCH 15/19] feat(api): bytes attachments, repo.withLock, cast-free Standard Schema typing (#234, #236, #237) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Sheet.setAttachment(s) values widen to string | Buffer | Uint8Array | BlobHandle — raw bytes are written to the object store during staging, making the common attach-these-bytes case one call (#234). - repo.withLock(fn) exposes the transact write mutex for non-transact git ops; FIFO with transactions, released on settle, and explicitly non-reentrant — withLock-in-withLock, withLock-in-transact, and transact-in-withLock all throw the new TransactionError('lock_held') instead of deadlocking, via an AsyncLocalStorage lock context (#236). - The exported StandardSchemaV1 types now mirror the published Standard Schema v1 interface exactly (PropertyKey issue paths, ~standard.types carrier), so compliant Zod v4 schemas assign with no as-cast; the compile-time contract is regression-guarded by store-zod.test.ts against real Zod v4, plus runtime validate/transform/reject coverage through the typed Store (#237). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- packages/gitsheets/src/errors.ts | 4 +- packages/gitsheets/src/index.ts | 3 + packages/gitsheets/src/repo-with-lock.test.ts | 174 ++++++++++++++++++ packages/gitsheets/src/repository.ts | 53 ++++++ .../src/sheet-attachment-bytes.test.ts | 130 +++++++++++++ packages/gitsheets/src/sheet.ts | 29 ++- packages/gitsheets/src/store-zod.test.ts | 126 +++++++++++++ packages/gitsheets/src/validation.ts | 33 ++-- 8 files changed, 533 insertions(+), 19 deletions(-) create mode 100644 packages/gitsheets/src/repo-with-lock.test.ts create mode 100644 packages/gitsheets/src/sheet-attachment-bytes.test.ts create mode 100644 packages/gitsheets/src/store-zod.test.ts diff --git a/packages/gitsheets/src/errors.ts b/packages/gitsheets/src/errors.ts index 1ab1f0a..125ee5c 100644 --- a/packages/gitsheets/src/errors.ts +++ b/packages/gitsheets/src/errors.ts @@ -21,6 +21,7 @@ const STATUS_BY_CODE = { index_not_defined: 500, push_daemon_running: 409, transaction_closed: 409, + lock_held: 409, ref_not_found: 404, not_an_ancestor: 409, path_render_failed: 422, @@ -77,7 +78,8 @@ export type TransactionErrorCode = | 'parent_moved' | 'commit_failed' | 'push_daemon_running' - | 'transaction_closed'; + | 'transaction_closed' + | 'lock_held'; export class TransactionError extends GitsheetsError { constructor(code: TransactionErrorCode, message: string, options?: GitsheetsErrorOptions) { diff --git a/packages/gitsheets/src/index.ts b/packages/gitsheets/src/index.ts index daeae86..142da17 100644 --- a/packages/gitsheets/src/index.ts +++ b/packages/gitsheets/src/index.ts @@ -27,6 +27,7 @@ export type { DiffOptions, DiffChange, AttachmentBlobHandle, + AttachmentContent, AttachmentEntry, } from './sheet.js'; @@ -82,6 +83,8 @@ export { validateRecord } from './validation.js'; export type { JSONSchema, StandardSchemaV1, + StandardSchemaProps, + StandardSchemaTypes, StandardSchemaIssue, StandardSchemaResult, StandardSchemaFailure, diff --git a/packages/gitsheets/src/repo-with-lock.test.ts b/packages/gitsheets/src/repo-with-lock.test.ts new file mode 100644 index 0000000..c620e6d --- /dev/null +++ b/packages/gitsheets/src/repo-with-lock.test.ts @@ -0,0 +1,174 @@ +// repo.withLock — the exposed write lock. See specs/api/repository.md#repowithlockfn +// and specs/behaviors/transactions.md (single-writer model). + +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { TransactionError } from './errors.js'; +import { openRepo, type Repository } from './repository.js'; +import { testRepo, type TestRepoHandle } from './test-helpers/test-repo.js'; + +const handles: TestRepoHandle[] = []; +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.cleanup(); + } +}); + +const USERS = `[gitsheet] +root = 'users' +path = '\${{ slug }}' +`; + +async function seededRepo(): Promise { + const fixture = await testRepo({ withInitialCommit: true }); + handles.push(fixture); + await mkdir(join(fixture.path, '.gitsheets'), { recursive: true }); + await writeFile(join(fixture.path, '.gitsheets', 'users.toml'), USERS); + await fixture.git('add', '.gitsheets/'); + await fixture.git('commit', '-m', 'add users sheet'); + return openRepo({ gitDir: fixture.gitDir }); +} + +describe('repo.withLock', () => { + it('returns the callback value (sync and async callbacks)', async () => { + const repo = await seededRepo(); + expect(await repo.withLock(() => 42)).toBe(42); + expect(await repo.withLock(async () => 'async')).toBe('async'); + }); + + it('releases on throw and propagates the error', async () => { + const repo = await seededRepo(); + await expect( + repo.withLock(() => { + throw new Error('boom'); + }), + ).rejects.toThrow('boom'); + // Lock must be free again — a transaction goes through. + const result = await repo.transact({ message: 'after-throw' }, async (tx) => { + await tx.sheet('users').upsert({ slug: 'ok' }); + }); + expect(result.commitHash).not.toBeNull(); + }); + + it('serializes against repo.transact — a transaction started during withLock commits after it', async () => { + const repo = await seededRepo(); + const order: string[] = []; + + let releaseHold!: () => void; + const hold = new Promise((resolve) => { + releaseHold = resolve; + }); + + const locked = repo.withLock(async () => { + order.push('lock-start'); + await hold; // keep the lock while the transact queues + order.push('lock-end'); + }); + + // Give withLock a tick to acquire before contending. + await new Promise((r) => setImmediate(r)); + + const txDone = repo + .transact({ message: 'queued behind lock' }, async (tx) => { + order.push('tx-run'); + await tx.sheet('users').upsert({ slug: 'queued' }); + }) + .then(() => order.push('tx-done')); + + // The transaction must not run while the lock is held. + await new Promise((r) => setTimeout(r, 50)); + expect(order).toEqual(['lock-start']); + + releaseHold(); + await Promise.all([locked, txDone]); + expect(order).toEqual(['lock-start', 'lock-end', 'tx-run', 'tx-done']); + }); + + it('queues behind an in-flight transaction from another async context', async () => { + const repo = await seededRepo(); + const order: string[] = []; + + let releaseTx!: () => void; + const holdTx = new Promise((resolve) => { + releaseTx = resolve; + }); + + const txDone = repo.transact({ message: 'holding' }, async (tx) => { + order.push('tx-start'); + await holdTx; + await tx.sheet('users').upsert({ slug: 'first' }); + }); + + // transact does async option/author resolution before acquiring the + // mutex — wait until the handler has provably started (lock held). + while (!order.includes('tx-start')) { + await new Promise((r) => setTimeout(r, 5)); + } + + const locked = repo + .withLock(() => { + order.push('lock-run'); + }) + .then(() => order.push('lock-done')); + + await new Promise((r) => setTimeout(r, 50)); + expect(order).toEqual(['tx-start']); + + releaseTx(); + await Promise.all([txDone, locked]); + expect(order[0]).toBe('tx-start'); + expect(order).toContain('lock-run'); + expect(order.indexOf('lock-run')).toBeGreaterThan(order.indexOf('tx-start')); + }); +}); + +describe('withLock non-reentrancy guards (lock_held)', () => { + async function expectLockHeld(p: Promise): Promise { + const err = await p.catch((e: unknown) => e); + expect(err).toBeInstanceOf(TransactionError); + expect((err as TransactionError).code).toBe('lock_held'); + } + + it('withLock inside withLock throws immediately', async () => { + const repo = await seededRepo(); + await repo.withLock(async () => { + await expectLockHeld(repo.withLock(() => 'inner')); + }); + }); + + it('repo.transact inside withLock throws immediately', async () => { + const repo = await seededRepo(); + await repo.withLock(async () => { + await expectLockHeld(repo.transact({ message: 'nested' }, async () => undefined)); + }); + }); + + it('a permissive-mode mutation inside withLock throws immediately (auto-transaction)', async () => { + const repo = await seededRepo(); + const users = await repo.openSheet('users'); + await repo.withLock(async () => { + await expectLockHeld(users.upsert({ slug: 'nope' })); + }); + }); + + it('withLock inside a transaction handler throws immediately', async () => { + const repo = await seededRepo(); + await repo.transact({ message: 'outer' }, async (tx) => { + await expectLockHeld(repo.withLock(() => 'inner')); + await tx.sheet('users').upsert({ slug: 'still-commits' }); + }); + }); + + it('two independent Repository instances do not trip each other\'s guard', async () => { + const repo = await seededRepo(); + const other = await openRepo({ gitDir: repo.gitDir }); + // Locks are per-instance (documented): other's withLock inside repo's + // withLock is allowed — they don't share a mutex. + const result = await repo.withLock(() => other.withLock(() => 'ok')); + expect(result).toBe('ok'); + }); +}); diff --git a/packages/gitsheets/src/repository.ts b/packages/gitsheets/src/repository.ts index 55949ad..aa32a3f 100644 --- a/packages/gitsheets/src/repository.ts +++ b/packages/gitsheets/src/repository.ts @@ -4,6 +4,7 @@ // remaining git shell-outs are genuine porcelain (ref resolution, sheet // discovery, author config). See specs/api/repository.md. +import { AsyncLocalStorage } from 'node:async_hooks'; import { execFile, spawn } from 'node:child_process'; import type { Readable } from 'node:stream'; import { promisify } from 'node:util'; @@ -32,6 +33,15 @@ import { EMPTY_TREE_HASH, makeBlobHandle, type BlobHandle } from './working-tree const exec = promisify(execFile); +/** + * Marks async contexts that hold the repository write lock via + * `repo.withLock`. Used to detect self-deadlock shapes (withLock-in-withLock, + * transact-in-withLock) and throw TransactionError('lock_held') instead of + * queueing forever. Module-level like `transactionContext` — the stored value + * is the Repository whose lock is held. + */ +const lockContext = new AsyncLocalStorage(); + export interface OpenRepoOptions { /** Path to a `.git` directory. If omitted, discovered from the cwd upward. */ readonly gitDir?: string; @@ -197,6 +207,40 @@ export class Repository { return child.stdout; } + /** + * Run `fn` while holding the repository's write lock — the same in-process + * mutex that serializes repo.transact. For coordinating non-transact git + * operations (external fetch + ref reset, hot reloads, raw plumbing) so + * consumers don't need a parallel lock shadowing gitsheets' own. + * + * Contends FIFO with transactions from other async contexts; released when + * `fn` settles (a throw propagates after release). NOT reentrant: calling + * withLock inside withLock, withLock inside a transact handler, or transact + * (incl. permissive-mode mutations) inside withLock throws + * TransactionError('lock_held') instead of deadlocking. + * See specs/api/repository.md#repowithlockfn. + */ + async withLock(fn: () => T | Promise): Promise { + if (lockContext.getStore() === this) { + throw new TransactionError( + 'lock_held', + 'repo.withLock inside repo.withLock would deadlock — the write lock is not reentrant', + ); + } + if (transactionContext.getStore() !== undefined) { + throw new TransactionError( + 'lock_held', + 'repo.withLock inside a transaction handler would deadlock — the transaction already holds the write lock', + ); + } + const release = await this.#mutex.acquire(); + try { + return await lockContext.run(this, () => fn()); + } finally { + release(); + } + } + /** Resolve a ref or commit hash. Returns the full commit hash or null. */ async resolveRef(ref: string): Promise { return resolveCommit(this.#gitDir, ref); @@ -285,6 +329,15 @@ export class Repository { 'nested repo.transact is not allowed — use tx.sheet(name) inside the handler', ); } + if (lockContext.getStore() === this) { + // Queueing would self-deadlock: the caller's own async context already + // holds the write lock via repo.withLock. (Permissive-mode Sheet + // mutations auto-open transactions, so they hit this guard too.) + throw new TransactionError( + 'lock_held', + 'repo.transact inside repo.withLock would deadlock — the write lock is already held by this context and is not reentrant', + ); + } const normalized = Transaction.normalizeOptions(opts); const author = await resolveAuthor(this.#gitDir, normalized.author); diff --git a/packages/gitsheets/src/sheet-attachment-bytes.test.ts b/packages/gitsheets/src/sheet-attachment-bytes.test.ts new file mode 100644 index 0000000..42213ce --- /dev/null +++ b/packages/gitsheets/src/sheet-attachment-bytes.test.ts @@ -0,0 +1,130 @@ +// One-call attachment writes from raw bytes (#234). +// See specs/behaviors/attachments.md#sheetsetattachmentrecord-name-content. + +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { openRepo, type Repository } from './repository.js'; +import { testRepo, type TestRepoHandle } from './test-helpers/test-repo.js'; + +const handles: TestRepoHandle[] = []; +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.cleanup(); + } +}); + +const USERS = `[gitsheet] +root = 'users' +path = '\${{ slug }}' +`; + +async function seededRepo(): Promise { + const fixture = await testRepo({ withInitialCommit: true }); + handles.push(fixture); + await mkdir(join(fixture.path, '.gitsheets'), { recursive: true }); + await writeFile(join(fixture.path, '.gitsheets', 'users.toml'), USERS); + await fixture.git('add', '.gitsheets/'); + await fixture.git('commit', '-m', 'add users sheet'); + return openRepo({ gitDir: fixture.gitDir }); +} + +/** Binary bytes (not valid UTF-8) to prove byte fidelity. */ +const BINARY = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0xff]); + +describe('setAttachment with raw bytes (#234)', () => { + it('accepts a Buffer directly — no repo.writeBlob pre-step', async () => { + const repo = await seededRepo(); + await repo.transact({ message: 'avatar from buffer' }, async (tx) => { + const sheet = tx.sheet('users'); + await sheet.upsert({ slug: 'jane' }); + await sheet.setAttachment('jane', 'avatar.png', BINARY); + }); + + const users = await repo.openSheet('users'); + const blob = await users.getAttachment('jane', 'avatar.png'); + expect(blob).not.toBeNull(); + expect(Buffer.compare(await blob!.read(), BINARY)).toBe(0); + }); + + it('accepts a plain Uint8Array', async () => { + const repo = await seededRepo(); + const bytes = new Uint8Array([1, 2, 3, 254, 255]); + await repo.transact({ message: 'uint8array' }, async (tx) => { + const sheet = tx.sheet('users'); + await sheet.upsert({ slug: 'jane' }); + await sheet.setAttachment('jane', 'data.bin', bytes); + }); + + const users = await repo.openSheet('users'); + const blob = await users.getAttachment('jane', 'data.bin'); + expect(Buffer.compare(await blob!.read(), Buffer.from(bytes))).toBe(0); + }); + + it('a subarray view writes only the viewed bytes', async () => { + const repo = await seededRepo(); + const backing = new Uint8Array([9, 9, 1, 2, 3, 9, 9]); + const view = backing.subarray(2, 5); // [1, 2, 3] + await repo.transact({ message: 'view' }, async (tx) => { + const sheet = tx.sheet('users'); + await sheet.upsert({ slug: 'jane' }); + await sheet.setAttachment('jane', 'view.bin', view); + }); + + const users = await repo.openSheet('users'); + const blob = await users.getAttachment('jane', 'view.bin'); + expect(Buffer.compare(await blob!.read(), Buffer.from([1, 2, 3]))).toBe(0); + }); + + it('bytes produce the same blob hash as the writeBlob two-step', async () => { + const repo = await seededRepo(); + const viaWriteBlob = await repo.writeBlob(BINARY); + + await repo.transact({ message: 'one-call' }, async (tx) => { + const sheet = tx.sheet('users'); + await sheet.upsert({ slug: 'jane' }); + await sheet.setAttachment('jane', 'avatar.png', BINARY); + }); + + const users = await repo.openSheet('users'); + const blob = await users.getAttachment('jane', 'avatar.png'); + expect(blob!.hash).toBe(viaWriteBlob.hash); + }); + + it('setAttachments accepts mixed value types in one call', async () => { + const repo = await seededRepo(); + const handle = await repo.writeBlob(Buffer.from('via-handle')); + await repo.transact({ message: 'mixed' }, async (tx) => { + const sheet = tx.sheet('users'); + await sheet.upsert({ slug: 'jane' }); + await sheet.setAttachments('jane', { + 'a.bin': BINARY, // Buffer + 'b.bin': new Uint8Array([7, 8, 9]), // Uint8Array + 'c.txt': 'plain text', // string (UTF-8) + 'd.bin': handle, // BlobHandle + }); + }); + + const users = await repo.openSheet('users'); + expect(Buffer.compare(await (await users.getAttachment('jane', 'a.bin'))!.read(), BINARY)).toBe(0); + expect(Buffer.compare(await (await users.getAttachment('jane', 'b.bin'))!.read(), Buffer.from([7, 8, 9]))).toBe(0); + expect((await (await users.getAttachment('jane', 'c.txt'))!.read()).toString()).toBe('plain text'); + expect((await (await users.getAttachment('jane', 'd.bin'))!.read()).toString()).toBe('via-handle'); + }); + + it('works through the permissive-mode standalone path too', async () => { + const repo = await seededRepo(); + const users = await repo.openSheet('users'); + await users.upsert({ slug: 'jane' }); + await users.setAttachment('jane', 'avatar.png', BINARY); + + // Re-open to read the committed state (this branch predates the + // freshness auto-refresh shipping in PR #239). + const fresh = await repo.openSheet('users'); + const blob = await fresh.getAttachment('jane', 'avatar.png'); + expect(Buffer.compare(await blob!.read(), BINARY)).toBe(0); + }); +}); diff --git a/packages/gitsheets/src/sheet.ts b/packages/gitsheets/src/sheet.ts index 60d3930..99db739 100644 --- a/packages/gitsheets/src/sheet.ts +++ b/packages/gitsheets/src/sheet.ts @@ -463,6 +463,14 @@ export interface AttachmentEntry { readonly blob: AttachmentBlobHandle; } +/** + * Value accepted by `Sheet.setAttachment` / `setAttachments`: raw bytes + * (`Buffer` / `Uint8Array`), UTF-8 `string` content, or an already-written + * `BlobHandle` from `repo.writeBlob` (reused by hash, no re-write). + * See specs/behaviors/attachments.md (#234). + */ +export type AttachmentContent = string | Buffer | Uint8Array | BlobHandle; + // Minimum-viable MIME map — covers the bulk of typical attachment uses // (images, audio, video, docs). Unknown extensions get application/octet-stream. const MIME_BY_EXT: Readonly> = { @@ -1528,14 +1536,14 @@ export class Sheet { async setAttachment( record: T | string, name: string, - blob: string | BlobHandle, + content: AttachmentContent, ): Promise { - await this.setAttachments(record, { [name]: blob }); + await this.setAttachments(record, { [name]: content }); } async setAttachments( record: T | string, - attachments: Record, + attachments: Record, ): Promise { if (this.#transaction === undefined) { this.#checkStrictMode(); @@ -1556,12 +1564,21 @@ export class Sheet { this.#ensureCoreSheetOpened(); const map: Record = {}; for (const [aName, content] of Object.entries(attachments)) { - // A string is hashed as its UTF-8 bytes; a BlobHandle already names an - // ODB blob (from repo.writeBlob or a diff), so reuse its hash directly. + // Raw bytes (Buffer/Uint8Array) and strings (UTF-8 bytes) are written to + // the object store here — the one-call attachment write (#234). A + // BlobHandle already names an ODB blob (from repo.writeBlob or a diff), + // so its hash is reused directly. map[aName] = typeof content === 'string' ? callCore(() => addon.writeBlob(this.#gitDir, Buffer.from(content, 'utf8'))) - : content.hash; + : content instanceof Uint8Array + ? callCore(() => + addon.writeBlob( + this.#gitDir, + Buffer.isBuffer(content) ? content : Buffer.from(content), + ), + ) + : content.hash; } callCore(() => this.#transaction!.coreTx.setAttachments(this.#name, recordPath, map)); } diff --git a/packages/gitsheets/src/store-zod.test.ts b/packages/gitsheets/src/store-zod.test.ts new file mode 100644 index 0000000..b6359fb --- /dev/null +++ b/packages/gitsheets/src/store-zod.test.ts @@ -0,0 +1,126 @@ +// Zod v4 ↔ Standard Schema typing contract (#237). +// +// The COMPILATION of this file is the type-level regression test: real Zod v4 +// schemas must assign to gitsheets' StandardSchemaV1 / ValidatorMap / the +// openStore + openSheet option types with NO `as` casts. If a change to the +// declared Standard Schema types breaks spec-compliant assignability, this +// file fails `tsc --noEmit` (and vitest's transform). The runtime half proves +// the same schemas validate / transform / reject end-to-end. +// +// See specs/behaviors/validation.md#type-level-contract-no-casts-required. + +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; + +import { afterEach, describe, expect, expectTypeOf, it } from 'vitest'; +import { z } from 'zod'; + +import { ValidationError } from './errors.js'; +import { openRepo, type Repository } from './repository.js'; +import { openStore, type InferRecord, type ValidatorMap } from './store.js'; +import type { StandardSchemaV1 } from './validation.js'; + +const handles: Array<{ cleanup: () => Promise }> = []; +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.cleanup(); + } +}); + +const UserSchema = z.object({ + slug: z.string(), + email: z.string(), + displayName: z.string().default('Anonymous'), +}); +type User = z.output; + +// --- Type-level assertions (checked at compile time) --- + +// 1. A Zod v4 schema IS a gitsheets StandardSchemaV1 — direct assignment, no cast. +const asStandard: StandardSchemaV1 = UserSchema; +void asStandard; + +// 2. A map of Zod schemas satisfies ValidatorMap — the openStore validators shape. +const validators = { users: UserSchema } satisfies ValidatorMap; + +// 3. InferRecord recovers the Zod OUTPUT type (post-transform/default). +expectTypeOf>().toEqualTypeOf(); + +// --- Runtime fixture --- + +const USERS = `[gitsheet] +root = 'users' +path = '\${{ slug }}' +`; + +async function seededRepo(): Promise { + const { testRepo } = await import('./test-helpers/test-repo.js'); + const fixture = await testRepo({ withInitialCommit: true }); + handles.push(fixture); + await mkdir(join(fixture.path, '.gitsheets'), { recursive: true }); + await writeFile(join(fixture.path, '.gitsheets', 'users.toml'), USERS); + await fixture.git('add', '.gitsheets/'); + await fixture.git('commit', '-m', 'add users sheet'); + return openRepo({ gitDir: fixture.gitDir }); +} + +describe('Zod v4 schemas as gitsheets validators (#237)', () => { + it('openStore accepts Zod validators with no cast and types flow through', async () => { + const repo = await seededRepo(); + const store = await openStore(repo, { validators }); + + await store.transact({ message: 'add jane' }, async (tx) => { + await tx.users.upsert({ slug: 'jane', email: 'jane@x.org', displayName: 'Jane' }); + }); + + const fresh = await openStore(repo, { validators }); + const jane = await fresh.users.queryFirst({ slug: 'jane' }); + expectTypeOf(jane).toEqualTypeOf(); + expect(jane).toMatchObject({ slug: 'jane', email: 'jane@x.org' }); + }); + + it('openSheet accepts a Zod validator with no cast', async () => { + const repo = await seededRepo(); + const users = await repo.openSheet('users', { validator: UserSchema }); + await users.upsert({ slug: 'zed', email: 'zed@x.org', displayName: 'Zed' }); + + const fresh = await repo.openSheet('users', { validator: UserSchema }); + const zed = await fresh.queryFirst({ slug: 'zed' }); + expect(zed).toMatchObject({ slug: 'zed' }); + }); + + it('Zod transforms (defaults) are reflected in the written record', async () => { + const repo = await seededRepo(); + const store = await openStore(repo, { validators }); + + await store.transact({ message: 'defaulted' }, async (tx) => { + // displayName omitted — Zod's .default() fills it during validation. + // Sheet is typed on the validator's OUTPUT; passing the pre-transform + // input shape is runtime-supported but needs an explicit widening here. + const input: z.input = { slug: 'anon', email: 'anon@x.org' }; + await tx.users.upsert(input as User); + }); + + const fresh = await openStore(repo, { validators }); + const anon = await fresh.users.queryFirst({ slug: 'anon' }); + expect(anon?.displayName).toBe('Anonymous'); + }); + + it('Zod rejections surface as ValidationError with standard-schema issues', async () => { + const repo = await seededRepo(); + const store = await openStore(repo, { validators }); + + const err = await store + .transact({ message: 'bad' }, async (tx) => { + await tx.users.upsert({ slug: 'bad', email: 123 as unknown as string, displayName: 'x' }); + }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(ValidationError); + const issues = (err as ValidationError).issues; + expect(issues.length).toBeGreaterThan(0); + expect(issues[0]!.source).toBe('standard-schema'); + expect(issues[0]!.path).toEqual(['email']); + }); +}); diff --git a/packages/gitsheets/src/validation.ts b/packages/gitsheets/src/validation.ts index 92475f0..9ceb121 100644 --- a/packages/gitsheets/src/validation.ts +++ b/packages/gitsheets/src/validation.ts @@ -25,12 +25,16 @@ export type JSONSchema = Record; // Subset of the Standard Schema v1 interface used here. Consumers may pass any // validator (Zod, Valibot, ArkType, Effect Schema) that implements `~standard`. // See https://standardschema.dev for the full interface. +// The declarations mirror the published interface EXACTLY (issue path keys +// are `PropertyKey`, results carry the optional `types` metadata object) so a +// compliant validator's own types assign here with no `as` cast — see +// specs/behaviors/validation.md#type-level-contract-no-casts-required (#237). export interface StandardSchemaPathSegment { - readonly key: string | number; + readonly key: PropertyKey; } export interface StandardSchemaIssue { readonly message: string; - readonly path?: ReadonlyArray; + readonly path?: ReadonlyArray | undefined; } export interface StandardSchemaSuccess { readonly value: Output; @@ -40,16 +44,21 @@ export interface StandardSchemaFailure { readonly issues: ReadonlyArray; } export type StandardSchemaResult = StandardSchemaSuccess | StandardSchemaFailure; +/** The compile-time-only Input/Output carrier on `~standard.types`. */ +export interface StandardSchemaTypes { + readonly input: Input; + readonly output: Output; +} +export interface StandardSchemaProps { + readonly version: 1; + readonly vendor: string; + readonly validate: ( + value: unknown, + ) => StandardSchemaResult | Promise>; + readonly types?: StandardSchemaTypes | undefined; +} export interface StandardSchemaV1 { - readonly '~standard': { - readonly version: 1; - readonly vendor: string; - readonly validate: ( - value: unknown, - ) => StandardSchemaResult | Promise>; - }; - /** unused — only here so consumers' generic type can flow Input/Output. */ - readonly __types__?: { input: Input; output: Output }; + readonly '~standard': StandardSchemaProps; } // --- Public validate --- @@ -121,7 +130,7 @@ function coreIssueToIssue(issue: { function standardIssueToIssue(issue: StandardSchemaIssue): ValidationIssue { const path: string[] = []; for (const seg of issue.path ?? []) { - if (typeof seg === 'string' || typeof seg === 'number') { + if (typeof seg === 'string' || typeof seg === 'number' || typeof seg === 'symbol') { path.push(String(seg)); } else if (seg && typeof seg === 'object' && 'key' in seg) { path.push(String(seg.key)); From 161b8d04816cfa85adb7df618f82d48b5ce56f48 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:28:49 -0400 Subject: [PATCH 16/19] docs(api): document bytes attachments, withLock, and cast-free validator typing Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- docs/api.md | 7 ++++--- docs/validation.md | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/api.md b/docs/api.md index 6adcca3..c98b7fe 100644 --- a/docs/api.md +++ b/docs/api.md @@ -80,6 +80,7 @@ const store = await openStore(repo, { | `repo.openSheet(name, opts?)` | Sheet handle; `opts.validator` (Standard Schema), `opts.root`, `opts.prefix` | | `repo.openSheets(opts?)` | All sheets keyed by name; `opts.root`, `opts.prefix` | | `repo.transact(opts, handler)` | Run a handler in a single-commit transaction | +| `repo.withLock(fn)` | Run `fn` holding the write lock transact uses — coordinate non-transact git ops. Not reentrant (`lock_held`) | | `repo.requireExplicitTransactions()` | Switch to strict mode (one-way) | | `repo.startPushDaemon(opts)` | Start async push to a configured remote | | `repo.resolveRef(ref)` | Resolve a ref or commit hash; returns `string \| null` | @@ -128,8 +129,8 @@ All write methods route through a transaction — permissive mode auto-opens one | `sheet.getAttachment(record, name)` | One attachment's BlobObject or null | | `sheet.getAttachments(record)` | Map of name → BlobObject | | `sheet.getAttachmentStream(recordOrPath, name)` | `Promise` — stream an attachment's bytes without materializing the record | -| `sheet.setAttachment(record, name, blob)` | Add or replace | -| `sheet.setAttachments(record, map)` | Bulk variant | +| `sheet.setAttachment(record, name, content)` | Add or replace — `content` may be raw bytes (`Buffer`/`Uint8Array`), a UTF-8 `string`, or a `BlobHandle` | +| `sheet.setAttachments(record, map)` | Bulk variant; values accept the same types, mixed freely | | `sheet.deleteAttachment(record, name)` | Remove a single attachment; throws `NotFoundError` if missing | | `sheet.deleteAttachments(record)` | Remove all attachments; idempotent no-op when no attachment dir | | `sheet.attachments(record)` | `AsyncGenerator` yielding `{name, mimeType, blob}` with `.read()` / `.stream()` | @@ -231,7 +232,7 @@ Subclasses: | --- | --- | | `ConfigError` | `config_missing`, `config_invalid` | | `ValidationError` | `validation_failed` (carries `issues: ValidationIssue[]`) | -| `TransactionError` | `transaction_in_progress`, `transaction_required`, `parent_moved`, `commit_failed`, `push_daemon_running`, `transaction_closed` | +| `TransactionError` | `transaction_in_progress`, `transaction_required`, `parent_moved`, `commit_failed`, `push_daemon_running`, `transaction_closed`, `lock_held` | | `IndexError` | `index_unique_conflict`, `index_not_defined` (carries `conflictingPaths`) | | `RefError` | `ref_not_found`, `not_an_ancestor` | | `PathTemplateError` | `path_render_failed`, `path_invalid_chars` | diff --git a/docs/validation.md b/docs/validation.md index d480bb8..f4130ca 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -98,6 +98,8 @@ Internally the library calls `validator['~standard'].validate(record)`. The retu The Standard Schema layer is **optional**. Without it, only JSON Schema runs. +Gitsheets' declared `StandardSchemaV1` types mirror the published Standard Schema v1 interface exactly, so a compliant validator (Zod v4, Valibot, ArkType, Effect Schema) assigns to `openSheet({ validator })` / `openStore({ validators })` directly — no `as` cast or wrapper needed. + For full type inference across reads and writes, use [`openStore`](concepts.md#store) instead of `openSheet` — see the [typed-sheet-with-Zod recipe](recipes/typed-sheet-with-zod.md). ## Order of operations From aa6a7df843e2b51165a30096cd22bd52a0e98f60 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:31:54 -0400 Subject: [PATCH 17/19] chore(plans): mark consumer-api-ergonomics done (PR #242) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- plans/consumer-api-ergonomics.md | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/plans/consumer-api-ergonomics.md b/plans/consumer-api-ergonomics.md index 0e055da..29657da 100644 --- a/plans/consumer-api-ergonomics.md +++ b/plans/consumer-api-ergonomics.md @@ -1,5 +1,6 @@ --- -status: in-progress +status: done +pr: 242 depends: [] specs: - specs/behaviors/attachments.md @@ -80,21 +81,21 @@ concerns). ## Validation -- [ ] `setAttachment(record, name, buffer)` and mixed-value `setAttachments` +- [x] `setAttachment(record, name, buffer)` and mixed-value `setAttachments` write attachments byte-identical to the `repo.writeBlob` two-step — vitest. -- [ ] `repo.withLock` serializes against `repo.transact` FIFO (observable +- [x] `repo.withLock` serializes against `repo.transact` FIFO (observable ordering), returns the callback's value, releases on throw — vitest. -- [ ] All three self-deadlock shapes throw `TransactionError('lock_held')` +- [x] All three self-deadlock shapes throw `TransactionError('lock_held')` immediately: `withLock` in `withLock`, `withLock` in a transact handler, `transact` (and a permissive mutation) in `withLock` — vitest. -- [ ] Real Zod v4 object schemas assign to `openStore({ validators })` and +- [x] Real Zod v4 object schemas assign to `openStore({ validators })` and `openSheet({ validator })` with no casts; `InferRecord` yields the Zod output type; suite compiles under `tsc --noEmit` — compile-time test. -- [ ] Zod validate/transform/reject flows work end-to-end through the typed +- [x] Zod validate/transform/reject flows work end-to-end through the typed Store (transform reflected in written bytes; failure carries `source: 'standard-schema'` issues) — vitest runtime. -- [ ] Full suites green: package vitest + type-check, `cargo test`, napi +- [x] Full suites green: package vitest + type-check, `cargo test`, napi `node --test` (core untouched). ## Risks / unknowns @@ -107,8 +108,20 @@ concerns). ## Notes -(Populated at closeout.) +- **Zod v4 assignability held with zero implementation-side casts** once the + declared types matched the published interface — the only friction found + was `Sheet`'s output-only typing (see Follow-ups). +- **`lock_held` guards are per-`Repository`-instance** (`lockContext` stores + the instance): two Repositories on one git dir don't trip each other's + guard — matching the documented per-instance lock scope, covered by a test. +- **transact resolves options/author *before* acquiring the mutex** — an + ordering-test gotcha (a queued transaction isn't "in line" until those + awaits finish), noted in `repo-with-lock.test.ts`. +- Suites: gitsheets vitest 312, axi vitest 118 + node-test 101, type-check + clean; rust tree untouched (validated green in PR #239's clean worktree). ## Follow-ups -(Populated at closeout.) +- Issue [#243](https://github.com/JarvusInnovations/gitsheets/issues/243) — + thread the validator's Input type through Sheet write methods (upsert of a + pre-transform shape currently needs a widening cast). From 0aa4a819acbfc0b39c646f021144ace3dde81df7 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 4 Jul 2026 15:32:10 -0400 Subject: [PATCH 18/19] chore(plans): fix sibling-PR reference in consumer-api-ergonomics Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F5fwEknfSxpaGCVEME5D4h --- plans/consumer-api-ergonomics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plans/consumer-api-ergonomics.md b/plans/consumer-api-ergonomics.md index 29657da..7ea8e2b 100644 --- a/plans/consumer-api-ergonomics.md +++ b/plans/consumer-api-ergonomics.md @@ -39,7 +39,7 @@ The ergonomics batch from the first production 2.x migration - A `setAttachmentBytes` alias (redundant once `setAttachment` takes bytes; the issue offered either). - Cross-process locking (out of scope like the transaction mutex itself). -- #184/#235 — the parallel `sheet-freshness-streaming` plan (PR #238). +- #184/#235 — the parallel `sheet-freshness-streaming` plan (PR #239). ## Implements From c5054d3b744abff56c29751e432bd29692d5cf41 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sun, 5 Jul 2026 03:05:53 -0400 Subject: [PATCH 19/19] docs(specs): correct attachment handle naming to shipped types (#244) The writeBlobFromFile example was already superseded by #242's bytes-accepting setAttachment rewrite; this sweeps the residual hologit-era BlobObject references to the shipped BlobHandle / AttachmentBlobHandle types. Deliberately no file-path helper: setAttachment(bytes) + readFile covers the case. Closes #244 Co-Authored-By: Claude Fable 5 --- docs/api.md | 4 +-- plans/spec-attachment-handle-drift.md | 40 +++++++++++++++++++++++++++ specs/api/sheet.md | 4 +-- specs/behaviors/attachments.md | 2 +- 4 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 plans/spec-attachment-handle-drift.md diff --git a/docs/api.md b/docs/api.md index c98b7fe..7b0f13e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -126,8 +126,8 @@ All write methods route through a transaction — permissive mode auto-opens one | Method | Purpose | | --- | --- | -| `sheet.getAttachment(record, name)` | One attachment's BlobObject or null | -| `sheet.getAttachments(record)` | Map of name → BlobObject | +| `sheet.getAttachment(record, name)` | One attachment's `AttachmentBlobHandle` or null | +| `sheet.getAttachments(record)` | Map of name → `AttachmentBlobHandle` | | `sheet.getAttachmentStream(recordOrPath, name)` | `Promise` — stream an attachment's bytes without materializing the record | | `sheet.setAttachment(record, name, content)` | Add or replace — `content` may be raw bytes (`Buffer`/`Uint8Array`), a UTF-8 `string`, or a `BlobHandle` | | `sheet.setAttachments(record, map)` | Bulk variant; values accept the same types, mixed freely | diff --git a/plans/spec-attachment-handle-drift.md b/plans/spec-attachment-handle-drift.md new file mode 100644 index 0000000..1d024ee --- /dev/null +++ b/plans/spec-attachment-handle-drift.md @@ -0,0 +1,40 @@ +--- +status: done +depends: [] +specs: + - specs/behaviors/attachments.md + - specs/api/sheet.md +issues: [244] +pr: 246 +--- + +# Close out attachment-handle spec↔code drift (#244) + +## Scope + +Resolve the spec drift flagged in #244: `specs/behaviors/attachments.md` documented a `repo.writeBlobFromFile('/path')` helper and hologit-era `BlobObject` handles that the 2.x surface never implemented. + +## Implements + +- `specs/behaviors/attachments.md`, `specs/api/sheet.md` (naming corrections; no behavior change) + +## Approach + +The primary drift (`writeBlobFromFile` example + "hologit BlobObject or whatever the new substrate provides" prose) was already superseded by #242's bytes-accepting `setAttachment` spec rewrite. This plan sweeps the residue: remaining `BlobObject` references corrected to the shipped types — `BlobHandle` for diff `srcBlob`/`dstBlob`, `AttachmentBlobHandle` for attachment getters — across `specs/api/sheet.md`, `specs/behaviors/attachments.md`, and `docs/api.md`. A file-path convenience helper was deliberately **not** added: with `setAttachment` accepting raw bytes (#234), `readFile` → one call covers the case; reopen if a consumer needs streaming file writes. + +## Validation + +- [x] `grep -rn "writeBlobFromFile\|BlobObject" specs/ docs/` returns only migration-guide historical references +- [x] Names verified against the shipped surface (`packages/gitsheets/src/sheet.ts`: `BlobHandle`, `AttachmentBlobHandle`) + +## Risks / unknowns + +None — docs-only. + +## Notes + +Decision recorded: spec amended rather than implementing `writeBlobFromFile`; rationale above. + +## Follow-ups + +- None. diff --git a/specs/api/sheet.md b/specs/api/sheet.md index 3cf3311..cabdbe8 100644 --- a/specs/api/sheet.md +++ b/specs/api/sheet.md @@ -248,12 +248,12 @@ for await (const change of sheet.diffFrom('HEAD~1', { records: true, patches: tr // change.srcHash / change.dstHash // blob hashes (null on add/delete) // change.src / change.dst // parsed records (records: true) // change.patch // RFC 6902 JSON Patch ops (patches: true) - // change.srcBlob / change.dstBlob // hologit BlobObject handles (blobs: true) + // change.srcBlob / change.dstBlob // gitsheets BlobHandle handles (blobs: true) } ``` - `srcCommitHash` accepts a commit hash, a tree hash, or a ref name (`'HEAD~1'`, `'main'`). Defaults to the empty tree — every current record yields `status: 'added'`. -- `opts.blobs?: boolean` — attach `srcBlob` / `dstBlob` (hologit `BlobObject`) handles. +- `opts.blobs?: boolean` — attach `srcBlob` / `dstBlob` (gitsheets `BlobHandle`) handles. - `opts.records?: boolean` — parse src/dst TOML into records. - `opts.patches?: boolean` — produce an RFC 6902 JSON Patch (`Operation[]`) from src to dst. Add and delete entries get a single-op patch (`add` / `remove` on the root); modify entries get the full op sequence. diff --git a/specs/behaviors/attachments.md b/specs/behaviors/attachments.md index 8b6e393..2a77999 100644 --- a/specs/behaviors/attachments.md +++ b/specs/behaviors/attachments.md @@ -69,7 +69,7 @@ Returns a blob map keyed by attachment name. Used for browsing. ```typescript const attachments = await sheet.getAttachments(record); -// → { 'avatar.jpg': BlobObject, 'avatar-128.jpg': BlobObject } +// → { 'avatar.jpg': AttachmentBlobHandle, 'avatar-128.jpg': AttachmentBlobHandle } ``` Returns `null` if the record has no attachment directory.