Skip to content

Track failed sync hydration attempts#90

Merged
steipete merged 9 commits into
openclaw:mainfrom
TurboTheTurtle:track-sync-failures-84
Jul 17, 2026
Merged

Track failed sync hydration attempts#90
steipete merged 9 commits into
openclaw:mainfrom
TurboTheTurtle:track-sync-failures-84

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Closes #84.

Adds a sync-attempt failure ledger for PR hydration errors, exposes it through a read-only CLI command, marks failures resolved after a later successful hydration, and treats pre-ledger read-only databases as an empty ledger instead of failing on a missing table.

Behavior proof:

gitcrawl sync openclaw/gitcrawl --numbers 90 --with pr-details
list pull request review threads for #90: github POST http://127.0.0.1:<redacted>/graphql failed with status 503: {"message":"forced review-thread outage for proof"}
{"failures":[{"number":90,"operation":"pull_review_threads","error_class":"error","retry_count":0}],"include_resolved":false,"repository":"openclaw/gitcrawl"}
{"repository":"openclaw/gitcrawl","pull_requests_synced":1,"pr_details_synced":1,"pr_files_synced":1,"pr_commits_synced":1}
{"failures":[{"number":90,"operation":"pull_review_threads","resolved_at":"2026-07-01T19:41:33.14832Z"}],"include_resolved":true,"repository":"openclaw/gitcrawl"}

The proof run used the actual CLI against a throwaway local archive and a local GitHub-compatible endpoint that forced one review-thread hydration failure, then allowed the retry to succeed.

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 12:48 AM ET / 04:48 UTC.

Summary
Adds a durable SQLite ledger for failed PR hydration operations, retry resolution, a read-only sync-failures command, coverage reporting, and privacy-preserving portable-export controls.

Reproducibility: not applicable. as a bug reproduction: this PR adds a new durable tracking capability. Its terminal proof does provide a high-confidence exercise of the intended failure, query, retry, and resolution workflow.

Review metrics: 3 noteworthy metrics.

  • Change surface: 20 files, +1120/-60. The implementation spans schema, storage, synchronization, CLI, coverage, portable exports, documentation, and tests.
  • Persistent data model: 1 table, 2 indexes, schema 11→12. The merge creates a lasting upgrade and compatibility obligation.
  • Public interfaces: 1 command plus additive JSON and coverage fields. Maintainers should explicitly accept the machine-readable API surface before release.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #84
Summary: This PR is the explicit implementation candidate for the canonical open failure-tracking issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] Merging establishes schema version 12 and additive CLI, JSON, coverage, and portable-export behavior that future releases must preserve or explicitly migrate.
  • [P1] The local ledger intentionally retains full operational error text; maintainers should confirm that default-local storage plus redacted opt-in portable export is the desired long-term privacy boundary.

Maintainer options:

  1. Approve the permanent contract (recommended)
    Accept schema version 12 and the additive command, JSON, coverage, and export behavior after explicit maintainer review.
  2. Narrow before landing
    Remove any unapproved permanent interface while preserving the core ledger and focused regression coverage.
  3. Pause the implementation
    Keep the canonical issue open and pause or close this PR if durable failure history is not an intended product contract.

Next step before merge

  • [P2] A maintainer should explicitly accept or narrow the permanent schema and public reporting contract; no mechanical contributor repair remains.

Maintainer decision needed

  • Question: Should gitcrawl permanently adopt schema version 12 and the proposed CLI, JSON, coverage, and portable-export contract for hydration failure history?
  • Rationale: No narrow correctness defect remains, but the change creates durable stored-data and public machine-readable interfaces whose permanence and privacy boundary require maintainer intent.
  • Likely owner: steipete — They authored the latest compatibility and privacy hardening commits and are best positioned to confirm whether that resulting contract should become permanent.
  • Options:
    • Adopt the full contract (recommended): Merge the ledger, reporting surfaces, resolution semantics, and privacy-preserving portable-export behavior as the supported operational contract.
    • Narrow the public surface: Retain durable tracking while removing or deferring any CLI, coverage, or export interfaces maintainers do not want to support permanently.
    • Decline durable tracking: Close the PR and keep failure history in transient logs or external operational tooling instead of the gitcrawl database.

Security
Cleared: No concrete security or supply-chain defect remains; failure history stays local by default, and explicit portable export redacts messages and rewrites residual database pages.

Review details

Best possible solution:

Adopt the ledger as one coherent operational contract only after maintainer sign-off, retaining local full-detail diagnostics, default exclusion from portable stores, redacted opt-in export, and compatibility coverage for older databases.

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

Not applicable as a bug reproduction: this PR adds a new durable tracking capability. Its terminal proof does provide a high-confidence exercise of the intended failure, query, retry, and resolution workflow.

Is this the best way to solve the issue?

Yes technically: the ledger directly addresses the linked issue and now handles coverage and portable-data privacy coherently. Final acceptance remains a product decision because the implementation creates permanent schema and public-interface commitments.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against ffae350a35b5.

Label changes

