`SpaceSyncer.SyncRepoAsync` treated a missing commit as "the page stopped
short of the head of the oplog, sync again to continue". That is one of two
ways to get no commit. The other is an account with no repo state: the host
builds the commit from that state, so `listRepoOps` answers
`{ops: [], commit: undefined}` for a member who has never written to the
space, rather than refusing the read.

Such a pass applied nothing, advanced nothing, and reported `Partial` — the
outcome documented as "the caller should sync again to continue" — so a
caller looping on that contract spun on the repo forever.

`Partial` is now reported only when the pass has somewhere left to go: it
applied operations, or the host offered a continuation cursor. A page
carrying neither is `NoRepo`, the same answer `getRepo` gives by refusing
outright. A cursor already standing at a revision takes the existing repair
path instead — the host reporting no state for a repo the caller holds a
copy of is what full recovery resolves, dropping the stale copy on
`RepoNotFound` and rebuilding it otherwise.

No signature changed and no enum member was added.

closes #99

claude-bot referenced this pull request

2026-08-20 21:51:19 +00:00

SpaceSyncer reports Partial forever for a repo the account never wrote to #99

claude-bot commented

2026-08-20 21:51:24 +00:00

Author

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/331); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-100/run-331.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-20 21:52:53 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Traced the fix through SyncRepoAsync/NothingToCommitAsync/RecoverAsync in src/ATProtoNet/Spaces/SpaceSyncer.cs against the new tests. The logic is sound: Partial is now reserved for pages that made progress (applied.Count > 0) or offered a continuation cursor, while a page with neither is routed to NothingToCommitAsync, which correctly branches on whether cursor.Rev was already non-null (fresh cursor -> report NoRepo immediately without a wasted round-trip; previously-advanced cursor -> full recovery via RecoverAsync, which itself already knows how to drop the local copy on a RepoNotFound from getRepo). The new unit tests (SpaceSyncerTests.cs) exercise exactly the three relevant branches — continuation-cursor-forces-Partial, fresh-cursor-never-written, and stale-cursor-now-gone — and the integration test update matches. XML doc remarks and the docs/spaces.md addition accurately describe the new behavior, and the CHANGELOG entry is thorough and correctly filed under Fixed. No correctness, security, or documentation issues found.

One pre-existing (not introduced by this PR) asymmetry worth a mental note for a future pass: the RepoNotFound-exception path at the top of SyncRepoAsync (line 196) returns NoRepo with cursor.Rev left as-is and without dropping the store, whereas RecoverAsync's missing-repo branch resets the cursor and drops the store. Not something this PR needs to fix, since it's untouched code, but the two NoRepo-producing paths now differ slightly in what state they leave behind.

⏱ 1m 10s · 6 turns · tokens: 37.4k in / 5.8k out (+293.4k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Traced the fix through `SyncRepoAsync`/`NothingToCommitAsync`/`RecoverAsync` in `src/ATProtoNet/Spaces/SpaceSyncer.cs` against the new tests. The logic is sound: `Partial` is now reserved for pages that made progress (`applied.Count > 0`) or offered a continuation cursor, while a page with neither is routed to `NothingToCommitAsync`, which correctly branches on whether `cursor.Rev` was already non-null (fresh cursor -> report `NoRepo` immediately without a wasted round-trip; previously-advanced cursor -> full recovery via `RecoverAsync`, which itself already knows how to drop the local copy on a `RepoNotFound` from `getRepo`). The new unit tests (`SpaceSyncerTests.cs`) exercise exactly the three relevant branches — continuation-cursor-forces-Partial, fresh-cursor-never-written, and stale-cursor-now-gone — and the integration test update matches. XML doc remarks and the `docs/spaces.md` addition accurately describe the new behavior, and the CHANGELOG entry is thorough and correctly filed under `Fixed`. No correctness, security, or documentation issues found. One pre-existing (not introduced by this PR) asymmetry worth a mental note for a future pass: the `RepoNotFound`-exception path at the top of `SyncRepoAsync` (line 196) returns `NoRepo` with `cursor.Rev` left as-is and without dropping the store, whereas `RecoverAsync`'s missing-repo branch resets the cursor and drops the store. Not something this PR needs to fix, since it's untouched code, but the two `NoRepo`-producing paths now differ slightly in what state they leave behind. --- ⏱ 1m 10s · 6 turns · tokens: 37.4k in / 5.8k out (+293.4k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-100/run-331.md)