fix(sync): preserve completed items when another hydration fails

[vincentkoc]

What Problem This Solves

Fixes: a failed issue or PR hydration discards completed work for other selected items.

User Impact

User impact: completed items remain available after a partial sync, and failed items appear in the existing failure ledger for targeted retries.

Incomplete sync and fill-pr-details runs return committed counts but exit nonzero, including quota stops. Automation must check the exit status. Older binaries can read the archive, but must not resume writes after a partial sync because they do not honor its retry checkpoint. No schema, configuration, or dependency change is required.

Maintainer compatibility decision: accepted. Nonzero quota-stop exits are intentional and approved for this change. Only checkpoint-aware writers may resume partially completed archives.

Why This Change Was Made

Each item now commits atomically with its requested children and derived evidence. Shared-head workflow observations remain consolidated; a failed group cannot discard unrelated completed items. Acquisition stops at the first quota-reserve failure without inventing failures for unattempted requests.

Actual acquisition and rolled-back persistence failures are recorded under the affected operation families. Metadata-only retries cannot clear failed comments or details. A recovered parent clears its prior failure atomically with the retained parent, even if child fetching still fails; stale or rolled-back parents do not. Quota stops retain the stopping request's snapshot without a post-batch credential lookup. Partial batches never certify successful coverage or advance the closed-sweep watermark.

Production code grows by 199 lines net to own per-item persistence, scoped failure resolution, shared-head failure isolation, and a durable retry lower bound using existing storage. Tests grow by 1,418 lines net.

Evidence

Validated at a6f2fbfdadfafb8b837a8a9a5bc6279851bb79dd with the existing Go 1.27.1 toolchain:

• Full internal/syncer tests passed, including sibling ordering, shared-head conflicts and lookup failures, quota request counts, transaction rollback/retry, failure resolution, cancellation, and checkpoint preservation.
• Focused native CLI tests passed: partial JSON counts with nonzero exit and quota handling.
• Regression tests cover atomic multi-family failure recording and empty resolution filters. The parent-retry regression failed on the previous head and passes now across issue and PR child families, stale observations, failure-record rollback, and resolution rollback without advancing completeness clocks.
• Explicit go vet ./internal/syncer ./internal/cli passed. Initial CI identified a mutex-copy test fixture; each subtest now constructs a fresh client instead.
• make docs, formatting, and git diff --check passed.
• All four commits are signed; added source, tests, and documentation were checked for private data.

Native Linux/macOS/Windows CI and the existing independent review are pending. No deployed binary, live acquisition, or production datastore has been changed.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 13, 2026, 2:44 PM ET / 18:44 UTC (Revision 3).

ClawSweeper review

What this changes

Preserve completed GitHub issue and PR hydrations through per-item transactions, retain scoped retry failures and checkpoints, and return committed counts alongside partial-sync errors.

Merge readiness

⛔ Blocked before merge - 2 items remain

The change remains necessary: current main still abandons completed acquisitions when another hydration fails. No blocking patch defect was found; the earlier mutex-copy finding is fixed, and the member author explicitly accepted the compatibility changes.

Priority: P2
Reviewed head: a6f2fbfdadfafb8b837a8a9a5bc6279851bb79dd

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused reliability repair with substantial failure-path coverage, resolved prior feedback, and an explicitly accepted compatibility contract.
Proof confidence	🌊 off-meta tidepool	Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof requirements, and no material authority change triggers an exception. Inspected tests exercise the sync and CLI owners with real SQLite and HTTP clients, but supplied test results are not an independent live-run transcript.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof requirements, and no material authority change triggers an exception. Inspected tests exercise the sync and CLI owners with real SQLite and HTTP clients, but supplied test results are not an independent live-run transcript.
Evidence reviewed	9 items	Current main still has the reported failure path: The main implementation returns immediately when comment acquisition fails, before the later persistence transaction. Earlier completed payloads therefore remain uncommitted. Latest release also retains early-abort behavior: The inspected v0.9.6 source likewise returns on hydration failure before persisting completed payloads. GitHub identifies v0.9.6 as the latest release, published September 12. Atomic persistence and truthful completion: Each completed payload receives its own transaction; counters accumulate only after commit, rollback failures are recorded by requested family, and any accumulated failure prevents successful coverage recording.
Findings	None	None.
Security	None	None.

