22 Commits

Include renames

Author	SHA1	Message	Date
3 people	f68d0dd3ff	fix(node): durable post-receive outbox for receive-pack (#26 split 1/4) (#558) ... * fix(node): durable post-receive outbox at the DB layer (#26 split 1/4) Reviewer 2 closed PR #224 on 2026-08-28 with a directive: split the work into four narrow PRs. This is Split PR 1 (durable post-receive lifecycle) at the DB layer; the handler refactor in crates/gitlawb-node/src/api/repos.rs:2007 (git_receive_pack) lands in the next slice so the test can drive the failure injection end-to-end. The pre-outbox crash window the reviewer flagged: receive_pack can apply a ref to disk and return Ok, and a process exit, a dropped future, or a DB failure before the bookkeeping at crates/gitlawb-node/src/api/repos.rs:2361 (push event + cert + webhook) loses the recovery record. Startup drain enumerates only sources written from that bookkeeping, so it cannot reconstruct the missing work. The partial fallback that re-derives from a row present in the bookkeeping substitutes did:key:recovered and an empty attestation, which is not equivalent to the original authenticated push. This commit adds the durable boundary the handler will lean on. NEW TABLE pending_ref_transitions (migration v27): - Written by the handler BEFORE smart_http::receive_pack, in state 'prepared', carrying the verified pusher DID, the raw RFC 9421 signature header, signature-input, and content-digest that authorized the push, the request id, and the parsed ref update. - The handler transitions the row to 'applied' on receive_pack Ok or 'cancelled' on Err. The drain reads only 'applied'. - A failed or cancelled receive-pack therefore leaves the row in 'prepared' or 'cancelled', which the drain never promotes. This is what closes the reviewer's second proof ("a failed or cancelled receive-pack does not turn a prepared intent into completed accounting or anchoring"). NEW TABLE anchor_jobs (migration v27, owned by PR 1, consumed by PR 2): - One row per (repo_id, ref_name, old_sha, new_sha) transition. PR 1 inserts it on 'applied'; PR 2 reads it and updates claimed_at. - ON CONFLICT (id) DO NOTHING makes the insert idempotent on the deterministic id, so a recovery re-pass cannot create a second upload request. This is the handoff boundary; the bundler call itself is PR 2. NEW DB METHODS on Db: - insert_pending_ref_transitions: writes one 'prepared' row per ref update, returns the persisted rows. - mark_pending_ref_transitions_applied / _cancelled: state flip, gated on the FROM state, idempotent. - list_pending_ref_transitions_applied: drain query, oldest first. - delete_pending_ref_transition: called by recovery after the artifacts land; a third pass is a no-op. - record_push_with_id: ON CONFLICT (id) DO NOTHING on the deterministic id. - insert_ref_certificate_idempotent: ON CONFLICT (repo_id, ref_name) DO NOTHING, returns None if a live-path cert already exists. - insert_anchor_job_idempotent: ON CONFLICT (id) DO NOTHING on the deterministic per-transition id. NEW HELPERS in db/mod.rs: - deterministic_id: SHA-256 hex with an ASCII Unit Separator between fields so two distinct tuples never collide on prefix overlap. - push_event_id_for, ref_cert_id_for, anchor_job_id_for: the derived ids above, one helper per artifact so a caller cannot derive a wrong id by mistake. NEW STRUCTS: - PendingRefTransition: the row shape. - AnchorJob: the handoff row shape. - pending_state: const strings ('prepared' / 'applied' / 'cancelled') shared by tests, the producer, and the drain so a typo on one side cannot silently mismatch the other. NEW TESTS in db::pending_ref_transition_tests (8 tests, all green): - insert_then_mark_applied_flips_state_for_every_ref: producer contract. - mark_applied_is_idempotent_on_repeat: re-fire is a no-op. - cancelled_rows_are_not_returned_by_the_drain: reviewer's second proof at the DB layer. - prepared_rows_are_not_returned_by_the_drain: same proof for the pre-flip state (handler crashed before reaching post-Ok). - mark_cancelled_is_idempotent_on_repeat: counterpart. - drain_then_re_derive_is_idempotent: reviewer's first proof at the DB layer. Inserts a row in 'applied' state directly via insert_pending_ref_transition_for_test, drains it, derives the artifact ids twice, exercises record_push_with_id and insert_anchor_job_idempotent directly, asserts exactly one push event row and exactly one anchor job row regardless of how many times the drain runs. - deterministic_id_avoids_prefix_overlap_collisions: the separator regression test. - push_event_id_for_is_stable: derived ids match across calls and differ on each varied input. OTHER: - Make RefUpdate and its fields pub(crate) so the DB methods can iterate the parsed ref updates. No public API change. NOT IN THIS SLICE (the handler refactor, next commit): - The receive-pack handler does not yet call insert_pending_ref_ transitions before the receive_pack call, nor mark_applied / mark_cancelled after. The DB layer is in place for it; the handler will call these methods and the startup drain will be wired in main.rs. - The startup drain in main.rs is not yet called; it will iterate list_pending_ref_transitions_applied, re-derive the artifacts, and delete the row. - The cert/push event issuance in cert.rs and the bookkeeping in api/repos.rs:2361 are not yet changed to use the deterministic ids. The helper functions exist and are tested; the callers follow. Compiles clean, clippy clean under -D warnings, fmt clean. * fix(node): wire durable outbox into the receive-pack handler (#26 split 1/4) This is the handler-level half of Split PR 1. The previous commit added the migration and the DB methods; this one threads them through crates/gitlawb-node/src/api/repos.rs:2007 (git_receive_pack), the cert issuer, and the startup drain. CHANGES IN THE HANDLER ====================== In git_receive_pack, AT THE LAST POSSIBLE MOMENT before the smart_http::receive_pack call, the handler now: 1. Generates a per-handler request_id (UUID). 2. Captures the raw Signature, Signature-Input, and Content-Digest headers from the request. 3. Calls db.insert_pending_ref_transitions(request_id, ...) which writes one row per ref update in state 'prepared'. The receive_pack call runs as before. After it returns: 4. On Ok: db.mark_pending_ref_transitions_applied(request_id) — the row is the ONLY thing that promotes a 'prepared' row to 'applied', and the drain reads only 'applied' rows. A process crash before this call leaves the row in 'prepared', which the drain never promotes. 5. On Err: db.mark_pending_ref_transitions_cancelled(request_id) — a failed receive_pack leaves the row in 'cancelled', which the drain never promotes. This is what closes the reviewer's two proofs: Proof 1 (crash window): if the process dies after mark_pending_ref_transitions_applied but before the bookkeeping writes, the row is in 'applied' and the next startup drain re-derives the push event, the per-ref certificate (carrying the ORIGINAL pusher DID, not a placeholder), and the anchor handoff. The drain uses the persisted authentic pusher DID and signature header, not a recovered placeholder. Proof 2 (failed receive-pack): the row is only ever flipped to 'applied' in the explicit Ok branch above. A 'prepared' or 'cancelled' row is invisible to the drain, so a failed or dropped receive_pack cannot turn a prepared intent into completed accounting or anchoring. BOOKKEEPING IS NOW DETERMINISTIC-ID =================================== The post-Ok bookkeeping at api/repos.rs:2448 now uses: - record_push_with_id with push_event_id_for(request_id, first_ref) — ON CONFLICT (id) DO NOTHING, so a recovery re-pass is a no-op. - issue_ref_certificate_idempotent with ref_cert_id_for(request_id, ref_name) — ON CONFLICT (repo_id, ref_name) DO NOTHING, returns None if a live-path cert already exists. - insert_anchor_job_idempotent with anchor_job_id_for(repo_id, ref_name, old_sha, new_sha) — the per-transition tuple key, so two pushes to the same ref produce one anchor upload per landed state. The legacy entry points (record_push, issue_ref_certificate, insert_ref_certificate) remain for callers that prefer a fresh UUID per cert; they are #[allow(dead_code)] for the PR 3 cert/CLI compat pass to decide whether to keep or remove. STARTUP DRAIN ============= crates/gitlawb-node/src/main.rs calls durable_outbox::drain_pending_ref_transitions(state, 1000) ONCE before serving, after migrations and after the existing peer / quarantine prunes. Non-fatal: a transient drain failure logs and leaves the rows for the next startup. durable_outbox::drain_pending_ref_transitions reads every 'applied' row, calls derive_one (which re-derives the three artifacts using the persisted authentic pusher DID and signature header), then deletes the row. A second drain pass is a no-op for both the artifacts (idempotent inserts) and the row (gone after the first pass). NEW END-TO-END TESTS ==================== crates/gitlawb-node/src/durable_outbox.rs adds three end-to-end tests in drain_tests, complementing the eight DB-layer tests in db::pending_ref_transition_tests: - drain_re_derives_all_three_artifacts_for_an_applied_row: the reviewer's first proof. Inserts a row in 'applied' state (the crash window), drains, asserts exactly one push event row, exactly one cert row carrying the original pusher DID (not a placeholder), and exactly one anchor job row. Asserts the deterministic cert id matches. Asserts a second drain pass is a no-op. - cancelled_row_produces_no_artifacts: the reviewer's second proof for the cancelled state. A row in 'cancelled' (receive_pack returned Err) is invisible to the drain. - prepared_row_produces_no_artifacts: the reviewer's second proof for the prepared state. A row in 'prepared' (handler crashed between insert_prepared and the post-Ok branch) is invisible to the drain. Each test names the invariant it pins and the production line it covers. Reverting that line turns the named assertion red. Compiles clean, 1099 tests pass with 0 regressions, clippy clean under -D warnings, fmt clean. Cross-PR overlap (declared in the PR description): - #134 (anchors auth): composes. The /arweave/anchors route already requires auth; this PR does not change the route. - #285 (advisory-lock session affinity): composes. The durable intent is written inside the same handler that holds the lock from #285; no changes to the lock layer. - #306 (Content-Digest on signed requests): composes. PR 1 persists the Content-Digest header that #306 makes mandatory. - #314 (small-order Ed25519): independent. PR 1's tests use strong keys. - #324 (libp2p keypair persistence): independent. PR 1 does not touch p2p identity. - #325 (gossip ref-update auth): independent. PR 1's signed envelope is the HTTP-side equivalent, not the gossip-side. - #382 (replication withheld-subtree trees): independent. PR 1 does not touch replication or pin selection. * fix(node): address four reviewer findings on #26 split 1/4 - P1-A: add startup reconcile step that promotes `prepared` rows to `applied` when the on-disk ref matches the row's `new_sha`. The recovery drain (which only reads `applied` rows) can now pick up a ref that landed when the live handler's `mark_pending_ref_transitions_applied` call errored or was interrupted. Strict SHA equality is the load-bearing check — a `prepared` row whose target did NOT actually land stays `prepared`. - P1-B: route the live handler's cert issuance through `cert::issue_ref_certificate` (the upsert) instead of `issue_ref_certificate_idempotent` (DO NOTHING). A re-push to the same ref now updates the cert's `old_sha` / `new_sha` / `pusher_did` / `issued_at` / `signature` to the new transition while preserving the deterministic `cert_id`. The recovery drain keeps the idempotent variant; both paths collapse to one row. - P2-A: refactor the drain into a `drain_pending_ref_transitions_with` testable seam that does per-row log-and-continue, and add `drain_pending_ref_transitions_all` that loops `DRAIN_PER_PASS_LIMIT=1000` rows for `DRAIN_MAX_PASSES=10` passes. A failing row no longer stalls the batch; a backlog above 1000 rows is fully processed across passes. - P2-B: add a `first_ref_name` column to `pending_ref_transitions` via migration v28. The live handler hoists a `first_ref_name` local and persists it on every row of the same `request_id`. The drain's `derive_one` keys the push event id on `row.first_ref_name` instead of `row.ref_name`, so live and recovery produce the same id and `ON CONFLICT (id) DO NOTHING` collapses a multi-ref push to one push event row (and one trust- score bump). Cert and anchor ids stay per-ref / per-transition. * fix(node): address four reviewer findings on #26 split 1/4 (round 2) * fix(node): address reviewer round-3 findings on #26 split 1/4 - P1: Delete outbox rows after live durable effects complete so they don't replay on every restart - P1: Parse git report-status for per-ref ok/ng results; mark only proven rejections as cancelled, uncertain outcomes as recoverable - P1: Introduce 'uncertain' state for receive-pack errors where some refs may have landed; reconcile checks these against disk at startup - P2: Promote deletion transitions during reconcile (new_sha == ZERO_SHA with absent ref = successful deletion) - P2: Loop reconcile across multiple passes so backlogs beyond the first page are processed in the same startup Closes review round 3 findings from reviewer-1 and reviewer-2. * fix(node): fix CI failures from round-3 changes - Add COMMENT ON TABLE to v29 migration so migration_bodies_are_non_empty passes - Return error on non-zero receive-pack exit (preserving backward compat with tests that expect Err(AppError::Git(_))) while still parsing report-status for outbox row handling * fix(node): update inv22 gate tests for receive_pack_raw refactor - F3 gate: scan for smart_http::receive_pack_raw( instead of receive_pack( - U5 gate: scan for all_refs_ok instead of receive_result.is_ok() * fix(node): require reflog proof before the reconcile promotes a row (#26 split 1/4) Closes the second half of the reviewer's round-3 [P1]: the startup reconcile still promoted on `disk_refs.get(ref) == row.new_sha`, which also matches a coincidental current tip (old=B, new=A while the ref is already A). That is not an exotic case — it is the ordinary shape of a REJECTED push, since git refuses an update whose expected old value does not match the ref. Both the SHA check and the MAX_RECONCILE_AGE window pass, the push never happened, and the drain then writes a push event, a signed certificate and an anchor for it. The live path proves landings from git's report-status body; that body is gone by the time the reconcile runs, so the proof is re-derived from the repository. `git::store::ref_reflog_entries` reads `logs/<ref>` and a row is promoted only when an entry carries its exact `old -> new` pair, stamped at or after the row was written (REFLOG_CLOCK_SKEW absorbs git's whole-second truncation against the row's sub-second `created_at`). The timestamp floor is what separates a landing from a later push that re-introduced the same pair: proof must postdate the intent it proves. Deletions stay exempt from the reflog half and keep the absence-plus-age rule already in place — git removes a ref's reflog along with the ref, so no other evidence can exist for one. `init_bare` now sets `core.logAllRefUpdates`, which bare repos default off; a repo with no reflog yields no proof and its rows stay put for human-attended recovery rather than becoming accounting the node cannot substantiate. Tests: reconcile_refuses_a_coincidental_tip_with_no_reflog_proof, reconcile_promotes_a_row_the_reflog_proves, reconcile_leaves_a_row_prepared_when_the_repo_keeps_no_reflog, reconcile_refuses_a_reflog_entry_older_than_the_row, reconcile_still_promotes_a_landed_deletion_which_can_have_no_reflog, init_bare_keeps_reflogs_so_a_landing_can_be_proven, ref_reflog_entries_is_none_when_the_repo_kept_no_log. No migrations added or renumbered. * fix(node): walk the reconcile backlog on a keyset cursor (#26 split 1/4) The multi-pass reconcile looped but never advanced. Each pass re-issued the same cursor-less `LIMIT n` query, and unlike the drain — which deletes every row it finishes, so its next page is always new work — the reconcile leaves every row it cannot promote exactly where it was. One unprovable row at the head of the ordering therefore pinned page one on every pass, and the backlog behind it was never examined at all, which is the condition the multi-pass loop was added to remove. Rows keep ageing toward MAX_RECONCILE_AGE while they wait, so "picked up next restart" can mean "never recovered". Pages now resume strictly after the `(created_at, id)` of the last row EXAMINED, via `list_pending_ref_transitions_prepared_or_uncertain_after`. A keyset cursor rather than an OFFSET because rows leave the set underneath the walk when they are promoted: an absent row simply does not appear on a later page, and it can never shift an unvisited row into a page that was already read. Pass budget, per-pass limit, short-page exit and the residual-backlog warning are unchanged. This matters more with the reflog gate in place: a landing in a repo that keeps no reflog is PERMANENTLY unprovable, so before this it would jam page one on every startup rather than once. Tests: reconcile_all_walks_the_backlog_past_the_first_page, reconcile_all_advances_past_an_unprovable_row (whose blocker is a no-reflog row, the class the gate introduces). * fix(node): bound the reflog read to a recent tail (#26 split 1/4) `ref_reflog_entries` slurped the whole reflog. A reflog grows one line (~150 bytes) per ref update and how often a ref is updated is PUSHER-controlled, so that was an attacker-sized allocation, taken once per stranded row, at startup — the moment the node can least absorb it. Every other attacker-influenced read in this crate is bounded (read_body_capped, MAX_SIGNATURE_ENTRIES, the pin/walk budgets); this one should be too. The bound costs nothing in proving power, which is why it is a ceiling rather than a tradeoff: the landing gate only ever accepts an entry stamped at or after `created_at - REFLOG_CLOCK_SKEW`, and reflogs are append-ordered oldest-first, so every entry that could possibly qualify is at the tail. The read now seeks to the last REFLOG_TAIL_BYTES (256KB, ~2000 recent updates to one ref) and `take`s that many bytes, so the allocation stays bounded even if the file grows between the stat and the read. A window into the middle of a file starts mid-record, so the bytes before its first newline are discarded: without that, a boundary landing inside a record's old SHA leaves a suffix that still tokenizes and yields a truncated SHA as a bogus `old -> new` pair — and landing proof is an exact pair match, so a bogus pair is a fabricated proof. An entry older than the window reads as absent, which is the safe direction: the caller treats "no matching entry" as unproven and leaves the row where it is. `Ok(None)` for a missing file, and the absence-of-evidence contract, are unchanged. Tests: ref_reflog_entries_reads_whole_records_from_the_recent_tail (lays the file out so the boundary lands 20 bytes into a record's old SHA — the alignment that would otherwise parse), an_entry_older_than_the_reflog_tail_reads_as_unproven. * fix(node): read the report through both side-band frames The report-status is framed twice whenever the client negotiates side-band-64k, which `git push` over smart HTTP does: the outer side-band envelope carries a stream that is itself pkt-line encoded. One strip pass left the first line as `000eunpack ok`, so the unpack check failed and parse_report_status returned None — read by the caller as "no report", which keeps every declared ref. The per-ref landing gate was therefore inert for exactly the pushes it exists to filter. The regression test is a byte-for-byte capture from git 2.50.1 rejecting one ref of a two-ref push, because a hand-written single-framed fixture parses under the old code and hides the case. * fix(node): address round-4 reviewer findings on #26 split 1/4 * fix(node): rustfmt round-4 reviewer fixes * fix(node): remove empty line after doc comment * fix(node): drop redundant reflog gate; implicit-ok on exit-zero no-report * fix(node): address round-5 reviewer findings on #26 split 1/4 * fix(db): v30 migration — add receive_pack_requests table and ordinal column The reviewer's round-5 finding: the push event identity was encoded into a mutable per-ref column (first_ref_name) whose correct value is knowable only after git, and the correction was not committed atomically with the per-ref outcomes. A crash between git updating a later ref and the rewrite left the durable rows naming the rejected ref, and derive_one's row.ref_name == row.first_ref_name guard then meant no push event ever landed for the accepted child. Per the state-transition model at .gravirei/plans/state-model-durable-post-receive.md, this migration introduces the request-level record that owns the push event and the trust score, and extends the per-ref child with an ordinal column the drain and the effect executor walk together. first_ref_name is dropped; the push event id is keyed on (request_id, accepted_ordinal) and lives on the request row. The next commit (handler rewrite) wires the live path and the recovery drain against the new tables. BREAKING CHANGE: pending_ref_transitions.first_ref_name is dropped. Callers that read or write that column must move to receive_pack_requests.accepted_ordinal. * fix(node): rewrite receive-pack handler against the request-level model (#26 split 1/4 step 2) The v30 migration added receive_pack_requests and dropped first_ref_name, but no Rust code used either. The handler still ran a four-branch per-ref state flip plus a first_ref_name rewrite after git returned, which is the mixed-outcome bug a push with the first ref rejected exposes: the live path and the drain computed different push-event ids. - Insert a receive_pack_requests row in state `received` BEFORE git runs (insert_receive_pack_request), carrying the request bytes and the SHA-256 of the body. Crash between intent and git return is now recoverable via the reconcile step. - Drop the first_ref_name rewrite at api/repos.rs:2454. The push event identity is now (request_id, accepted_ordinal); accepted_ordinal is computed once at the per-ref state flip. - Re-key push_event_id_for and ref_cert_id_for on (request_id, ordinal). Anchor job id stays per-transition. - Transition the request row to outcomes_committed (with parsed_report and accepted_ordinal) or rejected_at_git (no report + non-zero exit) in the same handler tail as the per-ref state flip. The drain (step 3) picks up outcomes_committed rows; today the live path also runs the per-ref effects inline. - Stub the step-3 surface (mark_request_effects_pending, complete, list_receive_pack_requests_due, update_request_attempt) with #[allow(dead_code)] + contract-pin comments so the rewrite is bisectable: reverting this commit restores the pre-rewrite live path. The U5 gate (replication tail spawn inside push_succeeded, before guard.release) is preserved; inv22_replication_tail_spawns_at_the_ durability_boundary stays green. * fix(node): use raw bytes for request_bytes_hash (#26 split 1/4 step 2) The v30 migration defined request_bytes_hash as BYTEA but the handler was binding a hex-encoded String. Postgres rejected the insert with "column request_bytes_hash is of type bytea but expression is of type text" and the handler shed the push with a 503. Switch the handler to bind the raw 32-byte digest and update the ReceivePackRequest struct field type to Vec<u8> to match. The drain-side test fixture already used Vec<u8> and needs no change. Pinned by the bin test suite going from 24 failures (all the receive-pack-cap tests that send a 4-byte body) to 0 regressions on the new model. * fix(node): factor out apply_request_effects; drain walks receive_pack_requests (#26 split 1/4 step 3) The step-2 commit moved the request row to be the unit of work but left the per-ref effects fan-out inline in the handler and the drain walking pending_ref_transitions per-ref. Step 3 factors the effects fan-out into apply_request_effects(state, request_id) and rewrites the drain to walk receive_pack_requests. - apply_request_effects lives in durable_outbox.rs; the live handler in api/repos.rs and the drain both call it. Idempotent on (request_id, accepted_ordinal). Returns EffectsOutcome::{Done, Nothing, Retry}. - Drain switches from list_pending_ref_transitions_applied + derive_one to list_receive_pack_requests_due + apply_request_effects. derive_one and the per-ref drain entry points are deleted. - The step-3 stubs in db/mod.rs (mark_request_effects_pending, mark_request_complete, list_receive_pack_requests_due) lose their #[allow(dead_code)] annotations; every stub has a caller in this PR. - Reconcile (reconcile_prepared_from_disk_all) is unchanged. The crash window between intent durable and outcomes commit still routes through per-ref reflog proof. - Drain tests rewritten to stage receive_pack_requests rows; the per-ref fixtures (make_row, lookup_accepted_ordinal) are gone. - New inv26_step3_live_and_drain_share_apply_request_effects assertion pins the live/drain sharing and the per-request drain walk. * fix(node): bounded retirement purge for terminal request rows (#26 split 1/4 step 4) The v30 partial index idx_receive_pack_requests_completed_at exists but no code reads it. Step 4 wires a periodic purge task that deletes terminal `complete` and `rejected_at_git` rows older than the retention window, along with their per-ref children. `quarantined` (a step-5 state) is never purged by the timer. - `purge_completed_receive_pack_requests(older_than, limit)` — deletes parent requests using the v30 partial index. - `purge_completed_pending_ref_transitions(older_than, limit)` — deletes children of purged parents, gated on the child's own applied_at / cancelled_at. - `purge_request_queue(db, retention_days, limit)` — orchestrator in durable_outbox.rs that calls both and returns the totals. The deletion order (parents first, then children) is the contract; a crash mid-purge leaves orphaned children that the next pass will pick up. - `spawn_queue_lifecycle_sweep` in main.rs — periodic task on the same detached pattern as `spawn_legacy_cid_sweep`, 24-hour interval, shutdown-aware. The interval matches the spec's "one per cluster per day" target. - Config knobs `queue_retention_days` (default 7, range 1..=365) and `queue_purge_batch` (default 1000, matches DRAIN_PER_PASS_LIMIT). - 3 new `durable_outbox::drain_tests::purge_*` tests pin the contract: only old terminal rows are deleted, the second pass is a no-op, and a row inside the retention window survives. - New `inv26_step4_queue_lifecycle_purge_is_wired` gate asserts the wiring (main.rs calls the purge, the DB helpers exist, the Config knobs exist). * fix(node): reference-transaction marker + quarantined state + failure matrix (#26 split 1/4 step 5) Steps 2-4 gave the request row the unit of work, the shared executor, and the bounded retirement policy. Step 5 closes the evidence gap: the reconcile now requires a per-request marker ref (refs/gitlawb/requests/<id>) whose value matches request_bytes_hash. A missing or mismatched marker quarantines the request; an operator reclassifies it. - v31 migration: adds `quarantined` to the state vocabulary and a partial index for operator queries. - Handler writes the marker ref before git-receive-pack; the marker is causally bound by being in the same async task as the receive-pack call. The marker's value is content-addressed (git hash-object of the request bytes), so the gate compares consistent SHAs on both sides. - git::store::read_ref reads a single ref's value, returning Ok(None) for absent refs. Used by the marker gate. - git::store::marker_value_for computes the content-addressed marker value; both the live handler and the reconcile use it so the write and the read agree. - Reconcile gains a marker gate between the age check and the reflog proof. Mismatch or absent ⇒ mark_request_quarantined + mark_children_rejected_for_quarantined_parent. - effects_max_attempts bound (config knob, default 8) flips retry-stuck requests to `quarantined` after N attempts, closing the infinite-retry DoS window. - New failure_matrix_tests submodule covers the spec's outcome × ref-kind × exit-point × recovery-scenario matrix (6 cells). - New inv26_step5_marker_quarantine_and_bound_are_wired gate asserts the marker gate, the bound check, the handler's pre-receive-pack ordering, and every load-bearing helper. - Existing 7 reconcile tests updated to stage a marker ref via the new `stage_marker` test helper. * fix(node): converge durable post-receive to singular request-level lifecycle Unify producer, evidence, aggregate outcome, executor, retry, and retirement on the request row: atomic intent and outcome commits, synthetic normalized reports for implicit-ok/reconciled paths, reconcile parent promotion with fail-closed deletions and competing-claimant guard, retry progression from both executable states with backoff/quarantine, terminal-parent-gated purge with marker cleanup and hidden refs, minimal digest-only intent plus request-level RFC9421 proof (v32). * fix(node): harden request occurrence lifecycle, proof, retry, and retirement Reject incomplete report framing and omitted refs as indeterminate, preserve uncertain children on completion, terminalize all-rejected, bind reconcile with landing history, durable proof with ack gate, occurrence-keyed anchors, background due loop, deletion quarantine with operator resolve, children-before-parent purge with marker tombstones, recovery prereq upgrade check, and bounded marker runner. * fix(node): keep recovery prereq upgrade best-effort on push path Refusing pushes when git config upgrade fails broke fake-git and non-repo disk-path tests with 503. Reconcile already fails closed on missing reflogs, so downgrade to warn-and-proceed. * fix(node): close reviewer round on outcome authority and lifecycle ownership Gate effects on unpack_ok with cleared ordinal and terminal no-effects, intersect executor with applied children, treat absent report-status as indeterminate regardless of exit, require landing history before child delete, claim due requests with recoverable leases, dedupe webhooks by occurrence ledger, ack-gate proof retention with terminal disposition, fair marker tombstones with dead-letter, kill_on_drop marker bounds, and loud migration version collision guard. * fix(node): load-bearing coverage for intent, guards, and delivery ledger Handler intent test plus inv22 gate so disabling the durable insert turns red; unpack-false, mixed ok/ng, divergent cancelled, partial sibling, and proof-ack tests pin each executor guard; webhook deliveries record sent_at with stale-pending reclaim; stale parsed_report comment corrected. * fix(node): centralize live retry, bound outcome commit, close retention orphans Route live Retry/Err through schedule_request_retry_or_quarantine so attempt progression matches the drain; retry post-git outcome commits 3x before restart repair; block terminal purge while non-terminal children exist; add v27 upgrade-path test. * fix(node): document attended-restart commit policy and pin executor retry arms Track outcome-commit success, skip metrics/touch on failure, correct the lease-expiry comment to startup reconcile; add restart-repair test for stuck received parents plus RED tests for landing-history and proof-ack failure paths. * fix(node): correct marker prereq comment to warn-and-proceed * fix(node): single partial-completion model with disposition gate Split apply_request_effects into phase-1 bundle (push, cert, anchor, history, webhook, proof ack before child deletion) and phase-2 gate that retries while non-cancelled siblings remain; Nothing only when nothing owed. Gate tail, Tigris, and effects on PostGitDisposition with documented replication carve-out; add partial-sibling webhook and disposition tests; fix stale comments. * fix(node): close review round on outbox ordering, marker visibility, and repair reachability Address the P1/P2 review findings: await the webhook delivery claim before accepted-child deletion; exempt the marker namespace from the fail-closed ref peeling guard so visibility walks tolerate blob-pointing request markers; keep request marker deletion bounded in the purge path; promote cross-page siblings via idempotent report superset; run stuck-parent repair when the prepared queue is empty; bound the durable intent write while admission permits are held; remove dead proof verification scaffolding. * fix(node): close review round on pre-git refusal lifecycle, namespace gate, and promotion merge * fix(node): close review round on namespace gate, intent lifecycle, and promotion atomicity Namespace gate hardening: - Refuse non-UTF-8 ref-update lines (git accepts bytes ≥0x80; gate never sees them) - Catch bare 'refs/gitlawb' name (starts_with missed it) - Add deny test for both holes plus refusal wiring verification Intent lifecycle guards: - Add IntentDropGuard: terminalize stranded intents on dropped futures - Bound refuse_receive_pack_before_git to 10s (no statement_timeout on pool) - Guard mirror fetch: hideRefs blocks non-exempt refs/gitlawb/* from +refs/*:refs/* Promotion merge atomicity: - Take row under SELECT FOR UPDATE so merge is atomic with state read - Prevents racing due worker from stranding newly applied children outside report * fix(test): correct inv22 namespace gate assertions to match actual implementation The test was looking for the wrong patterns: - UTF-8 check: error message string, not pre-loop condition - Namespace check: `*r == "refs/gitlawb"`, not `ref_name ==` - Visibility exemption: `&&` logic (both false to exclude), not `||` * chore: cargo fmt * fix(node): phase-aware intent guard, mirror import prune, terminal-state merge * fix(node): guard cleanup retry with fates, fail-closed mirror prune, refusal coverage * fix(node): bound guard attempts, sibling-wait outcome, fail-closed prune enumeration * fix(node): sibling-wait outcome, no-report path, bounded prune and guard retry * fix(node): no-report durability, cert landing order, bounded reconcile git, flush checks * fix(node): gate trust-score bump on a new push event row * chore: update rustls 0.23.37 → 0.23.45 to resolve RUSTSEC-2026-0285 * fix(node): surface git's report-status when a stdin EPIPE follows child exit * fix(node): cap receive-pack ref-update count before per-ref work and intent * fix(node): cap ref-update parsing in-loop and dedupe identical commands * docs(node): correct round-4 comment claims on client outcome and gate cost * fix(node): single replication tail, proven marker taxonomy, claimed webhook delivery (#558) - One post_receive_replication_tail per push: the carve-out spawn and the durability-bound spawn are mutually exclusive positions on the same proven-landed ref set; inv22 now fails on any unfiltered or ungated spawn. - Marker reconcile: bounded-read errors (git Err/deadline) stay prepared and retry instead of quarantining as 'missing marker ref'; only a successfully read marker proves missing or mismatch. Err-injection test pins no-quarantine on unproven reads. - Claimed webhook delivery: the synchronous claim phase returns the claimed Webhook rows so the send task never re-lists; a list failure after claim can no longer drop claimed deliveries. Source gate pins it. * docs(node): name claimed-hooks variant in fire_event_occurrence doc * fix(node): order pending_ref_transitions create before outbox alters, fill v29 (#558) * fix(node): proven-missing marker taxonomy, v38 upgrade test, report-ok tail fixtures (#558) * fix(node): read marker refs via rev-parse so present refs return their oid (#558) --------- Co-authored-by: Gravirei <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-09 23:40:38 +00:00
3 people	c805e35f40	fix(node): bound GraphQL query cost (#552) ... * fix(node): bound GraphQL query cost * fix(node)!: bound repo names so emitted cursors always parse The reposPage cursor embeds base64(JSON(owner_did, name)) while parse_repo_cursor rejects cursors over 4096 bytes, but repo names were unbounded on every write path. A single long-named visible repo made the server emit an endCursor it then rejected as "invalid repository cursor", dead-ending pagination for every caller past that row. Bound names to MAX_REPO_NAME_LEN (100 bytes) at the create_repo and fork_repo handlers via a shared validate_repo_name helper (bad charset is still a 400), and reject overlong peer-supplied names in Db::upsert_mirror_repo (the sync caller already drops the error and skips the mirror). Also in this round: - docs/graphql-pagination.md states the 200 page-size maximum and corrects the over-budget remedy: a smaller limit does not reduce the reposPage or repos price, which is charged at the full MAX_VISIBLE_REPO_PAGE_SIZE scan regardless of limit. - Tests pin refUpdates(limit: -1) clamping to zero rows, the parse_repo_cursor wrong-shape-JSON arm with its exact "invalid repository cursor" message, the longest-admissible-name cursor round-trip, and the mirror write-path rejection. BREAKING CHANGE: the GraphQL `repos` field now returns an error instead of the full list when more than MAX_VISIBLE_REPO_PAGE_SIZE (200) repositories are visible; larger lists must use `reposPage` with `limit`/`after`. Repository names are additionally limited to 100 bytes at creation, fork, and mirror-registration time. * fix(node): address beardthelion P3 findings on name bound and input messages - Assert each rejection's own message in the reposPage invalid-inputs loop: the loop no longer accepts either message for every query, so a misattributed message (limit arm returning the cursor message) goes red. - Pin the create/fork name-bound wiring through the handler: new test drives create_repo with an overlong name via test_state_lazy and asserts the 400; deleting the validate_repo_name call site would proceed to the lazy DB pool and fail with a connection error instead. - Document the API-side vs disk-side validate_repo_name divergence on the new function, and tie the store's literal 100 to MAX_REPO_NAME_LEN so the two validators cannot drift apart on length. --------- Co-authored-by: Marlowe <[email protected]> Co-authored-by: cairn-intern <[email protected]>	2026-11-08 23:28:09 +00:00
beardthelionandGravirei	37947208c5	docs: state that GITLAWB_ENFORCE_OWNER_PUSH defaults to true (#537) ... * docs: state that GITLAWB_ENFORCE_OWNER_PUSH defaults to true PR #330 flipped the code default to true and updated .env.example, the README env-var table, the AWS guide, the macOS app, and docs/RUN-A-NODE.md, but two prose mentions in operator-facing docs still describe the opposite. SECURITY.md line 57 ("Repository write authorization defaults" section) and README.md line 69 (Known limitations) both say the flag "defaults to `false`", which contradicts the live runtime and the rest of the same files. Reviewer 2 closed #224 with this as the P2 finding to fix in the config-and-policy split. The correct text already exists in the README env-var table; the prose restatements were missed. Bring the prose into line with the code. - SECURITY.md: rewrite the section to lead with the default being true and describe `false` as the explicit rolling-upgrade override. - README.md: rewrite the known-limitations bullet to state the default is true and the `false` opt-out is a rolling-upgrade only. - Add `crates/gitlawb-node/tests/owner_push_default_docs.rs`: a regression guard that reads the four surfaces (clap default, .env.example, README known-limitations prose, README env-var table, SECURITY.md) and asserts each one says the default is `true`. Reverting either doc edit turns one named test red; flipping `default_value_t` back to `false` turns the env example test red. Each surface is checked independently so a stale doc lies alone. No code change. No new migration. No dependency change. Compiles clean, clippy clean under `-D warnings`. Refs: closes the P2 finding from the review of #224. * fix(docs): clarify default value consistency for GITLAWB_ENFORCE_OWNER_PUSH * docs(readme): drop the default-open roadmap line and the em-dash punctuation --------- Co-authored-by: Gravirei <[email protected]>	2026-11-07 00:38:41 +00:00
beardthelionandKevin Codex	ed76adfe29	fix(node): fence a repo publish on the write attempt that owns it (#285) ... * fix(node): close the advisory-lock probe's session if it is dropped mid-acquire A cancelled .await does not cancel an already-sent SQL statement, so a pg_try_advisory_lock whose future is dropped still takes the lock server-side while the caller abandons the result, leaving nothing to release it. The connection then returns to the pool holding the lock and wedges that repo until sqlx recycles the session. Introduce LockProbe, which owns the connection across the in-flight try-lock and closes it in its own Drop if it is still held. close_on_drop is a one-way setter, so the arming lives in Drop rather than being set up front and cleared on success; disarming is Option::take, which is what into_conn does once an acquire is actually observed. This is now the only place that issues pg_try_advisory_lock. The committed gate drops a probe without taking its connection, which is the state a cancellation leaves behind, and polls a standalone observer until the lock frees. Deterministic on purpose: the timing sweep that found this window leaks roughly 1 in 600, which is not something a CI gate can rest on. Observed RED before this change with the lock still held for the full 10s window. Refs #279 * fix(node): add a dedicated, lazily-connected advisory-lock pool Pinning a connection for the lock's lifetime is only safe if those connections come from somewhere other than the pool serving ordinary request handlers, otherwise a push burst starves every other query. Add GITLAWB_DB_LOCK_POOL_MAX_CONNECTIONS (default 32) and a Db::lock_pool builder, with the sizing tradeoff documented on the field and in .env.example: every in-flight write pins one connection here, so the value is a hard ceiling on simultaneous writes node-wide. The pool connects lazily on purpose. The main pool must connect eagerly because it runs migrations, which is why it needs connect_db_with_retry's backoff and degraded-server handoff; that function is not a generic retry helper and the lock pool is built well after the db-ready handoff has already resolved. A lazy pool has no startup work, so it adds no new way for the process to fail to boot and needs no second copy of that machinery. If Postgres is unreachable when the first write arrives, that write fails on the pool's own acquire timeout, like any other database-backed request. Pure configuration, so no proof-first cycle: the knob is covered by a parse/default/reject-zero test. Db::lock_pool has no caller until the guard wiring lands, hence the temporary dead_code attribute. Refs #279 * fix(node): hold the lock-owning connection in RepoWriteGuard (#279) Postgres advisory locks are session-scoped: only the backend that took one can release it. acquire_write took the lock through fetch_one(&pool) and release unlocked through execute(&pool), two independent checkouts, so the unlock usually landed on a session that held nothing and returned false. Measured on main: two writers on one node and the same repo BOTH acquired, 50 of 50 sequential cycles leaked, and 100 writes left 100 orphaned advisory locks on the server. The guard now owns the PoolConnection that took the lock, drawn from the dedicated lock pool, and releases on that same session. The retry loop probes through LockProbe so a cancellation mid-acquire cannot strand the lock, and hands the connection back before each backoff so a spinner on a contended repo does not pin a slot while idle. Pool exhaustion is deliberately not retried. It is a different condition from lock contention, and retrying it would spend all 60 attempts on a capacity problem unrelated to this repo while reporting it as someone else holding the lock. Both #279 acceptance tests were observed RED first: the exclusion test admitted the second writer, and the leak test reported 1 lock held where 0 was required. Both GREEN after. Full crate suite 516 passed. Db::pool() is removed because this change was its only caller. Refs #279 * fix(node): free the advisory lock when a write guard dies without releasing A guard can exit without reaching release(): an early ? on the pre-write download, a panic, or an axum handler future cancelled when the client disconnects. Its session still holds the lock, so returning that connection to the pool would block every future write to the repo until sqlx recycles it, which on the 0.8.6 defaults is ten minutes idle or thirty minutes lifetime. Close the session instead and let Postgres free the lock at session end. One hazard found by writing the teardown test rather than by reasoning: PoolConnection::drop spawns onto the runtime for both closing and returning, and panics outright when no runtime handle exists. That panic would fire inside a Drop and abort the process during unwind. It is not introduced by this commit, it comes with owning a PoolConnection at all, but this is where it becomes reachable. So Drop checks for a runtime first and, with none, leaks the handle deliberately rather than panicking: the process is already exiting and socket teardown ends the session, which is what frees the lock at exit anyway. Observed RED before the fix, with the lock still held for the full 10s poll window against a standalone observer on a no-reap pool, and the teardown case panicking in sqlx-core connection.rs:208. Proven load-bearing after: neutering the close_on_drop call turns the drop test RED again. The must-not case (a released guard reuses its backend pid across four writes on a pool sized 1) passes in both states, so the signal is specific to the abandoned-guard path. Full crate suite 519 passed. Refs #279 * fix(node): read the advisory unlock's result instead of discarding it pg_advisory_unlock reports "you did not hold this lock" as a false RETURN VALUE plus a server WARNING, never an error, so let _ = execute(...) could not distinguish a real release from a no-op. Three blindnesses stacked in those four lines: execute discards the row, the boolean lives in the row, and let _ discarded the Result too. Read it through fetch_one into (bool,). A false means this session's lock state is not what we believe it is, so the connection stays in the guard for Drop to close rather than being handed back to the pool as clean. Only a confirmed unlock returns it. A query error gets the same treatment, since the lock must not outlive a session we can no longer reason about. Note this is the only unlock site: the pre-write download's error path returns through the guard, so Drop covers it and there is no second place to keep in sync. RED before: the connection came back on the same backend pid after an unlock that returned false. GREEN after, and proven load-bearing by treating false as success, which turns it RED again. The must-not case (a normal release still reuses its backend) passes in both states, so this does not over-close the happy path. Full crate suite 520 passed. Refs #279 * fix(node): bound the object-storage transfers that run under the write lock Two transfers happen while the per-repo advisory lock is held: the archive download inside acquire_write, which runs after the lock is taken and before the guard exists, and the upload inside release. Both were free before the guard pinned a lock-pool connection, because the lock's connection went back to the pool immediately. Now an unbounded stall holds a lock-pool slot for its whole duration, so enough concurrent stalls deny every write on the node with no reaping path. Add GITLAWB_LOCK_HELD_TRANSFER_TIMEOUT_SECS (default 300) and route both through it. A timed-out upload is UNKNOWABLE rather than failed, so the timeout arm takes no compensating action: the PUT may well have landed. The lock is released either way, which trades a narrow last-writer-wins window for not wedging the repo behind a stalled transfer. Deliberate, and stated here rather than discovered later. Note what is NOT bounded by this knob: acquire_fresh's download. It runs before any lock is taken, so it holds nothing. An earlier revision of this change put the bound there by mistake, because both functions contain an identical download call and the first match won. Two disclosures. Coverage: the committed tests cover the bound mechanism, not the wiring. Driving a genuinely stalled transfer through acquire_write needs either the object-store abstraction (out of scope) or a process-global AWS_ENDPOINT_URL_S3 mutation, which would make the suite order-dependent under the concurrent runner. That a stalled transfer is bounded is therefore verified by reading, not by execution. Behavior: on timeout with a local copy present, the download falls into the pre-existing self-healing fallback and the write proceeds against that local copy. This widens the conditions reaching that path from corrupt-or-unreachable to include merely-slow, so a stale tree could now be written and re-uploaded on a slow link. Kept consistent with the existing failed-download behavior rather than inventing new semantics here; changing it is its own decision. Full crate suite 522 passed. Refs #279 * fix(node): authorize before taking the write lock in close_issue close_issue acquired the per-repo advisory lock, then ran the owner-or-author check, then returned 403 with the lock still held. That was harmless only because the lock excluded nothing. Making it work, as the preceding commits do, turns this ordering into a denial primitive: any caller with repo read access could take the write lock on demand and be refused the write, while a legitimate writer burned its 60-attempt retry budget against a lock held by someone with no write authorization. On a public repo that is every permissionless identity. This series creates the exposure, so it closes it in the same series. The owner check is cheap and moves above the lock outright. The author fallback needs the issue's git-JSON blob, since there is no author column, so it now reads through acquire() rather than acquire_write(): an issue's author is set at creation and never changes, so reading it outside the lock races nothing, and only the mutation needs exclusion. Two deliberate behavior choices. A non-owner whose authorship cannot be established gets 403 rather than 404, so this route does not tell an unauthorized caller whether an issue exists. And the owner's existing 404-for-a-missing-issue path is preserved by re-reading under the guard, which the mutation wants anyway. Swept the other three acquire_write sites: merge_pr owner-gates at :200 before acquiring at :214, the repo write path checks did_matches at :916 before :933, and create_issue's read gate is legitimate because that caller IS authorized for the action it performs. close_issue was the only one with the wrong order. RED before: with an independent session holding the repo's lock, a stranger's request sat in the retry loop until the 3s deadline fired (Elapsed), which is the wedge itself. GREEN after, refused immediately. Proven load-bearing: disabling the pre-lock refusal restores the Elapsed. Full crate suite 523 passed. Refs #279 * fix(node): address the review findings on the advisory-lock series Seven fixes from a seven-reviewer pass. Eight of the nine substantive findings were defects introduced by this branch, and two contradicted claims made in its own earlier commit messages. P0, the worst of them. The under-lock download's timeout was folded into the same Err as a corrupt archive, so it fell through to the use-the-local-copy fallback and the write proceeded. Two ways that destroys data: we do not know whether we hold the latest tree, so re-uploading can overwrite another node's newer archive; and the abandoned download's extraction runs in an uncancellable spawn_blocking that ends in remove_dir_all + rename over the same path, so git would have been running against a directory a background task was about to delete. Now the Option is branched rather than collapsed: a genuine fetch error keeps the local-copy fallback, a timeout refuses the acquire and lets the guard's Drop free the lock. LockProbe closed its connection on ordinary contention, so a 60-attempt spinner tore down 60 backends -- the exact behavior an earlier commit claimed it avoided. The test written to guarantee that asserted the HOLDER's lock count, which cannot observe the probe's own connection, so it passed throughout. The correct predicate turned out to be narrower than the first attempt at this fix: not "we saw an answer" but "we positively know nothing was acquired," because a true answer means the lock IS held and dropping without handoff leaks it exactly as a cancellation would. The first attempt reopened U1's leak and U1's gate test caught it. tigris.exists() ran unbounded inside the lock-held span, so the knob's promise was not kept and the docs were wrong about the shape. The HEAD and the download now share one budget, so worst-case occupancy is one budget per span rather than two. close_issue's pre-lock author read used acquire(), whose fast path returns on a cached directory and never contacts object storage, so on a multi-node deploy an issue author would be refused their own issue. Now acquire_fresh, which refreshes without taking the lock -- the property the comment already claimed. That comment also asserted authorship is immutable, which a reviewer disproved by force-pushing a forged blob at refs/gitlawb/**; reworded to the real justification (a pre-check whose mutation re-reads under the guard). The underlying pushability is pre-existing and larger than this branch. The retry loop had no wall-clock cap, reaching ~360s against a 120s proxy idle timeout that was itself lowered from 600s after held connection slots caused a production outage. Capped at 90s, with the bail message naming the real bound. All four acquire_write call sites stringified their error, destroying the anyhow downcast that error.rs documents explicitly ("without this, every database outage surfaces as a 500 instead of a 503"). PoolTimedOut is in that 503 set, so a saturated lock pool told clients not to retry something transient. Now propagated. Tests: six that the plan required and the first pass never wrote. The R4 pool-isolation test (named in the plan as proving the pool split, entirely absent), close_issue's owner and non-owner-author twins (INV-21c, a two-principal gate with only its deny arm covered), a waiter-does-not-block- an-unrelated-repo case, and a config test for the transfer knob. The runtime-teardown test no longer returns green when DATABASE_URL is unset. Drop's no-runtime branch now detaches via leak() instead of mem::forget: PgConnection has no Drop impl, so the socket closes synchronously and the lock frees immediately rather than at process exit. Evidence. The probe predicate is proven load-bearing in BOTH directions: forcing always-close reddens the churn regression, forcing never-close reddens the cancellation gate, and only the correct predicate satisfies both. The author twin reddens when the fallback is removed. One honest limit: the author twin does NOT redden when acquire_fresh is reverted to acquire. With tigris disabled in tests the two calls are identical, so that fix is correct by reading, not by execution -- the same seam that leaves the transfer bound's wiring untested. Full crate suite 529 passed, clippy -D warnings clean. Refs #279 * fix(node): log lock-pool saturation in the request path, not via readiness Completes the shed-in-path half of the availability story. F7 already made a pool timeout a retryable 503 by letting the sqlx downcast through; this adds the operator-facing half, with the pool's own size/idle counters so an incident can distinguish "the pool is full" from "the database is gone" without reproducing it. Deliberately NOT a readiness probe. /ready gates Fly routing with no fail-open, so failing it on a saturated pool would pull this node's READS out of service too and push its write load onto peers carrying the same load. That is the downward spiral AWS's health-check guidance names and the SRE Book documents independently. A lock-pool readiness probe would also add nothing on the reachability axis, because both pools are built from the same database_url, so the existing app-pool ping already answers it. The error is still returned via .context() rather than replaced, because anyhow preserves downcastability through context layers and the 503 mapping depends on it. Cost is bounded by construction: one line per failed acquire, and a failed acquire has already paid a multi-second pool timeout. Refs #279 * fix(node): refuse a write when object storage is unknowable, and shed contention as 503 Four defects a seven-reviewer pass found in the previous two commits, three of them cases where one arm of a match contradicted its neighbour. The under-lock refresh read a failed Tigris HEAD as "no archive" (`exists().await.unwrap_or(false)`), skipped the refresh, and wrote against a possibly-stale tree, then re-uploaded over it. That is the outcome the timeout arm twenty lines below explicitly refuses. A failed HEAD and a failed download leave us knowing different things, so they no longer share a branch: only a download failure after a successful HEAD establishes that the local copy is a sound thing to fall back to and re-upload. A HEAD failure now refuses the write. Lock contention that ran out the acquire deadline surfaced as a 500 carrying the owner slug and repo name in the client-visible body, while pool exhaustion fifteen lines up returned a deliberate 503. Contention is transient and ordinary, so it now maps through a typed `RepoBusy` to a retryable 503 with a fixed body; the detail stays in the log at the raise site. `lock_not_taken` was assigned after the try-lock answered, so an error left a previous `true`-derived value standing and could return a lock-holding session to the pool. It is now cleared before the statement is sent. `leak()`'s no-panic property in the guard's no-runtime Drop branch depended on `min_connections == 0` without saying so; a future tuning change would have silently re-armed a panic inside Drop. Made explicit with the reason. Two tests that did not bind: - `waiter_on_one_repo_does_not_block_another` proved only that two lock keys do not collide. It now samples the pool counters across more than two backoff cycles. RED at 0/50 samples with `drop(probe)` moved after the sleep, the exact defect it names, which the previous version survived. - the exhaustion test's `Ok(Err(_)) | Err(_)` was satisfied by its own outer timeout. It now requires `PoolTimedOut` and asserts the slots are accounted for. RED with the pool sized N+1. `contended_acquire_sheds_as_repo_busy_not_internal_error` is new and drives the deadline path for real; the deadline became a field so it does not wait 90s. RED at 500-vs-503 with the downcast removed. The acquire deadline's docstring claimed a total under the 120s proxy idle timeout, which the 300s under-lock transfer bound in the same function contradicts. It bounds the wait only, and now says so. The backoff is clamped to the remaining budget. * fix(node): re-authorize close_issue under the guard instead of only re-checking existence The guarded re-read matched `Ok(Some(_))` and discarded the blob, so the authorization decision rested entirely on the pre-lock read. A comment above claimed the opposite. That is not a narrow window: `acquire_write` re-downloads the archive after locking, so the tree that gets mutated is routinely not the one the author was read from. With owner-push enforcement defaulting to false and branch protection covering only `refs/heads/*`, `refs/gitlawb/issues/*` is pushable, so a forged author blob landing between the two reads was honored. The blob is already in hand under the guard, so re-asserting owner-or-author costs a deserialize. A non-owner whose issue has vanished gets 403 rather than 404, matching the pre-check's existing refusal to reveal existence. `owner_can_still_close_after_the_reorder` seeded the owner as their own issue's author, which made it unable to fail: with the owner check disabled the author fallback granted the close and the test stayed green. It now seeds a third party, so only the owner arm can grant. RED with `is_owner = false`. Also drops the claim that the author twin covers the acquire-vs-acquire_fresh distinction. `RepoStore::for_testing` hardcodes `tigris: None`, so the two calls are identical in every test here and reverting that line leaves the twin green. Separating them needs an object-storage seam. Stating the gap beats asserting coverage that does not exist. * style(node): rustfmt the new contention test Whitespace only; cargo fmt --check gates the push. * fix(node): shed an unrefreshable write as a retryable 503 with a fixed body The two refusal arms added for the under-lock transfer bound returned bare anyhow errors, so AppError's From impl (which downcasts only sqlx::Error and RepoBusy) landed them in Internal: a 500 internal_error whose body was the error string. Two defects in one. A transient object-storage failure told the client not to retry, and the timeout arm's message interpolated the owner slug and repo name straight into the response body, contradicting the fixed-body policy the RepoBusy arm sets six lines above it. RepoUnavailable follows RepoBusy exactly: a fieldless type raised with the operator detail in a context string, downcast to its own rung, mapped to a 503 whose body interpolates nothing. The detail stays in the log at the raise site. The timeout arm logs at error! where its sibling logs at warn!, deliberately: a 300s stall pinned a lock-pool slot for five minutes and is the condition the transfer bound exists to surface, so it must keep paging when the handler classifier demotes the ordinary blip case. * fix(node): refuse a fresh-copy write when the archive HEAD cannot be read acquire_fresh collapsed a failed HEAD into "no archive exists" via unwrap_or(false) and served the local copy, while the sibling path under the lock already refused on the same condition. A push that hit a storage blip got an advertisement built from a possibly-stale tree, uploaded a whole pack, and was then refused by acquire_write for the reason the advertisement had already swallowed. The authorship pre-check on close_issue read the same way, which is an infrastructure failure resolving toward a denial. A failed HEAD now raises RepoUnavailable, so both callers surface it as the retryable 503 rather than a stale success. The download-failure fallback is unchanged: a present-but-unreadable archive is still self-healed from local. info_refs keeps its map_err for every other failure so the read path's error vocabulary does not move; only the typed error takes the From chain. A test-only TigrisClient constructor pointed at a closed port makes both refusals executable, so this is no longer verified by reading. It also proves the under-lock arm end to end, which the earlier draft had recorded as an untestable gap. * fix(node): log expected transient acquire failures at warn, not error Both acquire call sites logged every failure at error, including RepoBusy, which the raise site already logs at warn. Ordinary write contention paged. The previous commit widened the problem: info_refs now raises RepoUnavailable on a storage blip, so that site would have started paging on the condition this series just classified as transient and retryable. A classifier over the two typed refusals picks the level at both sites, mirroring the startup path's permanent-vs-transient split. Anything it cannot classify still logs at error, so an unknown failure keeps paging. * fix(node): log the read failure the close_issue pre-check folds into a denial The authorship pre-check treated a get_issue I/O error and a genuinely absent issue as the same None, with no log line. The client answer is deliberately identical, since a caller who cannot write must not learn whether the issue exists, but the two are not the same event and an operator had no way to tell a real authorization denial from a filesystem or parse failure behind it. Splitting the arm leaves the 403 exactly where it was and makes the read failure visible in the log. * test(node): poll for the closed session instead of sleeping a fixed 300ms The release-invariant test slept 300ms for a Drop-spawned close before comparing backend pids, which is a coin flip on a loaded CI runner. It now polls pg_stat_activity for the captured pid on a standalone connection, the same discipline poll_until_free documents: a pooled observer would be handed the session under measurement and hide the effect. The conversion was checked against the failure it exists to catch rather than assumed. Pooling the session on an unlock that returned false makes the test fail after the poll deadline, not hang, and no_reap_pool disables idle timeout and max lifetime so nothing but the close under test can retire that backend. A generous deadline would have been the same defect as the sleep. * fix(node): raise a failed fresh download with no local copy as RepoUnavailable acquire_fresh already refused a failed Tigris HEAD as RepoUnavailable, so the handler layer mapped it to a retryable 503. A failed GET on the same path still returned a bare anyhow error, which the info/refs map_err closure stringified to AppError::Git and answered 500. One endpoint therefore told the client a transient object-storage blip was permanent or retryable depending on which call failed, and close_issue's pre-check inherited the same split through its bare ?. Raise it at the source instead of at each consumer: From<anyhow::Error> for AppError already downcasts RepoUnavailable out of the context chain, so both callers pick up the retryable mapping without touching either. The new test drives HEAD 200 with GET 500, which is the exact state the refusal is for: archive present per HEAD, GET failed, no local copy to fall back on. * test(node): assert the second writer sheds as RepoBusy instead of timing out two_writers_on_the_same_repo_are_not_both_admitted wrapped the second acquire_write in an 8-second outer timeout and only checked the future had not finished. That passes for any stall, including lock-pool saturation or a slow CI box, so it could not tell a working shed from an unrelated hang, and it cost 8 seconds on every suite run. Use with_lock_acquire_deadline and assert the typed RepoBusy downcast while the first guard is still held, matching what contended_acquire_sheds_as_repo_busy_ not_internal_error already does. It now also fails loudly if a second writer is admitted, which the timeout version could not distinguish. * fix(node): read the close_issue author pre-check from a non-mutating snapshot The non-owner author fallback refreshed through acquire_fresh, which publishes into the live repo directory: its extract step removes the existing directory and renames the new one into place. That runs with no write lock held, so any signed non-owner could trigger a directory swap underneath an in-flight guarded write on the same path. read_snapshot downloads to a throwaway temp dir and hands back a RepoSnapshot that removes it on drop, so the pre-check still sees fresh data and the live path is never touched. download_to grows a publish flag to serve both shapes from one path. The wedge invariant still holds: a stranger is refused without waiting on the write lock. * fix(node): bound every await in the lock-acquire loop by the deadline The remaining budget was checked before the pool checkout but bounded neither the checkout nor the pg_try_advisory_lock query that follows it. A checkout starting just under the deadline could wait out the pool's own acquire timeout, and a slow query could be accepted after the budget was spent, so the advertised wall-clock cap held only on the fast path. Both awaits now run under the remaining budget and shed as RepoBusy when it runs out. The probe's Drop closes its session, which cannot hold a lock it never confirmed taking, so the query-timeout arm is a plain shed. * test(node): add an S3 mock that really enforces conditional writes The fence work landing next is only as good as what the tests can observe, and a mock that answers 200 to every PUT would make the whole suite vacuous. This one holds the object and its ETag, refuses a mismatched If-Match and an If-None-Match "*" over an existing object with 412, and mints a fresh ETag per successful PUT so two byte-identical archives never share a token. Capture-then-replay is deliberate rather than parking a handler and hoping it resumes: when tokio drops an SDK future the client can tear the connection down and cancel the server task with it. Replaying what arrived models the arm that matters (body fully transmitted, commit decided later) with no timing in it. Six tests pin the semantics in both directions so a hollowed mock cannot hide. * fix(node): stop pretending a held lock fences a late upload The timeout arm claimed that keeping the advisory lock protected a successor from an abandoned PUT. It does not. release takes mut self, so the guard drops the moment it returns and Drop closes the session; measured on this branch, a successor took the same repo's lock 5ms after release returned while the PUT was still in flight. The comment and warn now say what is actually true: the outcome is unknowable, the PUT may still land, the lock releases normally, and a conditional upload is what keeps a late publish from overwriting a successor's archive. Two tests replace the claim. Session disposition is the observable that separates the two shapes, so the unlock is pinned to run and be confirmed on the guard's own session with the connection returned to the pool, checked by backend pid. Successor admission is pinned too, but noted as not what proves the point, since the lock frees within milliseconds either way. * feat(node): let the storage client express and detect a conditional write upload takes an UploadPrecondition (IfMatch, IfAbsent, Unconditional) rather than an Option<String>, so the absent case and the deliberate no-fence case are distinguishable at the type level and a caller cannot lose the fence by passing None. head_etag reads the current ETag alongside exists, which keeps exists and its other callers untouched. A failed precondition has to be classified off the raw HTTP status: PutObjectError models no PreconditionFailed variant, so a 412 arrives as Unhandled with nothing useful on it. 412 is always a lost precondition and 409 is one under IfAbsent. 404 deliberately is not: no archive delete exists on this line, so a 404 on a conditional PUT means a wrong bucket or endpoint, and reporting that as retryable would send clients into a loop against a permanent fault. The three background uploads outside the write guard now publish with IfAbsent. They fire only where the archive is expected absent, and leaving them unconditional would defeat the fence from the side: init uploads an empty bare repo, so a push landing just before it could have its archive replaced by that empty one. A refusal there means someone else already published the key, which is logged as the correct outcome rather than a failure. * fix(node): fence the release publish so an abandoned upload cannot win acquire_write now reads the archive's ETag under the advisory lock and the guard carries it, so release publishes conditionally on the generation it actually refreshed from. An upload abandoned by an earlier writer no longer overwrites a successor's archive: the store rejects it, because the ETag it was written against is gone. A refused precondition gets exactly one supersede-retry, never a loop. The distinction that makes this sound is that a 412 is a definite answer, unlike the timeout arm where nothing is knowable, and the retrying writer still holds the lock, so whatever landed underneath was written without one and its tree is not the authority. Two losses in a row refuse instead of escalating. That retry is what keeps ordinary pushes working now that init publishes create-only: a first push racing init's upload of the empty repo loses once and then wins, rather than surfacing a 503 on the most common operation there is. release returns a must-use outcome and the four publishing handlers propagate it before any trust bump, webhook, or success body, so a publish the store refused reads as a retryable 503 instead of a 201. The three release(false) sites deliberately do not map it: they publish nothing, and a 503 there would shadow the 403 or 404 the route means to return. The download-failure fallback also publishes fenced now. That arm knows the stored generation (its HEAD succeeded, only the GET failed), so publishing unconditionally from it would reintroduce the same overwrite. * test(node): drive the abandoned-upload race end to end, and probe the real backend The headline test is the one that had to exist. Writer A's release is parked past its transfer bound and returns with the outcome unknowable, B acquires and publishes, and A's captured PUT is then replayed: the store answers 412 and B's archive survives. The create-only variant covers the arm whose real-world failure is silent rather than loud. The control is what makes those attributable. With no interleaved B, an abandoned-then-replayed PUT whose generation still matches lands. Without it the headline would only show that replays get rejected, not that staleness is what rejects them. Header assertions come last in all three, so a lost fence reds on the outcome it is about rather than on a wire-format check. A mock cannot prove Tigris honors any of this, and the vendor requires a Single-region or Multi-region bucket for conditional operations, so against a Global or Dual-region bucket the fence is a silent no-op. The credentials-gated probe checks both arms against the real endpoint and cleans up unconditionally, including when an assertion fails, which is the case it exists to catch. It accepts 412 or 409 on the create-only arm because both mean the precondition was enforced and both are already classified as a loss. * fix(node): reconcile F-series tests and test seams with the rebased guard shape The rebase onto main carried main's #174 F-series guard tests forward. The branch's guard replaces main's locked/released/test_pre_unlock_gate Drop mechanics with close-on-drop (runtime) and leak (off-runtime), so two tests that asserted the replaced mechanics are reconciled: the off-runtime disposal test now asserts the leak observable (the slot never returns to idle) and the detached-unlock-returns-connection test is dropped, its invariant covered by the U-series drop-frees-the-lock gates. The pre-unlock gate seam is restored (test-only) so the mid-unlock cancellation tests keep their deterministic park. ipfs.rs tests are updated to the branch's sync test-client constructor and 4-arg RepoStore::new. Refs #279 * fix(node): address the 2026-08-10 review round on the advisory-lock series - P1: serve the receive-pack advertisement from a non-mutating snapshot instead of acquire_fresh, so the unlocked advertisement cannot delete or swap the live repo directory under a concurrent guarded write (the stale-ETag success ordering). acquire_fresh loses its only production caller and is removed. - P1: read-gate, rate-limit, bound, and cancellation-clean the close_issue pre-lock snapshot, so a signed non-owner cannot drive unbounded Tigris downloads and blocking extractions, and an abandoned extraction no longer leaks its temp dir. - P1: classify raw 409/412 responses as a lost conditional write via SdkError::raw_response() in both upload and the publish supersede-retry, so an unparsable error body cannot acknowledge a write that was never published. - P2: wrap the cold-cache under-lock download failure in RepoUnavailable so it sheds as a retryable 503 like the HEAD arm, not a permanent 500. - P2: surface a refused create-only fork upload as PreconditionLost and refuse the fork, so an orphan archive cannot shadow a fork's DB record. Refs #279 * fix(node): record why the close-issue read gate cannot carry a synced row The gate added for the pre-lock snapshot reads a repo row's own visibility rules and public flag. A row synced from a peer is stored public and carries none of the owner's rules, so for that class the gate can only return allow. Refusing instead would deny the repo's real owner and the issue's real author on any node whose only copy is a synced one, and it would not buy the protection it appears to, because a synced row's recorded owner comes from the peer that sent it. Every other read gate in the API reaches the same verdict for such a row, so this is left as is and stated at the call site rather than special-cased here. The test that covered the gate seeds a locally created repo and cannot observe this. Adds one that pins the property directly: the gate allows an arbitrary caller on a synced row, and the handler refuses that caller anyway, because the owner-or-author check is what decides this route. * fix(node): address jatmn review on merged advisory-lock head Align .env.example db pool with validate floor, move snapshot cleanup into the blocking extract task, remove fork clones on PreconditionLost, restore upload_site_reached test plumbing, and use owner DIDs in receive-pack tests after #330 owner-push default. * fix(node): cap close_issue snapshot work on the read pool Take a git_read_semaphore permit before the non-owner author snapshot so parallel close attempts cannot each drive a full archive extraction while still under the hourly rate bucket. Format owner-DID test helpers. * fix(node): mark close_issue mirror visibility gate for push hook Annotate list_visibility_rules with mirror-rows-handled so the pre-push surface detector accepts the synced-row read gate. * fix: unblock pre-push after main merge Read CappedBody.text in gl whoami (#381 shape), restore legacy CID sweep wiring dropped in the merge, route rate-limiter cleanup through AppState, and allow build_lock_pool for test-only callers. * fix(node): carry transient acquire failures through every write handler Extend acquire_write_app_error so RepoBusy and RepoUnavailable map to fixed-body 503s on issue and pull paths, matching receive-pack. Fix the flaky u2 drain exhaustion test via a drain_faults seam and add handler regressions for contention and storage unavailability on create_issue. * fix(node): drop unused logcap helper after drain_faults seam * fix(node): route fork disk paths through validated_repo_disk_path CodeQL flagged remove_dir_all on a user-derived path in fork_repo. Use the same three-layer barrier as RepoStore::local_path and reject names the validator refuses before clone. * fix(node): harden release-upload outcomes and replication tail gating Treat timed-out or failed archive uploads as non-durable in into_result, gate pinata/gossip on confirmed publish durability while keeping inv22 spawn order, split lock-pool vs main DB pool validation, map LockPoolBusy through From<anyhow::Error>, and attach Retry-After on repo write contention errors. * fix(node): close jatmn round on publish authority and durability gating Revoke under-lock refresh swaps when the advisory lock ends, fail closed on unrecorded release outcomes before Pinata/gossip, compensate fork archives when create_repo fails, recompute acquire backoff from the live deadline, and route publish swaps through validated_repo_disk_path for CodeQL. * fix(node): harden issue close/create and fork publish paths on #285 Move close_issue rate limiting after the read gate so denied readers see 404 not 429, roll back local issue refs when publish refuses, re-check swap authority under publish_lock, propagate fork upload failures before DB insert, and update lock-pool config help text. * fix(node): close write-attempt ownership gaps on advisory-lock release Make publish swaps claim-exclusive via CAS, invalidate the local read cache on definite publish refusals, harden fork create compensation and clone cleanup, and keep snapshot temp dirs owned through the async handoff. * fix(node): gate replication tail on publish durability and split close_issue limits Wait for a confirmed release outcome before post_receive_replication_tail runs any walks or coalescing work. Give close_issue its own per-IP rate bucket so it cannot drain receive-pack quota. Add regression tests for both paths. * fix(node): add ValidatedRepoDiskPath for CodeQL and reconcile tail durability Introduce a validated-path newtype with the same inline join and component walk main uses, so path-injection sinks only accept sanitised repo paths. Reconcile the publish-durability gate with the disconnect-safe tail spawn via PublishDurabilitySlot and explicit UploadUnknowable handling. * fix(node): close three jatmn findings on advisory-lock PR Retry fork create confirmation before compensating or returning, schedule background recovery when the lookup stays unavailable, install publish durability outcomes before the recorded flag, and size the lock pool for non-push mutations (default 40). * fix(node): verify adversarial findings on advisory-lock PR Fail closed when publish durability never records, bound pg_advisory_unlock with the same transfer budget as upload, and cap close_issue pre-lock snapshots at git_acquire_timeout_secs instead of the write-lock transfer bound. * fix(node): compensate under the write lock and harden tail durability handoff Run create_issue rollback through release_compensating so issue refs are deleted only while the advisory lock is still held and only on definite publish refusals, not UploadUnknowable. Replace the tail durability slot's async try_lock drop with a std mutex so cancellation during polling cannot skip installing UploadUnknowable. * fix(node): refuse stale under-lock refresh and gate tail durability on release Under-lock GET failures now always shed as RepoUnavailable, even when a local copy exists, so a transient download error cannot publish over a newer stored generation. PublishDurabilitySlot only synthesizes UploadUnknowable after release starts, and receive-pack marks that boundary before spawning the tail. Adds regression tests for both paths; inv22 F4 gate accepts the release wrapper. * style(node): rustfmt advisory-lock gap-closure commits * fix(node): satisfy clippy on stale-local download test path join * feat(node): add a publish attempt/stage boundary The four lifecycle findings on this branch share one root cause: a resource was tracked by a coarse state ("release started", "the path exists", "a row with this name exists") when the safety question was about the IDENTITY and the PUBLICATION STAGE of one specific write attempt. This is the vocabulary that makes those questions answerable: - `PublishAttemptId` — minted before the request is built, carried with the bytes as object user metadata, read back off the store to decide whether what is published is THIS attempt's work. That turns "did my request succeed", which a lost response makes undecidable, into "are the published bytes mine", which the store can answer. - `PublishStage` / `PublishStageCell` — how far one attempt got, observable from OUTSIDE the future doing the work. A cancelled handler never returns an outcome, so the stage it had reached is the only thing that can classify it. - `UploadError` split by what a failure ENTITLES A CALLER TO DO rather than by which HTTP status came back. Destructive compensation is licensed only by `proves_not_published()`. Deliberately free of any object-storage type. #79 deletes `git/tigris.rs` and replaces it with a BlobStore layer; this module is what survives that swap, with each backend supplying only its own error classifier. * feat(node): stamp uploads with an attempt id and preserve dispatch ambiguity `upload` now reports its progress into a `PublishStageCell` and stamps every PUT with an attempt id in object user metadata, so both of the questions a cancelled or unanswered publish raises become answerable: - `PreparingArchive` is marked before the blocking compression and `PutDispatched` immediately before `send()`. The conditional PUT is not constructed until compression returns, so a caller abandoned in that window definitely never attempted publication — a fact nothing could previously observe, because the only report was the return value of a future that no longer existed. - `attempt_landed()` HEADs the key and compares the stored attempt id, which is what lets a client whose response was lost recover its own committed write instead of guessing. `UploadError::Other` is replaced by `NotPublished` (proven never committed) and `Ambiguous` (may have been dispatched and may have committed). The single backend-aware classifier is `classify_put_failure`: a 4xx is an answer the server gave before storing anything and proves non-publication; a 5xx does not, and neither does a timeout, a dispatch failure, or a response the SDK could not read. `SdkError` is `#[non_exhaustive]`, so the fallback arm is the cautious one. `delete` is replaced by `delete_if_attempt_matches`, which reads the attempt off the object and fences the DELETE with `If-Match` on the generation it came from. A second name lookup before an unconditional delete would only narrow the window in which a successor's object can be erased; the conditional delete closes it. * fix(node): key durability, cache validity and fork cleanup on the attempt Closes the four lifecycle findings by consuming the attempt/stage boundary at the sites that were deciding on coarse state. P1 — cancellation before PUT dispatch is not publish durability. `PublishDurabilitySlot` is armed on the guard's publish STAGE rather than on a "release started" flag, and its `Drop` classifies an abandoned handler by how far the attempt actually got: a cancellation during compression records a definite refusal, a cancellation after dispatch records unknowable, and a cancellation after the store acknowledged records `Released` (which the flag could never say). `publish_durability_confirmed` now requires `Released`, so the detached tail no longer does IPFS/Pinata/P2P/GraphQL/Arweave/peer work for refs that exist only in a rejected local write. A guard with no storage backend seeds `NoBackend`, keeping Tigris-less deployments' tails running. P1 — an unknowable generation is quarantined, not served as ordinary cache. `release` writes a sidecar marker naming the unresolved attempt beside the live tree (never inside it: it would be tarred into the next archive), and `acquire`'s existing-path fast path reconciles it before serving. Only the store confirming it holds that attempt lifts the quarantine; anything else is a retryable `RepoUnavailable`. The tree is deliberately NOT deleted — the PUT may have landed and this can be the only local copy. A confirmed publish, an under-lock refresh, and cache invalidation each clear the marker, so the quarantine is a bounded refusal rather than a standing outage. The release-side timeout arm also stops treating every stall alike: a bound that expires at `PreparingArchive` is a definite non-publication, and a bound that expires after dispatch spends one short, separately bounded HEAD trying to reconcile the attempt before falling back to unknowable. P1 — fork confirmation and cleanup bind to the creating attempt. The DB row id is minted up front and used as the attempt id, so the object's metadata, the clone's sidecar stamp and the row all name one attempt. `confirm_fork_repo_row` looks the row up BY ID and reports Ours / Foreign / Absent, so a concurrent create, mirror registration or retry can no longer be returned as this request's own commit. Every destructive step — `compensate_fork_archive`, its background retries, and `ForkCloneGuard::drop` — is conditional on the resource still belonging to this attempt, so recovery that observed `None` before a successor committed can no longer erase that successor's archive or directory afterwards. P2 — response-loss ambiguity is preserved rather than compensated. Guarded writes map an ambiguous publish to `UploadUnknowable` + quarantine instead of `UploadFailed` + cache invalidation + the caller's undo, so `create_issue` no longer deletes an issue ref whose archive is durable. Fork creation reconciles by attempt id: a create-only PUT that committed and lost its response is RECOVERED and the row inserted, and an unresolved one keeps its only local clone and refuses retryably. The fork name is no longer fenced behind the attempt's own orphan. Tests, each RED-checked by reverting its guard: - slot_drop_during_compression_records_a_definite_non_publication - slot_drop_after_dispatch_records_unknowable - slot_drop_after_the_store_acknowledged_records_released - slot_drop_with_no_storage_backend_records_released - publish_durability_confirmed_accepts_only_released - publish_durability_confirmed_refuses_quickly_after_unrecorded_slot_drop - receive_pack_cancelled_during_compression_publishes_nothing_and_runs_no_tail (+ receive_pack_that_completes_its_publish_still_runs_the_tail as control) - a_bound_that_expires_before_dispatch_is_a_definite_failure - an_unresolved_publish_quarantines_the_tree_and_a_later_read_refuses - a_quarantined_tree_is_served_once_the_store_confirms_the_attempt - the_next_write_clears_an_inherited_quarantine - a_confirmed_publish_leaves_the_tree_readable (control) - a_guarded_write_whose_response_is_lost_is_not_compensated - a_put_that_commits_and_loses_its_response_is_ambiguous_and_reconcilable - a_closed_response_with_no_http_status_is_ambiguous - upload_classifies_404_as_a_definite_non_publication - upload_classifies_500_as_ambiguous_not_definite_failure - a_generation_that_moves_between_the_head_and_the_delete_is_not_deleted - an_attempts_own_object_is_deleted_and_a_foreign_one_is_not - fork_confirmation_never_claims_a_concurrent_attempts_row - fork_confirmation_recognizes_this_attempts_own_committed_row - fork_confirmation_reports_absent_when_nothing_owns_the_name - fork_clone_guard_leaves_a_successors_mirror_alone - fork_recovery_resuming_after_a_successor_deletes_neither_object_nor_path - fork_compensation_removes_what_this_attempt_still_owns - fork_publish_that_loses_its_response_stays_recoverable - a_fork_whose_publish_lost_its_response_is_recovered_not_fenced - a_fork_whose_publish_is_unresolved_keeps_its_clone_and_refuses_retryably - an_ordinary_fork_publishes_and_commits (control) - mock_round_trips_the_attempt_metadata_a_put_stamped The S3 mock gains attempt metadata, conditional DELETE, a commit-then-lose- the-response mode, a delivered-but-not-committed mode, and a per-key object store (fork creation touches the source and fork keys in one request, and a single slot would have let an assertion about one silently read the other). The compression seam is a condvar gate so no test holds a guard across an await. * fix(node): settle the cache contract on the abandoned path and the attempt The publication boundary this branch introduced was applied at call sites that each had to be remembered, and five were not. Close the eight gaps that survived, grouped by what actually failed rather than by symptom. The marker's lifecycle was answered by the wrong event in three places. The swap that replaces a live tree now owns the clear, so a cache-miss download no longer leaves a stale marker that fences the archive it just installed, and the under-lock refresh no longer lifts a quarantine on a HEAD that downloaded nothing. init clears a predecessor's marker. A cancelled handler never reached release, so it applied the tail contract and not the cache contract. RepoWriteGuard::drop now settles the tree it abandoned, keyed on how far the publish actually got, and a guard that handed out its path for writing marks the tree whatever stage it reached. A marker naming no attempt is a definite non-publication rather than a permanent refusal: it invalidates and the next read serves the stored generation, so an abandoned first push cannot wedge a repo at 503. Only an unreadable sidecar still fails shut. Both of the store's live-path hand-outs now go through one confirming helper, so read_snapshot can no longer return an unconfirmed tree as an ordinary snapshot. The gate binds ahead of the lazy-migration spawn, because a quarantined tree backfilled to object storage by an IfAbsent PUT would publish exactly the refs the marker exists to withhold. The marker no longer rests on a single fs::write; an unwritable sidecar falls back to an in-memory record that reads consult. Fork cleanup claims ownership by renaming the stamp before it touches the tree, so a successor that claims the same path cannot lose its directory to a predecessor's check-then-unlink, and the directory-absent path no longer unlinks a stamp it does not own. A create-only precondition loss whose reconciliation HEAD fails is refused retryably instead of reported as a permanent name conflict, and its clone is released so the retry can re-clone rather than finding the destination occupied. The pin sweep skips a tree whose publish is unresolved, so provider CIDs are not rewritten from refs that may never become durable. That is the sidecar marker, distinct from the operator-level quarantined row flag. 26 tests, each written and observed failing before its fix. Suite 1211 passed, 0 failed, 1 ignored; fmt, clippy -D warnings and --locked clean. * fix(node): settle the abandoned tree at the disconnect, not at reaper exit The settlement that marks an abandoned write ran from RepoWriteGuard's Drop, and on a client disconnect that guard is held by the Arc clone riding the admission guard into the detached reaper, so it ran only once the process group had been reaped. Reads take no advisory lock and consult only the sidecar, so for the width of that window a concurrent fetch was served the abandoned refs off the live tree with no marker beside it. Confirmed by execution before the fix: no marker on disk, acquire returning the live path, and the abandoned ref still on the tree it served. Only the lock release has to wait for the reaper; the settlement does not. TreeSettlement is a token carrying the path, the publish stage and the hand-out flag, holding the exhaustive stage match that used to live in Drop. The handler takes it out of the guard before the guard is shared with the reaper, so on a disconnect the handler future drops first and the marker lands at the disconnect instant while the reaper still holds the lock. A successful release disarms it. Drop still settles a guard nobody took the token from, so the arms have one home. The regression test reads inside the window and asserts the abandoned refs are not served. It samples pg_locks from an independent session first, so a run where the reaper had already finished reports itself inconclusive rather than passing for the wrong reason. Suite 1212 passed, 0 failed, 1 ignored. The five settlement mutations are load-bearing, including one that reverts this split and reddens the window test. * fix(node): let exactly one classifier settle an abandoned tree Moving settlement into a token the handler holds left two classifiers able to run for one write. release() classifies the tree and only then awaits the unlock, while the handler disarms the token after that await returns, so a cancel in the unlock gap dropped a still-armed token and settled a second time. After a definite refusal that is a quarantine marker written beside the tree release had just deleted, which sends every later read down the refuse-and-reconcile path for a repo whose state was already resolved. Observed before the fix: the tree gone, the marker there. The arm is now an AtomicBool shared between the guard and its token. release stores false at the classification point, above the unlock await, and settle takes the arm with compare_exchange, so whichever classifier gets there first is the only one that acts. tree_settled stays, and now has a test that says why: after the handler takes the settlement the arm is still true, so tree_settled is the only thing stopping the guard's own Drop, which on a disconnect runs in the detached reaper, from settling again at reaper exit. The reap-window test was passing for weaker reasons than it claimed. Its fake git took SIGTERM and let the group die, so the reaper could free the advisory lock before the probe read; it now traps TERM and re-parks so only SIGKILL ends it. It asserts the marker exists at the disconnect instant rather than only mentioning it, and the served-refs half no longer passes when acquire fails for an unrelated reason. Suite 1214 passed, 0 failed, 1 ignored. Nineteen mutations load-bearing, including one that reverts the release-side disarm and one that reverts the settlement split. * test(node): use the test's own pool now that Db::pool is gone --------- Co-authored-by: Kevin Codex <[email protected]>	2026-11-04 21:38:41 +00:00
beardthelionandKevin Codex	c301a68161	fix(node): persist the libp2p identity instead of deriving it from the node DID (#324) ... * feat(node): add a persistent libp2p identity key file Add p2p_key_path (--p2p-key-path / GITLAWB_P2P_KEY, default ~/.gitlawb/p2p.key) with a resolver mirroring resolved_key_path, and load_or_create_p2p_keypair, which generates an Ed25519 keypair on first start, persists it 0600, and loads it thereafter. Mirrors the existing load_or_create_keypair idiom for the node identity PEM. A corrupt or unreadable key file is a hard error naming the path rather than a silent regeneration, so a disk problem cannot quietly rotate the node's network identity. Not yet wired into p2p::start; that follows. * fix(node): load the libp2p identity from the persistent key file p2p::start now takes the Ed25519 keypair loaded by load_or_create_p2p_keypair instead of computing one from the node DID, so a node's network identity is generated once from the OS RNG and kept on disk rather than recomputed from a public value on every start. The node DID parameter is gone from start; the call site loads the key first and continues without p2p if the key file cannot be read, matching how a swarm-start failure is already handled. The gossipsub message_id_fn is untouched and keeps its own hasher. * fix(node): create the p2p key with 0600 at creation and reject a loose one Open the key file with create_new and the mode set at creation, then fsync, instead of writing it and narrowing the mode afterwards. The secret is never on disk under a wider mode, an interrupted start cannot leave it readable, and the exclusive open also refuses a pre-existing entry at the path and makes a concurrent start take the key that landed rather than clobber it. Refuse to load a key file whose mode grants group or other access, and name the observed mode so the operator can fix it. Report an empty key file as empty rather than surfacing a protobuf decode error that blames a missing rsa feature. Pin GITLAWB_P2P_KEY onto the mounted volume in the Docker and fly configs and document it, so the key does not depend on home-directory resolution to land on persistent storage. * fix(node): publish the p2p key atomically and pin its directory Write the key to a scratch file in the same directory and hard-link it onto the final path. The bytes are durable before any name points at them, so a crash cannot leave a partial key that fails to load on the next start and takes the node off the network until someone reads the logs. A concurrent reader can no longer observe a half-written file either, since the final name appears complete or not at all. hard_link rather than rename: rename replaces its destination silently, so refusing to clobber an existing key would depend on a check followed by a separate rename, and a concurrent start can land in that gap. hard_link is atomic and refuses an occupied path, including a symlink, which it does not follow. Create the key directory 0700 and tighten it when an existing one grants group or other access. A 0600 key under a writable directory can still be replaced or unlinked. Tightening rather than refusing to start, because existing installs already have 0755 there and refusing would take p2p down on all of them through a path that only warns. Formatting on the branch is swept up here; it was already failing cargo fmt --check before this change. * style(node): comma instead of a dash in the key-load warning House style avoids em dashes in text we write. The swarm-failure warning beside it predates this branch and is left alone. * fix(node): require the p2p key path to name a directory A bare filename in GITLAWB_P2P_KEY put the key in whatever directory the process started from, and the directory guard was skipped entirely on that path: Path::parent returns Some("") for a bare filename, which the caller filtered out before ever reaching ensure_key_dir. The key file was created 0600 inside a directory that kept whatever mode it already had. Config::validate now rejects a p2p key path that names no directory, so the node says so at boot instead of starting with a key it cannot protect. That placement is the point: an error raised in the p2p start path is logged and stepped over, leaving the node running without p2p and reporting healthy. The check is lexical on the tilde-resolved path. canonicalize would fail on a parent that does not exist yet, which is the shipped ~/.gitlawb default and every container's first boot, and comparing against the working directory would reject /data/p2p.key under the image's WORKDIR, an absolute directory the operator did name. Three sites answered the parent question differently, which is how the gap arose: one filtered the empty case out, one already normalized it, and one opened "" and silently skipped its fsync. They now share key_parent, and Config::validate calls it rather than adding a fourth answer. load_or_create_p2p_keypair also refuses a path naming no directory. That is a backstop behind the config gate, not the gate, so a later caller that skips validation cannot quietly restore the old behaviour. * test(node): run the key-permission probe in its own process The probe zeroes the umask so the assertion means something: under a restrictive ambient umask the bits are masked to 0600 regardless of whether the code pins the mode, and the check passes either way. Zeroing it in the shared test process is the problem. umask is process-global and cargo runs these tests on threads, so any test creating a file in that window inherits 000. Measured before this change: an unrelated concurrent test's file was created 0666. The probe now runs in a child process, where the zeroed umask cannot reach a sibling and dies with the child. The parent is an ordinary test that runs concurrently with everything else. Double-gated with #[ignore] plus an env check so a bare --ignored sweep does not zero the umask in the shared process after all. The parent asserts the child ran exactly one test and that it passed, not just that it exited 0. A libtest filter matching nothing runs zero tests and still exits 0, so without that assertion a renamed fixture would read as a green permission check while asserting nothing. Verified by pointing the filter at a name that does not exist and watching the parent fail. * docs(node): scope the p2p key permission claim and note the rotation The old wording said the key file is "created with owner-only permissions" without qualification, which is only true on Unix: every permission path in p2p/mod.rs is cfg(unix), so on other platforms the file inherits whatever the directory gives it and nothing is enforced. Say what is actually enforced and where. Also document what operators now have to do rather than leaving them to discover it: - GITLAWB_P2P_KEY must name a directory, since a bare filename is refused at startup. - The PeerId rotates once on the first start after upgrading, so a GITLAWB_P2P_BOOTSTRAP multiaddr pinning a peer's old id with a /p2p/<PeerId> suffix needs updating or dropping. Suffix-less addresses and the HTTP seed list are unaffected. - If the node reports tightening a loose key directory, the key that was in it should be treated as possibly exposed and deleted so a fresh one is generated. * fix(review): close a `..` escape from the p2p key-path check Two reviewers found the same hole independently: the check rejected a path naming no directory, but a relative parent that walks back out through `..` named one and still landed in the working directory. `a/../p2p.key` and `./keys/../p2p.key` resolve to the cwd itself and `../p2p.key` resolves above it, so all three put the key exactly where the check exists to keep it out of, and had ensure_key_dir chmod that directory to 0700 on the way. Verified by running the paths through the predicate and printing where each parent lands. The rule is now that a relative key path must name a directory and must not walk back out: at least one Normal component, no ParentDir. `..` inside an absolute path stays accepted, since it cannot depend on where the process started. The predicate moves into names_no_usable_directory next to key_parent, and the config gate and the load_or_create_p2p_keypair backstop both call it, so they cannot drift apart. Also fixes two smaller gaps found in the same pass: - The permission fixture could report "1 passed" while asserting nothing. Its env gate returns early, and an early return is a passing test, so a renamed variable would look green. It now prints a sentinel after its assertions and the parent requires it. Confirmed by pointing the child at a different variable and watching the parent fail. - A GITLAWB_P2P_KEY starting with `~/` is refused when no home directory resolves, instead of creating a literal `~` directory relative to wherever the node happened to start. The backstop had no test, so it has one now, along with a both-directions test for the predicate. That test cleans up after itself: with the guard removed it really does write a key next to the source, which broke a later run once. * fix(review): reject `..` in an absolute p2p key path too The previous commit closed this for relative paths and exempted absolute ones, reasoning that an absolute path cannot depend on the working directory. That is true and it is not the hazard. `key_parent` hands `ensure_key_dir` the lexical parent, so `/data/keys/../p2p.key` chmods `/data` rather than the `keys` directory the path appears to name, and `/data/../p2p.key` run as root would try to tighten `/` to 0700. The exemption also had a test asserting the first of those was fine, so the gap was written down as intended behaviour. `..` is now rejected wherever it appears. An absolute path's root counts as naming a directory, so `/p2p.key` still validates and `/data/keys/p2p.key` is unaffected. Found by a second-model review pass after the in-process reviewers had cleared the relative half. * fix(node): scrub the serialized p2p private key from memory Two buffers held the private key in its protobuf form and dropped without scrubbing: the encoding produced when a new identity is generated, and the file contents read back on every subsequent start. Both are now Zeroizing, matching what gitlawb-core already does for its own key material. Scope worth being honest about: this scrubs our copies of the serialized form, not the libp2p Keypair itself, which owns the secret for the process lifetime and exposes no way to zeroize it. The gain is that the encoded bytes do not outlive the write and the read. zeroize was already in the tree through gitlawb-core, so this promotes it to a direct dependency of gitlawb-node and adds no packages; the lockfile change is the one line recording that. * fix(node): validate the p2p key path before securing its parent Reject filesystem-root parents, directory-valued paths, and symlinks before any directory is created or chmodded. Create missing ancestors at the ambient mode and pin only the nominated key directory to 0700. Refuse symlink reads with O_NOFOLLOW, propagate directory durability sync failures, and document the loose-key rejection path for operators. * fix(node): satisfy clippy single-element-loop on p2p path test * fix(node): tighten the p2p key storage contract before mutation Anchor parent-directory checks on symlink_metadata, reject ~/ escapes and non-regular existing keys with a bounded read, treat directory AlreadyExists as a first-boot race, and skip key-path validation when P2P is disabled. * fix(node): refuse foreign-owned p2p key paths and cap reads on all platforms Check directory and key ownership before chmod or load, walk ancestors for foreign control, bound non-Unix reads to MAX_P2P_KEY_BYTES, and gate unix-only fixture tests without panicking on other targets. * fix(node): hold the p2p key directory open across its trust checks Resolve the key path once, then do every check and mutation through an O_DIRECTORY|O_NOFOLLOW handle on the parent (openat/fstat/fchmod/linkat), so nothing can be swapped between the ownership check, the tighten, and the key open. Key reads go through one bounded, non-blocking loader on every platform; new keys are generated in an unlinked scratch file and published atomically, with concurrent first boots converging on the winner's identity. The storage contract is indexed by two matrix tests: filesystem shapes in p2p (each refusal proven side-effect free against a recursive snapshot) and path spellings plus the p2p-disabled guarantee in config. * fix(node): walk the p2p key ancestor chain descriptor-anchored Replace the path-following metadata walk (foreign_ancestor_error) and the post-walk create_dir_all gap with a single descriptor-anchored walk that verifies and creates the chain together. Missing components are created at 0700 relative to the verified parent and re-verified; AlreadyExists is a race the winner must pass. Non-unix builds keep create_dir_all. * fix(node): refuse group-writable and non-directory p2p key ancestors The ancestor walk now refuses any component with group or world write bits unless sticky, and any intermediate component that is a symlink or another object type. Tests drive the walk through real boots: 0770/0775 ancestors refused before any key IO, 0755 and 1777-sticky ancestors accepted with stable reload, and an intermediate symlink refused with its target untouched. Both guards proven load-bearing by neuter revert-checks. * fix(node): verify the cwd anchor for relative p2p key paths A bare-relative key path is now walked from a verified working directory: the cwd is opened as '.' with O_NOFOLLOW|O_DIRECTORY, fstat-verified for ownership and write bits before any relative component is touched, and a writable or foreign-owned cwd is refused before anything is created. The refusal names the working directory path. Child-process fixtures prove both directions without mutating the shared process cwd. * test(node): stop rendering uids in p2p assert-message strings CodeQL flags the uid-interpolating assert messages as cleartext logging to a console sink. Every assert message in the p2p test module now avoids interpolating uid values or the uid-bearing error text, while the substantive assertions (the refusal names both uids, the ownership check fires before the chmod) stay intact and are verified by the same assertions. * test(node): add walk rows to the p2p key-storage contract matrix The matrix now proves the ancestor-walk refusals table-driven: a group-writable (0775) ancestor, a world-writable non-sticky (0777) ancestor, and an intermediate symlink component are each refused with the no-mutation snapshot discipline, while the existing boot rows still reload to a stable PeerId. The fchmod-on-create path pins newly created ancestors to 0700 regardless of the ambient umask. * docs(node): note the verified working directory for relative p2p key paths README, .env.example, and the clap help for GITLAWB_P2P_KEY now state that a relative key path is anchored at a verified working directory, and that an unsafe cwd or ancestor is refused with p2p off while HTTP stays up. Also normalizes two pre-existing em dashes in the README settings table. * fix(node): pin and verify the mode of every key object this process creates POSIX applies the process umask to every requested creation mode, so the 0700 passed to mkdirat and the 0600 passed to openat were requests rather than results. Under a mask that strips owner bits the created directory landed 0000 and the no-follow reopen failed EACCES before the repairing fchmod was reached, so that half failed loudly. The scratch key was worse: it was published unreadable, the boot that created it succeeded on its already-open descriptor, and only the next boot lost p2p, behind a health check that still reported healthy. Every object this process creates now goes through one rule: create, pin the mode on the object just created, reopen, and verify the achieved mode by fstat. The directory pin is issued by name off the verified parent before the reopen, because a directory that landed 0000 cannot be opened at all, so a pin that waits for the reopen is unreachable in exactly the case it exists for. A race winner is verified as-is and never chmodded, which is the rule the ancestor walk already applied. Pinned's field is private to the pin module and no constructor is exported, so a descriptor that skipped the pin cannot be turned into a key-directory handle or reach publication. That is what keeps the rule from depending on review. Two adjacent corrections the same defect exposed. The leaf mode predicate was supersets-only: mode & 0o077 != 0 asks whether anything is granted beyond the owner, so 0000, 0100 and 0400 all passed it while being unusable, and an inherited setgid bit was judged on the wrong bit width. A loose directory is still tightened; an over-closed one is now refused with its remedy rather than widened, matching how an over-closed key file is already handled. And an unreadable key or key directory now names its own cause: every open failure previously reported a refused symlink, which is what an operator saw for a key masked at creation. The matrix runs first boot and fresh-process reload across umask 0000, 0022 and 0777, crossed with missing and existing ancestors and leaf, with concurrent creators and an injected write failure. It runs in child processes because umask is process global, and it requires at least one successful concurrent creator so a row where every creator fails cannot pass. All 21 rows were RED under umask 0777 before this change. * fix(node): create and publish the node identity key through the pinned path The node identity PEM had its own storage flow: create_dir_all with no mode, then write, then set_permissions. That is the exists-then-write-then- chmod sequence INV-23 prohibits, and it sits in the same ~/.gitlawb the p2p key uses, so the umask defect the previous commit fixed for one key was still live for the other. Measured on the current code, as a non-root uid, all three RED: umask 0000 directory landed 0777, world-writable, holding the key umask 0022 directory landed 0755, world-traversable umask 0777 directory landed 0000 and the write failed with EACCES The third is the one that mattered most: load_or_create_keypair runs before the listener binds, so the node exited there and no p2p code was reached at all. The umask-independence guarantee could not be demonstrated on the shipped default while this stood. The directory is now created through the same pin helper, at a verified 0700, and the PEM is published through the same scratch-then-link path at a verified 0600. An existing directory that grants access beyond the owner is tightened, which closes the other half of the same gap: a 0600 key inside a 0755 directory is still replaceable by anyone who can write that directory. An existing key is loaded untouched and never chmodded. Deliberately not the full ensure_key_dir. That carries the ancestor trust walk, and importing its refusals onto a path that never had them would turn an unsafe but currently booting deployment into a boot failure on upgrade. Only the immediate parent goes through the pin helper, which verifies the grandparent it is about to chmod a child of; ancestors above that keep the existing create_dir_all behavior. The one new refusal is a group or world writable non-sticky grandparent, which is the case where another local user can replace the node's identity outright. * fix(node): split p2p key config validation from live storage validation The key-path validator answered two different questions and its callers disagreed about what to do with the answer. Bare names, `..`, a trailing separator and the filesystem root are properties of the configured value, and Config::validate refuses them before the listener binds. The same function also stat'd the key path and its parent, so a symlinked parent, a non-directory parent or an unreadable parent exited the node as invalid configuration, while the identical class of fault found one layer later in load_or_create_p2p_keypair only logged and left HTTP serving. That split was invisible from the outside and the docs described only one half of it. Measured against the binary before this change: a symlinked parent, a regular-file parent and an unreadable parent all exited 1 before bind, which is the opposite of what README and .env.example promise. Validation is now lexical only, behind a P2pKeyConfigError so the two domains cannot be confused at a call site. Every live storage fact is left to the load path, which already re-establishes each one on a descriptor it opened rather than a pathname it stat'd: O_NOFOLLOW on the key open, a regular-file check by fstat, and ELOOP or ENOTDIR at the leaf. The verdicts are unchanged; what changes is that one policy now decides all of them. Two supporting changes. The whole p2p port gate moves ahead of the database connect, both arms together, because connect_db_with_retry retries forever and left the disabled arm unreachable without a database; only p2p::start still needs the pool. And a failed key load is now logged at error with a stable event name and mirrored into a gauge, because the policy this commit settles on is to keep serving HTTP with a green health check while the node is off the p2p network, and that is only defensible if the outage is visible to something other than a human reading startup logs. A new integration test drives the real binary with no database and proves both domains at the process boundary: nine lexical spellings exit before binding, ten storage faults serve HTTP with the failure logged, and GITLAWB_P2P_PORT=0 reaches its disabled log without touching the key tree. Every row asserts a before/after snapshot, because a refusal that mutates storage on its way out is its own defect and a returned error cannot show it. Three of those rows were RED before this change. A symlinked key directory also reports as a symlink again. Linux returns ENOTDIR rather than ELOOP when O_NOFOLLOW meets O_DIRECTORY, so the errno alone cannot separate a symlink from a regular file and the message had regressed to naming the wrong cause. * docs(node): state the two-domain key-path policy and how to see a degrade README and .env.example promised that p2p stays off while HTTP keeps serving when the key cannot be loaded, including for an unsafe ancestor or working directory. Measured against the binary, that was false for a symlinked parent, a non-directory parent and an unreadable parent, each of which exited before binding. The preceding commit made the code match the promise; this makes the promise precise. All three operator surfaces now name the split the same way. A value that cannot name a securable key file is refused before the node binds. Anything wrong with the storage itself degrades, and the sentence says how to notice: the node logs p2p_identity_key_load_failed at error and sets the matching metric, /health stays green in that state, and fixing the storage and restarting restores the same PeerId. A node quietly off the network is the cost this policy accepts, so the docs point at the signal rather than leaving an operator to find it in startup logs. Two corrections while here. The key directory is tightened only when it grants access beyond its owner; one closed too far is refused with a chmod 700 remedy rather than widened, so the sentence no longer implies the node will open up a directory an operator locked down. And the existing advice to delete a key after a tightening warning is now scoped to the loosening case it was written for: an over-closed directory is refused, not tightened, and nothing in it was ever exposed, so following that advice would have meant deleting a safe key and taking a second PeerId rotation that breaks every pinned bootstrap multiaddr. * test(node): pin the degrade signal and wait for both startup lines Two gaps in the boundary test, both found by extending the mutation spec to cover this commit's own guards. The degrade rows asserted only the prose message, so renaming the event field or dropping the level would have passed while every alert built on the operator docs broke and the node still looked healthy. The rows now pin the error level and the p2p_identity_key_load_failed name the docs tell operators to watch. The rows also waited on a single log line, which is a race: the degraded server logs "ready" from a spawned task while the p2p gate logs its verdict from the main task, so whichever the reader stopped at first left the other uncaptured. It passed when the test binary ran directly and when the row ran alone, and failed only once other work shifted the scheduling. Reads now wait for every needle before returning. The fix stays on the test side on purpose. Logging the p2p verdict before the degraded server is spawned would also make the order deterministic, but that is production sequencing changed for a test's convenience. * fix(node): keep key publication transactional and path-policy split Bare GITLAWB_KEY filenames publish into the working directory without chmodding it. A directory this process created is removed if pin or verify fails. Ancestor walks use search descriptors so a safe 0111 component is traversable. Scratch unlink is part of publish success and is fsynced. Rotation advice is limited to writable or readable exposure, not ordinary 0755 tightening. * fix(node): refuse a writable cwd for a bare identity key Publishing identity.pem into cwd must not chmod cwd, but a 0600 key is still replaceable if that directory is group or world writable. Apply the same trusted-parent check used for nominated key directories before the handle is returned. * fix(node): load identity without following pathnames Create already published through a directory handle. Load still used exists-then-read, which followed a symlink at the key and opened the grandparent by pathname. Open both no-follow, treat /identity.pem like a cwd publish so it reaches the filesystem, and open a search-only grandparent with the walk flags. * fix(node): parse key paths, leave identity parents, repair special modes A terminal `.` in GITLAWB_P2P_KEY was accepted then retargeted by Path onto the parent directory. Identity creation imported p2p's 0700 pin onto any existing named parent. A setgid-only 2700 key directory skipped repair and disabled P2P. Darwin ancestor walks still required directory-list permission. * fix(node): type-check the non-unix p2p scratch writer The shared scratch-then-link path names pin::Pinned on every target, but the pin module was unix-only, so gitlawb-node did not compile off Unix. * fix(node): refuse identity path spellings that Path would retarget GITLAWB_KEY had the same terminal-dot hole as GITLAWB_P2P_KEY, and its tilde expansion did not refuse a suffix that escaped home. Bare filenames stay legal. * fix(node): create every missing identity parent on one pinned walk The identity key path split one creation transition across two policies. `create_dir_all` built every ancestor above the immediate parent at the ambient umask, then `create_dir_pinned_at` ran `verify_trusted_parent` on the last of those and refused it. Under umask 0002 that ancestor lands 0775, so the phase that made it handed it to the phase that rejects it: the node exits before binding and the failed boot leaves the directory behind at 0775, so the next attempt starts from state the first one made. Replace the split with one descriptor-anchored walk. Pass 1 walks up from the key's parent until a component opens; that descriptor is the anchor, and it is the same fd the next step creates into. Pass 2 walks back down calling `create_dir_pinned_at` per component, each one pinned 0700 and verified at its achieved mode before it becomes the parent of the next, so from the anchor down no pathname is resolved twice. Pre-existing ancestors are adopted as-is and never chmodded, since GITLAWB_KEY is a file path rather than a dedicated-directory setting; write-authority is still required on the directory each component is created in. A boot that fails past the walk now removes exactly what it created, deepest first, through unlinkat with AT_REMOVEDIR against the descriptor each entry was made in. A race-adopted directory is never in that list, and AT_REMOVEDIR refuses a non-empty directory, so a leaf another boot has published into survives. The accumulator is an out-parameter so a mid-walk failure and a post-walk failure reach the same single rollback arm. Two consequences worth naming. A symlink at the deepest existing ancestor is refused when the missing suffix reaches it, where `create_dir_all` followed it. Ancestors ABOVE that anchor are still resolved by pathname and their symlinks still followed, unchanged from before this walk, because O_NOFOLLOW binds only the final component of a path-based open; closing that would mean an openat chain from the root and the full ancestor policy that GITLAWB_KEY paths deliberately do not get. The doc comment now says so rather than claiming more than the code does. And a losing concurrent first boot can remove an intermediate another boot adopted but has not yet written into, turning that boot's start into an ENOENT failure; both still fail closed. Also replace the root-adjacent test. It proved a pathname property by running the full mutating path against the real filesystem root and assuming the runner lacks permission. Under euid 0, common in build containers, `/` is accepted (root-owned, 0755, no group or other write bit), the production path returns Ok, the test's panic arm fires before its cleanup assertion, and a real private key is left at /identity.pem, after which the leading exists() check makes every later run a silent skip. The routing is now asserted lexically through a named predicate, with no filesystem access and no dependence on euid. The matrix covers one, two and three missing levels under umasks 0000, 0002, 0022 and 0777, a pre-existing 0755 ancestor that must stay 0755, a relative multi-level path, a symlinked deepest ancestor, refusal without partial mutation, and a populated leaf whose removal must be refused and reported. Created-directory modes are asserted by exact equality, and each success row reloads in a fresh process and requires the same DID. * test(node): race real concurrent first boots of the identity key The rollback added with the pinned walk can remove an intermediate that a concurrent boot has already adopted, and until now that was reasoned about rather than executed. Nothing in the suite started two real first boots against the same key path. Add a `concurrent-first-boot` layout to the self-exec fixture and a driver that races several children through `load_or_create_keypair_at` on one missing multi-level path. A two-phase file rendezvous puts every child inside a spin loop before any of them is released, so the spread across the racing call is one loop iteration rather than process-startup jitter. Three arms: unaided boots, one injected write failure against one unaided boot, and four injected failures against one, which is what actually opens the window. The assertions are properties of every legal outcome, never one interleaving, so the test is deterministic even though the race is not. A boot that claims success has a parseable 0600 key on disk, all successes agree on the DID, surviving directories are exactly 0700 with no scratch residue, and if no key was published then nothing claimed success and a retry must still work. Observation counters are printed, not asserted, so a run that never hits the window says so rather than passing as proof. Two things the race showed that reading could not. The documented window is real: a boot was refused with "No such file or directory" on its publish after a loser rolled back the leaf it had adopted. But the more common shape is one hop earlier, the loser removing an adopted intermediate so the ENOENT lands on the next component's creation instead. And under enough concurrent rollbacks every boot can fail, not merely the losers, so several nodes starting against a shared first-boot path can all need a restart. Two-child racing never opened the window at all in 500 iterations. * fix(node): resolve identity key path components one at a time `O_NOFOLLOW` binds only the final component of a path-based `open(2)`, which is all the kernel offers for a multi-component pathname. Three sites resolved a whole configured path in one open and so guarded only the last position: the load path, the existing-named-parent fast path, and the walk that creates missing parents. Every interior symlink was followed, and `verify_trusted_parent` then attested the directory that was reached rather than the path taken to it. That is enough to substitute the node's whole identity, not merely misplace its key. Load and create resolve the configured pathname identically, so repointing one interior link redirects both. Point it at a directory holding another PEM and the node boots as that DID while its real key sits untouched on disk and nothing is logged as wrong. Point it at an empty directory and the node mints a fresh identity and reports a first boot. Neither needs the attacker to read anything: ownership and mode checks still pass, because they run on the object at the end of the path. Resolve each component with `openat` and `O_NOFOLLOW` against the descriptor of the one before it, from `/` for an absolute path or the working directory for a relative one, so a symlink is refused at every position. Interior components open search-only and the final component keeps the caller's flags, so a legitimate 0111 ancestor still resolves and the leaf descriptor is still the one that gets fsynced and chmodded. Components are adopted with no ownership or mode judgment. The only new refusal is a symlink on the key path: a shared 0775 volume, a foreign-owned ancestor and `/etc` all keep working, because `GITLAWB_KEY` is a file path rather than a dedicated-directory setting. `verify_trusted_parent` stays exactly where it was, on the descriptor a child is created in. The p2p key path is untouched; it already walked component by component. This is pre-existing rather than a regression: the same probe against the branch point produces the same result. It is fixed here because closing it replaces the walk this branch just added, so splitting it would mean reviewing that code twice. Covered by rows for an interior symlink with the next level present, an absolute target escaping the base, a two-link chain, the fast path, a relative configured path, and both load-path variants asserting on the DID the node ends up presenting rather than on a mode or an error string. The cases that were already refused are pinned too, including a symlink at the immediate parent and a dangling link, neither of which had a test before. --------- Co-authored-by: Kevin Codex <[email protected]>	2026-11-04 17:02:15 +00:00
beardthelion	4985e38af2	fix(ipfs): page the legacy CID scan instead of materializing every repo ... GITLAWB_IPFS_MAX_LEGACY_PROBES bounded the probe fan-out, but nothing bounded the load that ran before the first probe. On a legacy (NULL-provenance) or capped/incomplete provenance set, get_by_cid called list_all_repos, cloned every id, fetched visibility rules for the whole set, and fetched the node's entire quarantine list, then retained all three for the request. /ipfs/{cid} is anon-reachable and CIDs are enumerable from the unsigned pins index, and an object pinned from MAX_PIN_SOURCES repos is permanently at cap, so every later anonymous GET for it paid that whole preload with the scarce walk permits held, even at a probe budget of 1. The selection is now keyset-paged on (created_at, id) ASC, with the quarantine flag returned per row so the separate whole-node quarantine query is gone. Both key columns are immutable, so paging is exact: a repo touched mid-scan cannot cross a page boundary and go unvisited, and updated_at is attacker-bumpable, which would have let a caller sort their own repos ahead of the true holder. The pager lives outside the oid loop, so its cursor and its accounting are per request rather than per candidate. Before fetching another page the scan checks that a row on it could still reach a verdict; a spent probe or visit budget stops the read and TAINTS, so a truncated search still sheds the retryable 503 and never a false 404. A page of pure denials is deliberately not a stop condition, since a cheap denial costs no probe and a public object buried behind private repos must still be reached. Both page queries stay clamped to the remaining request budget, and the rules query still fails closed. A new v25 migration adds idx_repos_created_at_id. Bounding what the handler materializes is only half the property: repos carried no index in the new keyset order, so Postgres seq-scanned the table and top-N sorted it to return every page while the walk admission was held, which moves the same O(rows) cost onto the database rather than removing it. Measured on 50k rows: 951 shared buffers and ~44ms per page without the index, versus an Index Only Scan at 7 buffers and ~0.09ms with it, and the keyset predicate pushed down as an Index Cond instead of filtering after a scan. Nothing names the index in any query text, so its entry says why it exists and a test asserts its presence structurally. Two order-dependent tests were reseeded rather than left green: the buried-row and acquire-taint cases encoded their ordering through updated_at, which no longer determines iteration. Both were re-confirmed RED under their documented mutations after the reseed. Three regressions land with the fix. One drives a one-probe, one-row-per-page request against three candidate repos and asserts it materializes one row and runs one query, which a query counter alone could not have caught. One seeds two distinct source-less oids under a single CID and pins the per-request property directly: the second candidate re-reads the pages the first paid for, so the whole request costs one pass rather than one per candidate. The third covers the index, including the INV-7 upgrade path. Refs Gitlawb/node#173.	2026-08-11 21:40:19 -05:00
beardthelion	aeb6895141	test(node): stop the repos.rs structural gates splitting at the wrong marker ... Three gates scanned the "production half" of api/repos.rs by splitting at the first cfg(test) attribute. Adding test-only items above the code they check moved that split point above every line being scanned, so all three failed looking for production code that was still right there. They now split at the test module, which is what they meant. This is the same defect already fixed in the ipfs gate during the merge, and no split-on-the- attribute site remains. Both behavior-bearing gates were re-proven load-bearing: F2 goes red when the Pinata object set is re-derived before the pin permit is taken, and the tail gate goes red when release stops consuming the same success flag. F3 is a presence check whose marker cannot be removed by a single compiling edit, so it is not mutation-proven here.	2026-08-02 20:16:58 -05:00
beardthelion	e5c1da2bc6	Merge #174 into the CID tree-gate branch ... Reconciles the served-git concurrency cap with the /ipfs CID tree gate. The two branches independently reworked the same three seams, so most of this is picking one mechanism per seam rather than taking a side per file. /ipfs walk admission: keeps this branch's provenance-first resolver and its lazy legacy scan, but drops the detached serve task in favour of #174's shared Arc<WalkAdmission>, cloned into each spawn_blocking. Both close the permit-release bypass; the Arc does it without leaving an abandoned request's full legacy scan running against a held slot. Ported in with it: the request-budget gate at all four stages with each child deadline clamped to the remainder, the walk timeout computed inside the closure so queue delay is charged, taint sources in place of a single truncated flag, and the deterministic-fault 500 gated on nothing else having tainted. The two per-request ceilings are now composed rather than one shadowing the other. The visit ceiling was unwired by the merge and the walk cap answered to only one of the two knobs; both are honoured, the tighter one winning. Object probe: object_type_bounded takes a Duration and returns ProbeError, so this branch's call shape keeps #174's absence-vs-unreadable-store discriminator. A clean `missing` from cat-file is byte-identical whether the object is absent or the pack is unreadable, and the discriminator is store readability, not git's wording. Write lock: the advisory-lock pool's after_release hook does not cover an unlock that errors on a live session. Measured: a poisoned connection returned to the pool still held the lock 15s later, because the hook's own unlock_all fails the same way. So the guard now disposes of that connection, in release and in the Drop backstop. Post-push replication goes to #174's coalescer. This branch's requeue_faults integration suite has no equivalent seam on that design and is not carried over; it is recoverable from the pre-merge head. Also clamps the provenance path's three per-source lookups to the request budget. They run while the walk permits are held, which is the stall hazard the preload clamp already covered, and the structural guard found them because it now requires every occurrence to be wrapped instead of assuming exactly one. 1346 tests pass across the workspace; fmt and clippy are clean under --locked. The four re-anchored structural guards were mutation-tested and all four go red on the defect they name.	2026-08-02 16:09:57 -05:00
beardthelion	3ced62fab0	fix(node): spawn the replication tail above guard.release() (#174) ... The post-receive tail was spawned after `guard.release(...)`, which on a successful push awaits the Tigris upload and then the advisory unlock while the handler future is still tied to the client connection. The pack has already landed on disk by then, so a disconnect in that window left a durable push whose pins, recovery copy and announce were never spawned: the dropped tail F2 closed, one step earlier in the handler. Spawn it above `release`, gated on an explicit `receive_result.is_ok()`. The `?` below the spawn can no longer be what keeps a rejected push from pinning and announcing a half-applied repo, so the success check is now a named flag that `release` consumes too. The tail is read-only on the repo directory and holds neither the lease nor the advisory lock, so overlapping it with the upload waits on nothing the handler still owns. Tests: a handler-level disconnect driven through a parked `release` (new store-level test seam), plus the must-not direction, a failed receive-pack that spawns no tail. The U5 source gate now binds the new boundary: spawn inside `if push_succeeded`, above `guard.release(push_succeeded)`.	2026-07-29 12:37:39 -05:00
beardthelion	ade53cbf47	fix(node): spawn the replication tail at the durability boundary ... The post-receive replication tail was spawned at the end of git_receive_pack, after touch_repo, the trust-score update, per-ref certificate issuance and webhook fan-out. A client or proxy disconnect anywhere in that window dropped the tail, and with it this push's pins, recovery copy, and announcements - even though the push was already durable on disk. Spawn it immediately after the receive_result ? instead. That is the first point where the push is known to have landed: guard.release() a few lines up runs on failure too, so anchoring there would spawn a tail for a push git rejected, pinning and announcing a half-applied repo - the state release() deliberately refuses to upload to Tigris, and one a hostile pusher can produce on demand by aborting a pack mid-transfer. RefUpdate gains Clone so the tail takes the parsed updates while the certificate and webhook loops keep iterating their own copy. This makes the tail run concurrently with certificate issuance rather than after it, so a ref can be announced before its signed certificate exists. Accepted, and stated here rather than left for a reviewer to find: cert issuance already fails open (errors are logged and skipped) and the gossip event carries cert_id: None regardless, so no announce consumer reads a certificate out of it. Evidence is an ordering gate in inv22_gates.rs rather than a cancellation-race test, matching the F3 gate's instrument: forcing a disconnect to land inside the cert/webhook window is timing-dependent and would be flaky, while the ordering is precisely what changed. The gate is mutation-verified in both directions - moving the spawn back below the webhooks fails the upper bound, hoisting it above the ? fails the lower one.	2026-07-29 03:04:30 -05:00
beardthelion	7d32a79e96	fix(node): hold /ipfs walk admission through the blocking walk, not the handler future ... The global and per-source walk permits were handler locals, so a client disconnect released them the moment the future dropped while the uncancellable spawn_blocking probe/walk/read kept running against a slot the pool now considered free. A disconnect flood could admit replacements while prior blocking git children still occupied threads and PIDs, weakening GITLAWB_MAX_CONCURRENT_IPFS_WALKS and GITLAWB_IPFS_WALK_PER_SOURCE on a surface any unauthenticated caller can reach. Hold the pair in an Arc and clone it into each of the three blocking closures. The permits release when the last clone drops, which collapses both failure directions into one case: a disconnect leaves the abandoned closure holding the slot, and a panicking closure leaves the handler holding it. No loop arm changes and the JoinError arms keep today's taint-and-continue. A move-and-return shuttle was implemented and rejected. It is correct only if every arm that continues the loop re-binds the returned value, which the compiler cannot enforce - the absent-object arm Ok(Ok(None)) => continue, taken on nearly every iteration of a random-CID scan, drops it - and it forces a second decision about the panic arms whose least-bad answer would let one broken repo shed every other owner's /ipfs lookups. The regression strengthens the existing admission test: it now asserts the slot is still held after the future is dropped and before the child is killed, with a child-still-alive precondition so it cannot pass vacuously. Only the walk site runs under state.git_bin and can be pinned that way; the probe and read deliberately shell to real git, so a source-scan gate in inv22_gates.rs binds all three clone sites instead. Removing any one clone turns that gate red.	2026-07-29 02:46:51 -05:00
beardthelion	b881447b59	fix(node): own /ipfs admission for the life of the walk ... get_by_cid held the global walk permit and the per-source permit as handler locals, so dropping the request future released both while the spawn_blocking probe, visibility walk, and object read kept running; a cancel-spam or timeout client could exceed MAX_CONCURRENT_IPFS_WALKS and the per-source cap, and the bare cat-file probe/read children had no teardown at all. Move the gated serve pipeline into a detached task that owns an AdmissionGuard, awaited by the handler, so admission releases only after the bounded work is gone; back the /ipfs probe and read with run_bounded_git twins for process-group teardown. Adds cancel-mid-walk, timeout-reap, and cancel-spam regressions plus two load-bearing INV-22 rows. Also reflows one U1 test line to satisfy cargo fmt.	2026-07-20 18:34:09 -05:00
beardthelion	de2ad5a83d	fix(node): hold read admission through the filtered-pack reaper ... The path-scoped upload-pack branch kept the read and per-caller permits as handler locals and called upload_pack_excluding with no AdmissionGuard, so a cancelled filtered clone released admission while KillGroupOnDrop was still reaping the git group, bypassing the read and per-caller concurrency caps. drive_git_child now returns the disarmed guard on success so one guard rides both build_filtered_pack stages (rev-list then pack-objects); the handler builds the guard from the permits as the plain path does. Adds a disconnect regression and an INV-22 gate row, both proven load-bearing.	2026-07-20 17:54:49 -05:00
beardthelion	9f4a8c7d2f	fix(node): serialize a disconnected push's repo until its group is reaped (#174 F3) ... On client disconnect, RepoWriteGuard::Drop released the per-repo advisory lock immediately (it holds no child/pgid), while KillGroupOnDrop's detached reaper was still tearing down the git process group for ~4s. A second same-node push could then acquire the repo and race the first's still-writing objects/ dir and Tigris upload. Add an in-process per-repo write lease (block-and-wait, keyed like the advisory lock) that SUPPLEMENTS the retained cluster-wide PG lock. The lease rides into the disconnect reaper via a clone on the write-path AdmissionGuard (None on all read paths) and a second clone held across guard.release() for the clean-path Tigris upload, so it frees only after the group is reaped. A bounded-wait steal reclaims a leaked lease (block-and-wait has no coalesce degradation). The lease is always taken before the PG lock and released after, so the two serializers cannot invert. Deterministic fake-git race test + inv22 gate row.	2026-07-20 10:58:37 -05:00
beardthelion	b897e27cf2	fix(node): bound post-receive Pinata replication memory (#174 F2) ... The detached Pinata task moved each push's full object-id list into the closure and parked it on pin_semaphore.acquire_owned(); a tokio semaphore bounds active holders, not the waiter queue, so a slow Pinata backend let N queued pushes each retain an MB-scale OID list (unbounded memory/tasks). Capture only the small ref-update tuples and re-derive the object set via the existing reaped/deadline-bounded git helpers AFTER the pin permit is acquired, so retained memory is O(ref tuples), not O(object lists). Per-ref effects are unchanged (still one spawn per push, no coalescing or shedding, so no dropped announcements). Adds an inv22 gate row forbidding a retained object list across the acquire, plus re-derivation-equivalence and reaping tests.	2026-07-20 10:19:50 -05:00
beardthelion	e866584784	fix(node): clamp the initial /ipfs metadata queries to the request budget (#174 F6) ... list_all_repos and list_visibility_rules_for_repos ran as bare awaits while holding the scarce IPFS walk permits, so a query blocked in Postgres pinned the slots past GITLAWB_IPFS_REQUEST_BUDGET_SECS and shed later requests as 503s. Wrap both in the remaining request-deadline budget; on timeout the RAII permits drop and the handler sheds a retryable budget 503. Fails closed: a timeout on the visibility-rules query denies the request rather than serving an unfiltered listing. Appends an inv22 gate row asserting both queries stay deadline-wrapped.	2026-07-20 10:00:40 -05:00
beardthelion	671cee184f	fix(node): make RepoWriteGuard::release cancellation-safe (#174 F4) ... Await pg_advisory_unlock while the pooled connection is still owned by self; take it and set released only after the await resolves. A cancel mid-unlock now leaves the Drop backstop armed (conn still Some), so the session-level advisory lock is no longer leaked onto a pooled connection. Adds a cancel-mid-unlock regression test (via a test-only pre-unlock gate) and an append-only inv22 gate row asserting the take/released orderings.	2026-07-20 09:14:01 -05:00
beardthelion	ec36e7f2d7	fix(review): close the round's review findings on the #174 fix series ... object_type now distinguishes repo-level git failures (not-a-repository, error:-prefixed corruption) from genuine object absence, so a corrupt repo taints the /ipfs scan into the retryable 503 instead of vouching for a definitive 404; the drain reads visibility rules by the freshly fetched record id, closing a latent fail-open on id divergence. The four budget gates collapse into one stage-labelled helper, the three scan sites share state::acquire_scan_permit (inv22 tripwire repointed and sever-verified), and the Tigris stall tests use a local accept-and-park listener instead of a non-routable address.	2026-07-17 20:16:04 -05:00
beardthelion	79910fc169	fix(node): requeue coalesced post-push work via an in-lock pending slot and loop-drain (#174) ... try_begin now merges a losing push's tip pairs into the in-flight key's pending slot in the same critical section as the presence check, bounded at 1024 pairs with a full-scan marker on overflow (force_full_scan threads to the resolver; empty tips never encode the marker). The task loop-drains: finish_or_take_pending either hands back the batch with the key retained, or removes the key and disarms the guard atomically, so a successor task's key survives the guard drop. Drains re-fetch rules fresh and replay the tail pipeline under the U4 helper gates only; the loop holds no task-level pool permit, so a hot repo cannot deadlock the pool at size 1.	2026-07-17 19:17:17 -05:00
beardthelion	2259a248ce	fix(node): admit the post-receive git scans to the encrypt pool, defer-not-shed (#174) ... replication_withheld_set's walk arm, resolve_candidates_for_push's git stages, and fail_closed_full_scan_objects each take a git_encrypt_semaphore permit before their spawn_blocking and carry it inside the closure, so a push burst holds at most pool-many concurrent scans and a started scan finishes with its permit. Deletion-only pushes and repos without path-scoped rules never park. Contention defers the landed push's tail (logged with queue_wait_ms), never errors it; the acquire sites document the accepted disconnect-during-park residual.	2026-07-17 18:37:40 -05:00
beardthelion	98ebb47216	fix(node): bound the post-push encryption task set by per-repo coalescing without dropping recovery copies (#174) ... After a successful path-scoped push the handler spawns a DETACHED task that runs pin_new_objects then withheld_recipients_gated, which acquire_owned().await's on git_encrypt_semaphore — it DEFERS (blocks) when the pool is full. The semaphore caps active walks but nothing capped how many detached tasks were spawned and PARKED on that await: N rapid path-scoped pushes spawned N tasks, each holding cloned object lists/rules/paths/keys — an unbounded parked-waiter set (P2-2). 2a54c15 deliberately chose defer-not-shed because dropping the walk loses the withheld-blob recovery copy and there is no reconciliation sweep to rebuild it. So bound the OUTSTANDING-TASK set by per-repo coalescing rather than shedding: EncryptInflight (a bounded Arc<Mutex<HashSet<repo_id>>>) + try_begin — before the spawn, if a task for the repo is already in-flight skip the duplicate (the pending walk covers the newer objects); otherwise spawn and move an RAII EncryptInflightGuard into the task that removes the key on drop (completion, error, or panic-unwind). This bounds the set to <=1 pending task per repo and never drops work; withheld_recipients_gated's defer is unchanged. The second detached post-push spawn (Pinata replication, also runs pin_new_objects) is scoped out with rationale: it parks on no semaphore (no unbounded waiter set) and does per-push per-ref work (branch->CID, gossip, subscription, Arweave, peer-notify) keyed to this push's ref_updates — coalescing it would DROP a later push's announcements, a correctness regression. Regressions (mutation-verified RED->GREEN): the outstanding set is bounded to 1 per repo under saturation (never-coalesce -> 32 tasks RED); a coalesced repo's key is released when its task ends so it is reprocessed, never permanently skipped (guard-drop no-op -> repo locked out, recovery copy lost forever, RED — the exact durability regression the fix prevents); distinct repos each admit; cold-set first push admits; plus an inv22_gates structural tripwire that fails if the handler gate is removed.	2026-07-15 13:10:04 -05:00
beardthelion	ca7a7074dd	test(node): INV-22 completeness guard for the served-git concurrency surface (#174) ... A source-scan tripwire that fails when a new site reintroduces the concurrency-cap class PR #174 fixed, or when one of the five gates (U1-U5) is removed. INV-22: a permit held per op recovers only if every path is duration-bounded and reaps the group before releasing admission, and every detached git task carries admission. Lives in tests/ (a separate crate) on purpose — a guard scanning the file it lives in would match its own identifier literals and pass vacuously; scanning src/ from here keeps every check load-bearing. Checks: run_bounded_git confirms child exit via child_terminated_without_reaping (U1); KillGroupOnDrop launches the reaper via Handle::try_current (U2); git_receive_pack acquires state.git_write_per_caller (U4); the encryption walk runs through withheld_recipients_gated over git_encrypt_semaphore (U5); and the bounded recipients walk is invoked exactly once — only inside the gated helper — so a new spawn_blocking bypass (count > 1) fails (P1-e non-bypass). Proven load-bearing: a synthetic second call site turns the count tripwire RED, and each named gate maps to a fix whose own regression already goes RED on revert.	2026-07-14 20:33:01 -05:00