Label justifications:

  • P2: This is a bounded, normal-priority operational observability improvement rather than an urgent regression or availability failure.
  • merge-risk: 🚨 compatibility: The PR increments the persistent schema and establishes lasting CLI, JSON, coverage, and portable-export contracts.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body demonstrates the changed behavior with a real CLI run: a forced hydration outage is recorded, queried, retried successfully, and then shown as resolved.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body demonstrates the changed behavior with a real CLI run: a forced hydration outage is recorded, queried, retried successfully, and then shown as resolved.
Evidence reviewed

Acceptance criteria:

  • [P1] go test ./internal/store ./internal/syncer ./internal/cli.

What I checked:

  • Persistent schema contract: The branch raises the database schema from version 11 to 12 and adds the sync_attempt_failures table with uniqueness, resolution, retry, and lookup-index semantics. (internal/store/schema.go:445, b757d5bcc9aa)
  • User-facing reporting contract: The branch adds the sync-failures CLI command with JSON, resolved-history, and limit options, and exposes unresolved failure counts through coverage output. (internal/cli/app.go:2311, b757d5bcc9aa)
  • Real behavior proof: The PR body records an actual CLI run against a throwaway archive and GitHub-compatible endpoint, showing a forced 503 hydration failure, queryable ledger output, successful retry, and a populated resolution timestamp. (b757d5bcc9aa)
  • Portable-data boundary: The latest branch excludes failure history from portable stores by default and provides an explicit opt-in path that redacts error messages and securely rewrites the database. (internal/store/portable.go:15, 3a1c0cf60b20)
  • Prior review continuity: The earlier coverage finding was addressed by wiring unresolved failures into repository and total coverage, and the later portable-store finding was addressed by the privacy-hardening commits; the latest completed review reported no remaining line-level defect. (internal/store/archive_coverage.go:110, b757d5bcc9aa)
  • Validation state: The current head is cleanly mergeable and all supplied checks succeeded: Docker, Go on macOS, Go on Ubuntu, and verified-secret scanning. (b757d5bcc9aa)

Likely related people:

  • steipete: Authored the latest commits protecting portable exports and hardening failure-ledger reporting, making them the strongest available routing candidate for the remaining contract decision. (role: recent feature hardening contributor; confidence: high; commits: 3a1c0cf60b20, b757d5bcc9aa; files: internal/store/portable.go, internal/store/sync_failures.go, internal/syncer/syncer.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.
Review history (8 earlier review cycles)
  • reviewed 2026-07-01T19:51:59.982Z sha 62069ed :: needs maintainer review before merge. :: none
  • reviewed 2026-07-06T12:11:04.399Z sha 62069ed :: found issues before merge. :: [P2] Wire the ledger into coverage reporting
  • reviewed 2026-07-06T18:35:34.972Z sha 6743734 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-07T16:39:26.529Z sha 6743734 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-07T18:15:04.462Z sha 055cbf7 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-08T15:37:37.025Z sha 055cbf7 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-16T06:39:30.311Z sha cc59e34 :: found issues before merge. :: [P1] Exclude the failure ledger from portable stores
  • reviewed 2026-07-17T04:37:20.094Z sha 3a1c0cf :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 1, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 1, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 6, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Merged main to clear the conflict and wired the new failure ledger into coverage so unresolved hydration failures are counted in the coverage JSON totals. Focused store/CLI coverage and sync-failure tests pass locally.

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 6, 2026
# Conflicts:
#	internal/cli/app_test.go
#	internal/cli/archive_coverage_test.go
#	internal/store/archive_coverage.go
#	internal/store/archive_coverage_test.go
#	internal/store/store.go
#	internal/store/store_test.go
#	internal/syncer/syncer.go
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Merged main again to clear the conflict. Focused store, CLI, and syncer tests pass locally.

@clawsweeper

clawsweeper Bot commented Jul 15, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 16, 2026
steipete and others added 2 commits July 16, 2026 20:51
Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 17, 2026
steipete and others added 2 commits July 16, 2026 21:44
Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
@steipete

Copy link
Copy Markdown
Contributor

Maintainer repair proof for exact head 3768ea9:

  • Full local gate passed: formatting, go mod tidy diff check, go vet, govulncheck (no vulnerabilities), deadcode audit, make test-coverage (85.1% total), real binary build/version smoke, and scripts/test-release.sh.
  • Built-binary behavior proof persisted a forced pull-request hydration failure and listed it through sync-failures.
  • Default portable prune: sync_attempt_failures table count 0; portable metadata lists it as excluded.
  • Opt-in portable prune: table count 1; error text exactly [redacted for portable export].
  • Both exports forced the privacy rewrite with --no-vacuum. Current and historical private error fragments were absent from raw SQLite bytes afterward.
  • Regression coverage includes empty JSON arrays, canceled-context ledger writes, zero-row coverage capability, deleted ledger bytes, interrupted scrub recovery, and a live-reader WAL checkpoint block that retains the pending scrub marker.
  • AutoReview clean after accepted findings were fixed.

Hosted exact-head checks are green. Contributor credit remains with @TurboTheTurtle in commit trailers and the changelog.

@steipete
steipete merged commit 602d331 into openclaw:main Jul 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Track failed sync and hydration attempts

2 participants