mirror of
https://github.com/Twigpine/node.git
synced 2026-11-09 18:08:02 -10:00
a2262d92b2
* feat(node): implement reconciliation sweep as durability backstop (#218) Reconciliation sweep: periodic background worker that re-derives pin/seal sets and fills gaps so a dropped replication job never means data loss. - visibility_pack.rs: all_object_paths (two-phase ls-tree + cat-file --batch-all-objects), allowed_blob_tree_sets_bounded, ObjectPath/BlobTreeSets type aliases - db/mod.rs: atomic policy transactions, get/set_node_state, has_ipfs_cid, filter_ipfs_pinned_oids, filter_pinata_pinned_oids, repo_policy_epoch, migration v27 (node_state table + policy_epoch column) - ipfs_pin.rs: PolicyFence struct, dispatch fence re-check before POST - pinata.rs: dispatch fence re-check before POST - encrypted_pin.rs: dispatch fence re-check - reconciliation.rs: full sweep worker with keyset cursor, PolicyFence - api/ipfs.rs: effective_cid fallback in list_pins - main.rs: spawn reconciliation sweep worker - .cargo/audit.toml: RUSTSEC-2026-0258 h2 ignore (pending #368) * fix: add missing parameters to pinning functions and adjust timeout settings * fix(tests): update CID assertions to handle Option type and adjust parameters in pinning functions * fix(node): close review findings on reconciliation sweep (#218) Reviewer-1 P1 / Reviewer-2 P1: v27/v28/v29 were referenced by tests but never added to the MIGRATIONS array, so every existing deployed node would fail to boot (node_state, repos.policy_epoch, pinned_cids.cid NOT NULL → NULL all unreadable). Forward-only migrations: v27 pinata_only_clear_legacy_equal_cid: ALTER cid DROP NOT NULL, UPDATE cid = NULL WHERE cid = pinata_cid v28 node_state_key_value: new key/value table for the sweep cursor v29 repos_policy_epoch: BIGINT NOT NULL DEFAULT 0 Reviewer-2 P1: visibility_pack.rs:542 phase 1 used ls-tree -rz, which under -r recurses into blobs and never emits tree entries. Trees then arrived only from the phase-2 catch-all with empty path and the fail-closed filter denied every one — the sweep could repair a non-flat repo's git graph. Switched phase 1 to ls-tree -r -t -z and add a per-commit rev-parse <commit>^{tree} that registers the root tree at "/". 48 visibility_pack unit tests pass. Reviewer-1 P2 / Reviewer-2 P2: list_pinned_cids called is_raw_cidv1 on the cid column before decoding it as Option<String>, so SQL NULL failed the decode and the new Pinata-only rows never reached the handler. Decode cid as Option first, drop only the both-NULL case. The is_raw_cidv1 filter was removed entirely: the new contract lists every row that has something to advertise, and the handler at api/ipfs.rs:list_pins is the seam that decides what to do with a legacy-shape row (the resolver 404s on a mismatched key, per #173 U4). Updated list_pinned_cids_omits_unrepaired_legacy_row to match. Reviewer-1 P2: two new sqlx::test cases pin the gate order: sweep_skips_quarantined_repos_before_scan (SQL filter, plus a defense-in-depth per-row re-check) and sweep_skips_private_repos_before_scan (per-repo listable_at_root). Both use expect(0) on the mock IPFS so any future reorder that moves the scan ahead of the gate fails the build. Also: record_pinned_cid ON CONFLICT now rewrites cid (R1-P2: a stale wrong CID from a previous push is repaired by a subsequent one). record_pinata_cid ON CONFLICT clears cid to NULL when the existing row has the legacy cid = pinata_cid shape; on INSERT, stores cid = NULL when raw_cid == pinata_cid (the Pinata-only signal). * feat: implement per-backend continuation offset for missing OIDs - Enhance `missing_oids` function to accept a `start_after` parameter, allowing for rotation of the missing set based on the last attempted OID. - Update `run_pass` to load and save the continuation offset for both IPFS and Pinata backends, ensuring that previously attempted OIDs are retried last in subsequent passes. - Introduce tests to validate the behavior of the new offset mechanism, ensuring deterministic ordering and proper handling of gaps in the missing set. - Ensure that the offset is cleared when no missing OIDs are found, allowing for fresh starts in subsequent passes. * fix(node): trace provenance/structural invariants through every consumer The v30 local_ipfs_provenance column was set by the writers but only consulted by has_ipfs_cid and filter_ipfs_pinned_oids. The pin loop's "is this already done" check still used row existence (is_pinned), so a Pinata-only row hit the early-return branch without ever reaching the local writer, and the flag stayed FALSE forever. Three other seams carried the same shape-vs-provenance confusion. P1 (ipfs_pin.rs:1843): pin loop's skip keys on has_ipfs_cid (writer-owned local_ipfs_provenance = TRUE), not on row existence. A Pinata-only row now falls through to the local writer path, which calls record_pinned_cid_with_source and flips the flag on the conflict branch. The Pinata pin loop's skip remains keyed on has_pinata_cid (a separate seam). P1 (db/mod.rs v30 backfill): tighten the WHERE clause to cid IS NOT NULL AND pinata_cid IS NULL. The previous permissive (cid <> pinata_cid) clause marked pre-v30 Pinata-first uploads (where record_pinata_cid deliberately stored cid = raw_cid without a local Kubo push) as local IPFS pins, so the sweep filtered them as already-durable and an operator enabling IPFS later never got a local recovery copy. Ambiguous dual-shape rows stay at FALSE under the strict rule and the sweep re-derives on re-pin (cheap, idempotent). P1 (db/mod.rs record_pinned_cid_with_source ON CONFLICT): restore cid = COALESCE(pinned_cids.cid, EXCLUDED.cid). A Pinata-only row with cid = NULL (the raw_cid == pinata_cid shape) would otherwise be marked complete with local_ipfs_provenance = TRUE and a NULL resolver key. P1b (visibility_pack.rs): remove the synthetic "/" path on root trees and add a structural entry-level check (structurally_safe_root_tree / _for_caller) in both the sweep gate (allowed_blob_tree_sets_bounded) and the per-request tree gate (allowed_tree_set_for_caller_bounded). A tree is admitted only if every direct entry in its serialized bytes is safe at /{filename} and (for tree entries) the child tree is itself in the allow set. The previous path-based "/" check let the root tree of a public repo slip through even when its bytes name a denied subtree entry. P2 (api/ipfs.rs list_pins): add local_ipfs_provenance to PinnedCidRecord, include in list_pinned_cids SELECT, and surface as local_pinned / pinata_pinned in the JSON response. A Pinata-only row previously returned the raw resolver key as local_cid, making it indistinguishable from a real dual-backed row. The new fields let a gl consumer distinguish local-only, remote-only, and dual rows without re-inferring semantics from nullability. local_cid is preserved for backward compatibility. Tests: - migration_v30_backfills_local_ipfs_provenance_heuristically: flipped both-distinct expectation to FALSE under strict rule. - migration_v12_makes_cid_nullable_and_preserves_classification and migration_v27_clears_legacy_equal_cid: both-distinct / distinct row expectations flipped to FALSE. - sweep_never_pins_root_tree_naming_withheld_subtree (new): assert root tree + secret subtree tree absent from pinned_cids. - allowed_tree_set_gates_withheld_subtree_tree: root tree assertion inverted to EXCLUDED for anon. - list_pins_reports_writer_owned_provenance_for_all_shapes (new): assert 4 row shapes produce correct local_pinned / pinata_pinned JSON. Scope discipline preserved: PolicyFence capture order, _pin_permit reuse for the seal phase, fresh per-arm rederive_budget, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration all untouched. v31 schema unchanged. * fix(node): structural tree gate at every depth + non-commit ref tolerance The prior round installed a `local_ipfs_provenance` column and applied a structural tree gate only to root trees. The fresh review pass found two structural gaps the writer contract didn't propagate to: P1 (visibility_pack.rs:1519): the path-based tree admission admitted a tree whenever its path was allowed, regardless of whether its serialized bytes named a denied subtree. Concrete failure: a public repo with `/public/secret/file` and a `/public/secret/**` deny admitted the `/public` tree (its path is `/public`, allowed) but `/public` still contained a `secret/` entry pointing at the denied subtree — anyone who obtained `/public`'s CID could `cat-file -p` it and read the subtree's name and child OID. The prior round's post-pass structural check ran on root trees only. The fix: replace the prior round's `structurally_safe_root_tree` / `_for_caller` / `root_tree_oids` helpers with one recursive `tree_structurally_safe` that walks every tree's entries at every depth, recursing on tree entries. Bundled the rule/path args into a `TreeCheckCtx` struct to fix the clippy `too_many_arguments` warnings the prior round introduced. A tree is admitted iff every direct entry is safe at its path AND (for tree entries) the child tree is itself structurally safe there. A denied-subtree tree fails at its level; the parent fails because the denied-path child entry fails; the propagation continues up to the root. The `admitted` set memoizes trees proven safe at some path so the recursion short-circuits on cycles and on trees reachable at multiple allowed paths. P1 (visibility_pack.rs:533): `all_object_paths` called `assert_all_refs_are_commits`, which bailed on any ref that didn't peel to a commit (tag-of-tree, tag-of-blob). The sweep (`run_pass`) converted that error into a full-scan failure that skipped the whole repo. A repo with a pushable tag-of-tree never got its missing public pins repaired by the new hourly worker, even though its commit-reachable graph was otherwise safe to classify (`ipfs_cid_tree_served_despite_non_commit_ref` is the in-repo example of that supported shape). The fix: remove the `assert_all_refs_are_commits` call from `all_object_paths` (and the analogous one in `blob_paths`, used by the encrypted recovery path — the same reasoning applies). `git rev-list --all` already silently skips non-commit refs, so the commit-reachable object set is what the sweep needs. The unused `assert_all_refs_are_commits` helper is removed. Unclassifiable ref targets still fail closed at a different layer: the cat-file catch-all enumerates them with no path, and the path-based allow filter drops empty-path entries. Test inversions and new regressions: - `migration_v12_makes_cid_nullable_and_preserves_classification` and `migration_v27_clears_legacy_equal_cid` — both-distinct / distinct row expectations flipped to FALSE under the v30 strict backfill (prior round's tightening is re-affirmed here; the v30 backfill is `cid IS NOT NULL AND pinata_cid IS NULL`, the only unambiguous local-IPFS shape). - `ipfs_cid_gate_withholds_blob_from_unauthorized` — root tree assertion inverted to NOT_FOUND (the recursive structural gate denies root trees whose entries name a denied subtree). - `ipfs_cid_legacy_provider_cid_repaired_on_repush` — legacy-shape row insert now sets `local_ipfs_provenance = TRUE` to reflect that the bytes ARE on local IPFS (the legacy CID is just the wrong key). - `ipfs_cid_tree_served_despite_non_commit_ref` — root tree assertion inverted (same reason as the gate test). - `fails_closed_on_annotated_tag_of_a_blob` and `fails_closed_when_a_ref_cannot_be_traversed` — renamed to `skips_*`, assertions inverted from `Err` to `Ok` (a ref pointing at a non-commit object is no longer a walk error; the blob falls out of the gap set because `git rev-list --all` skips it). - `ipfs_cid_walk_error_fails_closed` — removed (relied on the now-removed `assert_all_refs_are_commits` to force an error; the fail-closed arm is still covered by `ipfs_cid_commit_tag_walk_error_fails_closed`, which uses a different trigger — a nonexistent ref object makes `rev-list --all` fail). - `sweep_does_not_publish_public_ancestor_tree_naming_withheld_subtree` (new) — recursive depth test: a `/public/secret/**` repo asserts root tree, `/public` tree, and `/public/secret` subtree are all absent from `pinned_cids` while `/public/visible.txt` is IPFS-pinned. - `sweep_repairs_commit_reachable_object_in_repo_with_tag_of_tree` (new) — tag-of-tree repo with a separate `mktree`'d tree (not commit-reachable); asserts the commit-reachable blob is IPFS-pinned and the unclassifiable tag-of-tree is not. Scope discipline preserved: PolicyFence capture order, `_pin_permit` reuse for the seal phase, fresh per-arm `rederive_budget`, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration, v30 strict backfill, v31 schema, per-(repo, backend) reconciliation offset, and the `local_ipfs_provenance` writer/reader contract are all untouched. * test(node): ignore pre-existing RED replication-tail test on CI `api::repos::tests::receive_pack_tail_survives_a_disconnect_during_release` documents an open bug in `api/repos.rs` (the replication tail must be spawned above `release` and gated on `receive_result.is_ok()`; today it's spawned below, so a disconnect during `release` drops the pins, recovery copy, and announce). The test's `assert!` message starts with "RED:" — this is a TDD-style "keep the design intent visible in the code" test, not a working-behavior assertion. The test was added 2026-07-29 by `beardthelion` (PR #215 lineage), well before the #218 sweep work, and is unrelated to this PR. CI is failing on it under load (passes in isolation) because the open bug hasn't been fixed yet. `#[ignore]` until the open bug is fixed. The test's design intent and the fix description stay in the doc comment and the assert message; only the CI failure is suppressed. Run the test explicitly with `cargo test -- --ignored` to verify the bug is fixed when the tail-spawn moves above `release`. * fix(node): align encrypted manifest anchor log with one-shot contract The "will retry next pass" log at reconciliation.rs:1034 was false: plan_seal returns SkipUnchanged on every subsequent pass once the recipients tag matches, so sealed stays empty and the anchor block never runs again. A transient Irys outage at the moment of a fresh seal therefore LOSES that anchor permanently — the next pass has no delta to anchor and no retry fires. This is intentionally best-effort: re-anchoring an unchanged manifest would burn Irys writes for no recovery benefit. Factored the warn message into ENCRYPTED_MANIFEST_ANCHOR_FAILED_MSG and added a unit test that pins the constant; the test goes RED if "will retry" reappears in the log. * fix(node): address eight reviewer findings on #218 reconciliation sweep (round 3) * fix(node): close audit.toml array and drop inline comment * fix(node): repair the stale for-each-ref fake-git fixture (round 8 P1) `upload_pack_shares_one_deadline_across_walk_and_filtered_serve` — the #174 shared-deadline guard for the FILTERED serve arm — was red on both `test (stable)` and `test (beta)`. The fake git answered `for-each-ref` with the single token `refs/heads/main`, a ref NAME, where `blob_paths` phase 2 asks for `--format=%(objectname) %(objecttype)`. The phase-2 parse fails closed on a line it cannot read, so the walk returned an error and the request surfaced as a generic 500 instead of being reaped into a 504. That silently repurposed the test from "the filtered serve shares one deadline" into "the walk errors" — the guard proved nothing while it was red. The fixture is what was stale, not the production path: the arm now emits the column shape the walk parses. A commit tip has no peeled referent, so two columns is the whole line. The 1.2s walk cost stays on `ls-tree` and the serve's on `pack-objects`, so the two phases remain independently attributable. * fix(node): peel annotated tags in the blob_paths ref walk (round 8 P1) `%(objecttype)` reports the type of the ref's OWN object, which for an annotated tag is `tag`. The phase-2 `if kind == "blob" || kind == "tree"` arms therefore never saw the referent, and a blob reachable ONLY through an annotated tag escaped the withheld set — while `rev_list_keep`'s `git rev-list --objects --all` does peel tags and served it. Phase 2 closed the leak for a direct ref-to-blob and for that shape only. The walk now asks for the peeled atoms `%(*objectname) %(*objecttype)` and inserts the referent with an empty path, alongside the direct blob/tree tips it already handled. Line parsing moves to a field-count match: two columns for a non-tag tip (the peeled atoms expand to empty), four for a tag tip, anything else still fails closed. Measured against git 2.50, the `*` atoms peel the WHOLE chain — a tag-of-a-tag-of-a-tag-of-a-blob reports the blob — so a `tag` peeled type does not occur on stock git. That arm is kept anyway, because `push_delta.rs`'s ref-type guard documents `%(*objecttype)` as a one-level peel: under such a git it finishes the chain with `rev-parse <tip>^{}` plus a `cat-file -t` probe rather than bailing and 500ing every clone of a repo that merely carries a nested tag. Both children are bounded by the walk's shared deadline and are unreachable on the common one-line-per-ref path. * fix(node): one empty-path decision for both blob_paths consumers (round 8 P1) `blob_paths` has two consumers and only one of them carried the empty-path policy. `withheld_from_pairs` (the deny side, what the smart-http serve filter excludes) withheld a phase-2 empty-path entry from everyone but the owner; `allowed_blob_set_for_caller_bounded` (the allow side, what `GET /ipfs/{cid}` hands over) called `visibility_check(..., "")` directly. On a public repo no glob matches the empty path, so that returns `Allow`: the serve filter withheld the OID while the IPFS gate served the bytes — the leak phase 2 exists to close, reopened one layer over. The policy moves into `pair_decision`, which both consumers now call, so the two gates cannot disagree and a future change to the empty-path rule moves both at once. `withheld_blob_recipients_bounded` routes through it too: an unclassifiable blob grants a recovery copy to the owner only, never to a rule's named reader whose grant was written against a path this object does not have. No behavior change for non-empty paths — `pair_decision` delegates to `visibility_check` unchanged. * test(node): bind the phase-2 ref tests to a commit-unreachable blob (round 8 P2) `skips_a_ref_pointing_at_a_blob` and `skips_an_annotated_tag_of_a_blob` were vacuous. Both hung their ref on `fixture()`'s `secret` blob, which is COMMITTED at `secret/b.txt`, so phase 1's per-commit `ls-tree` already produced `(secret, "/secret/b.txt")`, the `/secret/**` rule already denied it, and `withheld.contains(&secret)` passed with phase 2 deleted outright. Both now use `orphan_blob`, a blob written straight to the object store that no commit's tree names, so the ONLY route into the withheld set is the `for-each-ref` phase. That is also the real leak shape: `rev_list_keep`'s `git rev-list --objects --all` does follow a ref to such a blob, so an under-withheld one ships in the clone pack. Verified by mutation: with `if false` injected at the phase-2 filter, every test below goes RED (they were green under that mutation before). New coverage: - `ref_only_blob_is_denied_on_the_allow_side_too` — the allow-side denial for the shared `pair_decision`; it also asserts deny and allow agree on one OID, and that the owner IS still admitted, so it cannot pass by denying everything. Reverting the allow side to `visibility_check` turns it red on exactly that assertion. - `skips_a_nested_annotated_tag_of_a_blob` — real-git tag-of-a-tag. - `peels_a_tag_whose_peeled_target_is_still_a_tag` — drives the one-level-peel fallback through the fake-git seam, so that arm is tested rather than dead. * fix(node): advance the reconciliation continuation only for dispatched work (round 8 P2) `ipfs_last` / `pinata_last` were captured from the missing set above the pin permit, both `PolicyFence` captures and both pin loops, then written under nothing but `if ipfs_enabled` / `if pinata_enabled`. Every stage in between can legitimately produce nothing — a fence capture that fails, a quarantine/visibility recheck that says skip, a pin-boundary re-derivation that errors — and each is transient. Advancing past OIDs that were never attempted is not a harmless retry delay: `missing_oids` rotates strictly past the stored offset, so the whole unattempted prefix lands BEHIND the entire backlog on the next pass. For an at-cap repo — the only kind the continuation exists for — the backlog never drains inside one cap window, so those objects are starved indefinitely. That is a durability hole in the durability backstop. The offset now moves only for work actually handed to a backend, tracked at the dispatch boundary as `to_pin.last()` (not the missing set's last: an OID the pin-boundary re-derivation dropped was never offered). It is recorded before the call, so a pin phase that times out mid-batch still counts as dispatched. The write becomes three outcomes rather than two, via one `next_offset_write` helper so the backends cannot drift apart: * work dispatched -> advance to the last dispatched OID * nothing missing, query OK -> None, marking the row done * nothing dispatched, or the missing-set query failed -> write NOTHING, leaving the resume point a capped pass already paid for `*_scan_ok` is what separates the last two: an empty missing set means "everything is pinned" or "the filter query failed and we know nothing", and only the first should mark the row done. Test: `sweep_leaves_offset_untouched_when_nothing_was_dispatched`, using a `#[cfg(test)]` seam on the pin-boundary re-derivation — that stage cannot be starved from the outside, because the mid-scan re-filter runs first on the same rules and budget and makes the sweep `continue` well before the offset write. It asserts the stored offset comes back byte-identical after a declining pass, then that the same repo does advance it once dispatch happens, so the assertion cannot pass on a dead write site. Restoring the old semantics turns it red. * fix(gl): render the Pinata provider CID in `gl ipfs list` (round 8 P2) The server-side wire change landed without its client half. `cmd_list` read `pin["cid"].as_str().unwrap_or("?")` as the entry heading and nothing in the CLI read `pinata_cid` at all, so a Pinata-only row printed as a bare `?` — the listing showed nothing usable for an object that is in fact durably stored. `cid` is legitimately null for such a row: it is the key `GET /ipfs/{cid}` resolves against, and the node stopped aliasing a Pinata provider CID into it (round 3) precisely because that CID 404s there. So the fix is on this side. Rendering moves into `render_pin`, which leads with the node-resolvable CID when there is one and otherwise the provider CID, labelled so nobody feeds it back to this node's resolver. A dual row keeps both visible. A `backends:` line names where the bytes are, read from the writer-owned `local_pinned` / `pinata_pinned` booleans rather than re-inferred from CID shape — which is what the node's own comment warns against. Older nodes send neither flag; those `cid`-only rows fall back to CID presence and render as they always did (`render_pin_handles_a_legacy_cid_only_row`). * fix(node): implement third fence check for IPFS pinning and enhance visibility rules handling * fix(node): enhance documentation and add tests for ProgressState and helper functions * fix(node): address round-9 reviewer findings on #218 reconciliation sweep * fix(node): correct consumer-4 recipient test for call-invariant set * fix(git): fail closed on non-UTF-8 tree listing in walk_tree_oids_inner (#218 round 10 P1) walk_tree_oids_inner previously returned Ok(()) on a non-UTF-8 ls-tree -z listing, inserting only the tree OID and skipping every child blob/tree OID. A direct tree ref (or an annotated tag peeling to a tree) is valid Git input; git rev-list --objects --all still enumerates the tree and every descendant, so the keep-side removed the tree from the served set but passed the child blob OIDs to pack-objects, exposing their bytes to an anonymous clone. Phase 1 already bails on the same input at the blob_paths walk; this fix puts the tree-walk on the same fail-closed outcome. The new fails_closed_on_non_utf8_tree_tip test pins the invariant for both a direct refs/tags/direct-tree ref and an annotated tag-of-tree (the two non-commit shapes round 9 added tolerance for), confirming the walker bails rather than returning Ok with a partial withheld set. * fix(recon): run encrypted recovery even when public list is empty (#218 round 10 P1) A path-scoped repo whose only reachable object is a direct blob or direct tree ref yields an empty public list (the anonymous public classifier removes the only object from the served set) while withheld_blob_recipients_bounded still assigns that object to the owner recovery set. The early `continue` on object_list.is_empty() (and the second one after the mid-pass refilter) suppressed encrypted recovery too, and a lost or failed encrypted copy was never repaired. Track empty-public-work as a flag and gate the public phase on it. Encrypted phase 2 runs regardless. The backend enable flags and `_pin_permit` move out of the gated block so phase 2 can read them when the public phase did not run. * fix(db): rename migration v12 test to v32 (#218 round 10 P3) The test name and its docstring/comments still say "v12", but the cid-nullable migration is at v32 (line 1163). The test itself is correct — it drives the real path — so this is naming drift rather than dead coverage. A future reader looking for the migration would otherwise chase a v12 that no longer exists. * fix(git): index OID membership before quadratic classifier scan (#218 round 10 P2) all_object_paths was doing O(O×P) work in two places: the catch-all cat-file branch checked `blob_set.iter().any(|o| o == oid)` and `tree_set.iter().any(...)` for every reachable object, and the phase 1 loop also did a per-commit unused `git rev-parse <commit>^{tree}` probe whose result was discarded (the root-tree OID is recomputed by `root_tree_oids`). On the 50k-object repos the sweep exists for, the quadratic scan and the redundant per-commit child process can consume each authorization deadline before any object reaches a backend, producing a permanent hourly skip. Maintain a separate `blob_oids` / `tree_oids` HashSet alongside the path-pair sets so the membership check is O(1). Drop the unused per-commit rev-parse probe. * fix(git): bound tree walk by ls-tree invocation count, not just depth (#218 round 10 P2) walk_tree_oids_inner stopped at MAX_TREE_WALK_DEPTH and the shared deadline. A wide shallow tree reachable from N ref tips or from multiple parents still spawned one ls-tree child process per subtree well inside the depth cap, and the wall-clock bound could not stop that — expiry was the only stop. The ceil was wall-clock only. Add a structural invocation cap (MAX_TREE_WALK_INVOCATIONS) the walker fails closed at, and a memo of walked tree OIDs so the same tree reachable from multiple ref tips is walked once. Round 10 P2. * fix(pinata): suppress push when DB record is not durable (#218 round 10 P2) pin_new_objects pushed (sha, cid) on provider POST success regardless of whether record_pinata_cid persisted the row. A closed/failed DB write or an explicit-transaction source record timeout meant the next sweep queried the DB, found no row, and re-offered the same gap — with reconciliation having counted a fill that never happened. Track a `db_record_durable` flag. On hard failure of either record_pinata_cid (non-timeout) or record_pin_source (any failure, since it's a multi-statement transaction), set the flag false and skip the `pinned.push`. The `BoundedDbError::Elapsed` autocommit case stays as the previous comment's "may have committed" reasoning: the push fires so the reconcile still sees the fill. Round 10 P2. * fix(node): serialize policy narrow with irreversible upload via in-process mutex (#218 round 10 P1) The PolicyFence's epoch read was a one-way read: a visibility narrow (rule insert, quarantine set) could commit between the fence's epoch read and the upload's HTTP POST, and the upload's DB-record INSERT's epoch check would see the new epoch and reject — but the provider already accepted the object. Encrypted envelopes had no fence at all. Add a per-repo in-process mutex that PolicyFence acquires at capture and releases at drop. The narrow paths (set_repo_quarantine, set_visibility_rule, remove_visibility_rule) acquire the same lock for the duration of their DB transaction. A narrow therefore blocks until the in-flight batch finishes, and an in-flight batch waits for the narrow to commit before it even reads the epoch. Multi-process / multi-node is a known gap: a Postgres advisory lock per repo would extend the same guarantee across processes. Deferred to a follow-up. * fix(pinata): update post-upload stalled record test for round 10 P2 contract The previous test asserted that a successful upload with a timed-out post-upload source record still returned the (sha, cid) pair. That assertion pinned the buggy behavior the reviewer called out: the reconcile would count a fill that was not durable, then re-offer the same gap. The test name and the `record_pin_source` arm of `pin_new_objects` are unchanged; the assertion now reflects the new contract: when the post-upload source record times out (its `mark_pin_sources_incomplete` already sets the source set to incomplete, so the next pass re-offers), the pair is suppressed so the reconcile sees the gap as not-filled. Round 10 P2 follow-up. * fix: clippy allow + rustfmt on round 10 commits clippy::too_many_arguments on walk_tree_oids_inner (8 args after the round 10 memo + invocation counter threading). rustfmt re-ran on the if/else blocks added in the has_public_work refactor (round 10 P1 #42). No behavior change. * fix(node): address round-11 review findings on #218 reconciliation sweep P1: remove the whole-batch policy mutex. PolicyFence no longer holds a per-repo lock from capture through drop and the narrow paths (set_visibility_rule, remove_visibility_rule, set_repo_quarantine) no longer acquire it, so a visibility narrow commits immediately and the batch aborts on its next is_current check instead of the narrow blocking behind the sweep. Accepted residual is a single in-flight object; the fenced DB record still refuses to land a raced row as durable. encrypt_and_pin_stops_sealing_when_reader_removed_mid_batch is green again. PolicyMutexes is deleted, resolving the key-leak P2 by removal. P2: cover the round-10 has_public_work branch with sweep_seals_withheld_blob_when_public_list_is_empty: a direct-blob-ref repo yields an empty public list with a non-empty owner recovery set; asserts no public gaps/fills, no cleartext pin, exactly one POST (the seal envelope), and a recorded encrypted copy. P3: peel-case isolation in the non-UTF-8 tree-tip test (delete the direct-tree ref before the second call so the peel arm must run); v12->v32 rename drift in the migration test (boundary, comments, and the stale (2)->true doc line); accurate comments for the pre-refilter flag and the post-refilter fall-through. * fix(node): address merge-readiness findings on #218 reconciliation sweep Reworks the sweep around explicit state-machine contracts instead of adding conditionals: backend results carry last_attempted plus confirmed durability separately (PinBatchOutcome), provider permits scope to provider effects only, discovery pages by commit window with a persisted cursor, legacy provenance migrates as unknown, and the pin list advertises only validated raw local keys. - Empty-path owner recovery without path rules: phase 2 drops the has_path_scoped_rule gate and runs on the actual recipient result, with a dedicated recovery recheck (no anonymous-listability requirement); matrix test over blob/tree/tag refs x zero/root rules, including recreate-after-delete. - Pinata Elapsed is non-durable unless verify_pinata_record proves the exact row and fence epoch; loop test with row-locked repos. - list_pins gates cid/local_cid on is_raw_cidv1 plus confirmed provenance; response-shape matrix uses real CIDs with a legacy row and GET /ipfs follow-through; docs match the nullable wire contract. - Permits acquired per provider effect and dropped after; no nesting at pool size 1 (test renamed to the new shape). - Scan discovery pages SCAN_COMMIT_WINDOW commits oldest-first with a node_state cursor (advance on drained/empty/attempted, delete on cover); refilters and phase-2 recipients run on the same window; walk-layer paging/invocation/union tests plus a 1050-commit cursor-to-completion sweep test. - v35 migrates legacy rows as unknown (no TRUE backfill); v35 test asserts unknown + preserved Pinata history; sweep re-derivation test re-pins instead of filtering. - Drained completions DELETE continuation rows (no tombstones); FK cascade on repo delete; row-count regression test. - Audit: lru waiver gets an explicit retirement owner plus a weekly drift guard; revocation model recorded as a product decision on PolicyFence (prompt revocation, single-in-flight residual). * fix(tests): configure git identity in bare clone for tag fixture The windowed-union test creates its annotated tag in the bare clone, which carries no config. It passed locally only via the developer's global git identity; CI has none, so 'git tag -a' failed there. Verified with an empty GIT_CONFIG_GLOBAL/SYSTEM. * fix(tests): ignore pre-existing hung-git pid race on CI read_object_bounded_returns_by_deadline_with_a_hung_git reads the fake's pid file with no readiness handshake; a slow scheduler yields 'No such file or directory' (stable-only red, identical beta green). Unrelated to #218 (store.rs untouched); re-enable with a readiness signal from the fake. * fix(node): address round-12 review findings on #218 reconciliation sweep - P1 roots: admit structurally safe root trees at "/" into the allow set and candidates, so safe snapshots fully replicate; roots naming a withheld subtree stay excluded via the recursive structural check. - P1 per-pass WalkBudget on sweep enumeration (entries / output bytes / git invocations) with fail-closed budget-exhaustion; serve paths keep the uncapped budget. - P2 recovery lane: independent recovery cursor pages unlistable history across restarts instead of sharing the scan window. - P2 gap-hold: failed gap queries hold discovery advance but not the healthy backend, via test-only failure seams. - P2 nested tags: collect full tag chains so nested tag objects pin without an inner ref. - P2 RepoStore: thread the Tigris-backed store into the sweep with an acquire timeout and shutdown select; restore cache-miss and skip missing. - P2 Pinata source: keep confirmed pairs when the primary already covers the repo; suppress push for stalled post-upload records without coverage. - Update large-history sweep expectation to blob+root+commit per round. * fix(node): address consolidated review findings on #218 reconciliation sweep P1: wire WalkBudgetExceeded through the sweep cursor logic. Both the public scan and the recovery walk now dispatch the typed budget-exhaustion signal at the sweep boundary: a budget-hit window advances past content the sweep refuses to classify partially (scan cursor save/delete via the existing persist site; recovery cursor likewise), while git/policy/panic/timeout failures keep the cursor for retry. visibility_pack documents the obligation; the match arms are the handler, with a one-line pattern note for future typed walk errors. P2: run encrypted recovery when the public scan fails. Scan walk/panic/timeout arms now fall through with an empty public list instead of continuing the whole repo iteration, so phase 2 runs on the recovery lane's own cursor like the unlistable-repo path. The scan cursor stays untouched for retry. Refilter/recheck-public failures remain fail-closed whole-iteration continues by design. Includes a repo-scoped test seam (process-global set, safe across spawn_blocking and parallel tests via fresh UUIDs). P2: bump policy_epoch only when remove_visibility_rule deletes a row, mirroring set_repo_quarantine's rows-affected guard, so a typo'd or double-deleted glob cannot abort fenced batches. Also adds the repo-loop outcome matrix (every early exit × scan cursor / recovery cursor / phase 1 / phase 2) and deny tests: budget-hit advances both lane cursors with zero pins/seals (real 250k-entry trip, early-trip fixture runs in ~30s), scan failure still seals the recovery copy, delete-missing-rule leaves the epoch unchanged. Each deny was verified by temporarily restoring the old branch. * fix(node): address jatmn review findings on reconciliation sweep re-home * fix(node): reload persisted sweep cursor between passes * fix(node,gl): pinata offset warn wording and boundary-safe pin timestamp --------- Co-authored-by: Gravirei <[email protected]> Co-authored-by: Kevin Codex <[email protected]>