Skip to content

sync: say why the tombstone sweep refused, instead of refusing silently - #61

Merged
myobie merged 1 commit into
mainfrom
sync/sweep-says-why
Aug 23, 2026
Merged

sync: say why the tombstone sweep refused, instead of refusing silently#61
myobie merged 1 commit into
mainfrom
sync/sweep-says-why

Conversation

@myobie

@myobie myobie commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Step 1 of the Bluey plan. No behaviour change: the gate decides exactly what it
decided before.

The fault

The sweep forgets a tombstone only when every configured peer has acked while
this node held it. That gate is correct and load-bearing — forgetting early lets
a peer still holding the file as Present adopt it back, and the deletion loses
to the oldest copy on the fleet.

The fault was not the refusal. It was that the refusal said nothing. The only
log line sat inside if !swept.is_empty(), so "waiting on a peer" and "nothing
to sweep" were the same observation from outside. An entry could wait on one
roaming peer forever and look identical to a healthy one.

acked_through returned a bare Option, throwing the reason away at the point
it was known.

The change

ack_gate returns a typed SweepState, and every refusal names itself:

token meaning
disabled the sweep is off, which is the default
policy-retains catalog keeps tombstones by design
never-sweeps-wildcard-peers a wildcard peer set can NEVER sweep
peers-unresolved:1/2 a configured peer is not in the peer book
waiting-on:bluey named peers, because "waiting" without a name was the gap
ready the gate is open

The wildcard trapdoor

"*" is the obvious way to add a roaming peer to an entry. It does not make the
sweep block on that peer — it turns the sweep off entirely, because
membership is whatever peers.toml holds at that moment and a peer leaving the
book must never be what makes a deletion forgettable.

So the natural action silently disabled the feature. That case now logs at
warn and its token says it can never sweep, rather than that it merely did
not.

Reported two ways, deliberately

  • fabric sync ls gains a sweep= column, so an operator asks instead of
    grepping.
  • The log fires only on a CHANGE. The sweep runs on every pass; an
    unconditional line would add thousands of identical entries an hour to a
    validation log that already writes about 368 KB an hour. Making the fix bury
    its own signal would have been a poor trade.

Tests, each proved to bite

  • Collapsing named waiting peers into peers-unresolved fails on "the peer
    holding the sweep up must be named"
    .
  • Reporting the wildcard as an ordinary refusal fails on "the token must say it
    can never sweep, not merely that it did not"
    .

A second test pins the state on Silber today: Bluey is in the node peer book but
in neither sync entry — both read peers=hetz,droppy — so it cannot gate
them. Adding it to an entry would show as a named wait rather than as silence.

Wire compatibility

SyncEntryStatus.sweep carries #[serde(default)], matching the counters
beside it, so an older client decodes a newer daemon. sync ls renders
unknown rather than an empty string when a daemon predates the field, so a
missing value never reads as a state.

Verification

macOS: 233 lib, 10 provisioning, 3 sync_slice, 21 local_slice. Clippy matches
main with an empty set difference both ways, and every file I touched is
rustfmt clean. The exec.rs and node.rs rustfmt hunks are pre-existing on
main and untouched here.

That is a macOS claim; this PR's CI run is the Linux face and I read the job
steps before calling it green.

The sweep forgets a tombstone only when every configured peer has acked while
this node held it. That gate is correct and it is load-bearing: forgetting a
tombstone early lets a peer still holding the file as Present adopt it back, and
the deletion loses to the oldest copy on the fleet.

The fault was not the refusal. It was that the refusal said NOTHING. The only
log line sat inside `if !swept.is_empty()`, so "waiting on a peer" and "nothing
to sweep" were the same observation from outside. An entry could wait on one
roaming peer forever and look identical to a healthy one.

`acked_through` returned a bare Option, which threw the reason away at the point
it was known. It is now `ack_gate` returning a typed `SweepState`, and every
refusal names itself:

  disabled                     the sweep is off, which is the default
  policy-retains               catalog keeps tombstones by design
  never-sweeps-wildcard-peers  a wildcard peer set can NEVER sweep
  peers-unresolved:1/2         a configured peer is not in the peer book
  waiting-on:bluey             named, because "waiting" without a name was the
                               thing that was missing
  ready                        the gate is open

The wildcard case is the trapdoor and it is why this is a warning rather than a
note. `"*"` is the obvious way to add a roaming peer to an entry, and it does
not make the sweep block on that peer: it turns the sweep OFF entirely, because
membership is whatever `peers.toml` holds at that moment and a peer leaving the
book must never be what makes a deletion forgettable. Doing the natural thing
silently disabled the feature. Now it says so.

The state is reported two ways, and the split is deliberate. `fabric sync ls`
carries a `sweep=` column, so an operator asks rather than greps. The log fires
only on a CHANGE, because the sweep runs on every pass and an unconditional line
would add thousands of identical entries an hour to a validation log that
already writes about 368 KB an hour.

Tests cover every reason, and I proved each one bites. Collapsing named waiting
peers into `peers-unresolved` fails on "the peer holding the sweep up must be
named". Reporting the wildcard as an ordinary refusal fails on "the token must
say it can never sweep, not merely that it did not".

A second test pins the state on Silber today: Bluey is in the node peer book but
in NEITHER sync entry, both of which read peers=hetz,droppy, so it cannot gate
them. Adding it to an entry would show as a named wait rather than as silence.

No behaviour change. The gate decides exactly what it decided before.
@myobie
myobie merged commit 6c01a54 into main Aug 23, 2026
2 checks passed
@myobie
myobie deleted the sync/sweep-says-why branch August 23, 2026 18:41
myobie added a commit that referenced this pull request Aug 24, 2026
`cargo test` did not compile on main. The `sync ls --json` schema test in
`src/main.rs` builds a `SyncEntryStatus` literally, and #61 added a
`sweep` field to that struct without updating it, so the bin test target
failed with E0063.

CI stayed green through all of it, because the workflow runs `--lib` and
three named integration targets and never builds the binary's own tests.

So the gap is not the missing field. The gap is that a test could stop
COMPILING and every check still passed.

FIXING THE COMPILE ERROR THEN EXPOSED A REAL FAILURE, which is the point.
`sync_ls_json_schema_exposes_counts_and_drift` pins the exact JSON of
`fabric sync ls --json`. #61 added `sweep` to that output. The test that
exists to catch a wire-schema change did not catch it, because it had not
run since before the change.

I WATCHED THE NEW CHECK FAIL. `cargo test --bins` failed on the compile
error, then failed again on the schema assertion with `sweep` present on
the left and absent on the right, then passed once the expectation
carried the field. That is the positive control, in this order, on this
machine.

Two changes, and they belong together because either alone leaves the
hole open: the expectation now carries `sweep`, and the workflow now runs
`cargo test --locked --bins`.

Verified on macOS 15 arm64. 12 bin tests pass. Linux is CI's to confirm.

Agent: Silber.fabric
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant