Address review: thread the empty-source policy through unstable, and guard the class

main

Commit

nodo2w ago

unstable.Client accepted gitsync.SyncPolicy and dropped AllowEmptySource, SourceAssertedEmpty and TargetAssertedEmpty on the floor, so Plan/Sync/Replicate there could not use the feature at all — it was accepted by the API and then ignored.

The interesting part is why no test failed. unstable already had a test asserting that "advanced options" propagate, and it enumerates the fields it checks by hand, so it covered exactly what someone had remembered to add to it. A newly declared policy field is therefore invisible to it by construction. That is the same shape as the two protocol findings on this branch: the check existed, and the check's own blind spot was the bug.

So both config builders now get a reflection guard: for every bool on SyncPolicy, set it alone and require the same-named bool on syncer.Config to be set. A new policy bool is covered the moment it is declared, and the test fails until it is threaded — verified by removing one assignment and watching it go red, rather than trusting that it would. A field whose config counterpart is deliberately named differently, or deliberately absent, is meant to be listed in the skip map with a reason instead of quietly renamed to pass.

Also corrects the ErrNoRefsSelected doc, which described the empty-source errors below it as meaning the source has "no refs AT ALL". That contradicts the fail-closed contract those errors exist to express: they cover a source that ADVERTISED no refs, which is deliberately the weaker statement, because whether the repository really holds none is not something a client can determine.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01714HJZAqpgwuwp6fcMWEhG Entire-Checkpoint: 01M0JF4F1CK0PDKS2AG0JF5DYN

Checkpoints

Fix Git Sync Source Scope Handling

Claude CodeOpus 5
View session
Checkpoint 1