How this fits together

Gitcrawl synchronizes GitHub issues, discussions, and PR details into a local SQLite archive used by search and review tools. Its sync pipeline controls which observations become durable evidence and when successful coverage advances.

flowchart TD
  A[Selected GitHub items] --> B[Fetch requested details]
  B --> C[Consolidate shared workflow observations]
  B --> D[Record acquisition failures]
  C --> E[Commit each completed item]
  E --> F[SQLite archive and evidence]
  D --> G[Partial counts and error]
  E --> G
  E --> H[Advance coverage only on success]

Loading

Before merge

• Resolve merge risk (P1) - Existing scripts that treat quota-limited fill completion as success will now receive a nonzero exit and may stop downstream processing; this compatibility change is explicitly accepted.
• Resolve merge risk (P1) - Older binaries can read these archives but cannot safely resume partial-sync writes because they ignore retry checkpoints; writer downgrade remains unsupported.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test growth	production +199 net lines; tests +1,418 net lines	The body justifies production growth through per-item persistence, failure isolation, scoped resolution, and durable retry checkpoints.

Merge-risk options

Maintainer options:

1. Retain the accepted compatibility contract (recommended)
   Land with the documented nonzero quota-stop behavior and checkpoint-aware writer requirement already accepted in the PR body.

Technical review

Best possible solution:

Retain per-item atomic commits, truthful partial results, and the accepted checkpoint-aware writer contract.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: let one item finish hydration and make a later comment fetch fail; current main returns before persisting the completed payload. This review did not execute that scenario.

Is this the best way to solve the issue?

Yes. Per-item transactions preserve useful work while keeping children and derived evidence atomic; scoped failure resolution and checkpoint preservation prevent partial completion from masquerading as successful coverage.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 36b4ec4c4af7.

Labels

Label justifications:

• P2: This is a bounded archive reliability repair that preserves useful completed work after partial acquisition failures.
• merge-risk: 🚨 compatibility: Quota stops change command exit behavior, and resuming partial archives requires a checkpoint-aware writer.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof requirements, and no material authority change triggers an exception. Inspected tests exercise the sync and CLI owners with real SQLite and HTTP clients, but supplied test results are not an independent live-run transcript.

Evidence

What I checked:

• Current main still has the reported failure path: The main implementation returns immediately when comment acquisition fails, before the later persistence transaction. Earlier completed payloads therefore remain uncommitted. (internal/syncer/syncer.go:210, 36b4ec4c4af7)
• Latest release also retains early-abort behavior: The inspected v0.9.6 source likewise returns on hydration failure before persisting completed payloads. GitHub identifies v0.9.6 as the latest release, published September 12. (internal/syncer/syncer.go:210, b72d4ae715d6)
• Atomic persistence and truthful completion: Each completed payload receives its own transaction; counters accumulate only after commit, rollback failures are recorded by requested family, and any accumulated failure prevents successful coverage recording. (internal/syncer/syncer.go:340, a6f2fbfdadfa)
• Checkpoint upgrade coverage: The checkpoint uses existing sync_runs storage while remaining excluded from successful freshness queries. Added tests cover a legacy archive reopening after partial persistence and a failed final run record, plus a new archive retaining its initial retry bound. (internal/syncer/closed_sweep_test.go:220, a6f2fbfdadfa)
• Earlier finding resolved: The previously reported mutex-copy fixture now constructs a fresh shared-head client inside every subtest. The GitHub commit patch confirms that correction. (internal/syncer/partial_sync_test.go:415, 2c859c1ef10e)
• Latest revision preserves parent-retry invariants: The latest commit resolves prior parent failures only when the parent observation applies, within the same transaction as child-failure bookkeeping. Its regression cases cover stale observations and rollback of either failure recording or resolution. (internal/syncer/syncer.go:667, a6f2fbfdadfa)

Likely related people:

• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Andy Ye: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)

• reviewed 2026-09-13T18:20:16.630Z sha b725747 :: blocked before merge. :: [P2] Construct a fresh shared-head client without copying its mutex
• reviewed 2026-09-13T18:29:41.607Z sha 2c859c1 :: blocked before merge. :: none