Repository navigation
fix(sync): avoid starving windowed full scans - #305
Merged
steipete merged 8 commits intoOct 23, 2026
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Reproduce starvation when an old incomplete channel keeps backlog resume from reaching a live channel with new messages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A sync with a since cutoff cannot complete history, so resuming its backlog can starve catalog discovery and live channels indefinitely. Require an unbounded window for backlog resume while preserving the existing early return. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rszrszrsz
force-pushed
the
fix/windowed-full-since-starvation
branch
from
October 21, 2026 12:37
59fdfeb to
1d32d03
Compare
clawsweeper
Bot
added
clawsweeper:intake:bug
Contributor
|
Codex review: needs maintainer review before merge. What this changesThis PR limits incomplete-history backlog resumption to full syncs without a time window. Example: Run a full sync since 2026-10-01 with an incomplete old channel and new messages in another channel.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review This PR fixes a current-main sync starvation bug with a focused change; no actionable correctness findings remain. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe sync planner receives CLI options and stored channel progress, selects Discord channels, and sends their messages through the existing history crawler into SQLite. flowchart TD
CLI[Sync CLI options] --> Planner[Guild sync planner]
State[Stored history progress] --> Planner
Planner --> Backlog[Unwindowed backlog resume]
Planner --> Catalog[Channel catalog discovery]
Backlog --> Crawler[Message history crawler]
Catalog --> Crawler
Crawler --> Archive[SQLite archive]
Technical reviewBest possible solution: Keep the existing backlog planner for unwindowed full syncs and route windowed full syncs through normal catalog discovery. Do we have a high-confidence way to reproduce the issue? The current-main early return and non-completing window path establish the failure in source, supported by contributor-supplied failing-base regression output and real CLI logs; I did not execute a reproduction. Is this the best way to solve the issue? The existing predicate is the smallest complete fix boundary: excluding bounded runs prevents starvation without changing unwindowed batching, explicit channel targeting, or history completion semantics. AGENTS.md: found but not applied because it conflicted with ClawSweeper's review contract. Codex review notes: model internal, reasoning medium; reviewed against 6d670fc461f4. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
Reviewed October 21, 2026, 9:11 AM ET / 13:11 UTC (Revision 2). |
Contributor
Author
|
Added the captured regression output and redacted real CLI runs to the Evidence section. @clawsweeper re-review |
Contributor
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:bug
* fix(ci): pin Node releases with incident key rejection * fix(images): upgrade Alpine certificate packages
…nclaw#307) * fix: preserve ingestion scope and recover rich-text integrity * fix: guard recovery snapshots against newer source observations * fix: distinguish retained metadata receipts from projection failures * fix: bound receipt reconciliation without starving later failures * fix: enforce durable SQLite writer connections * fix: preserve SQLite writer path identity through symlink parents * fix: retain Windows drive-relative writer paths * feat: measure opt-in native OpenAI embedding attempts * test: pin public embedding default-tier measurement contract * fix: reconcile full members under the native tail owner * fix: measure and bound member pagination waits separately * fix: preserve native Components V2 and fence recovery pages * fix: support protected file credentials for unattended services * fix: keep newer edits when delayed message pages arrive Bot API REST pages and Gateway refetches now preserve a stored message whose edited_at is later, or which is tombstoned, together with its attachment, mention, search and embedding projections. * chore: satisfy current lint gates for landed collector fixes Use context-aware test queries and commands, prefer switch/range forms, and document the two intentional exceptions: the descriptor-relative macOS ACL check has no non-deprecated x/sys wrapper, and protected-credential fixtures must avoid world-writable temporary ancestors such as /tmp. * fix: keep history verification retryable when clearing its marker is cancelled A cancelled context can be reported after the history_complete deletion was already committed, which left an empty channel without its marker and no verification retry. Attempt the existing idempotent restoration whenever clearing reports failure, as already done when the crawl itself fails. Durable writer commits made this window reproducible under race-test load. * fix: keep stage exclusions, newer edits and removal replay correct - Keep the established type_13 stage kind so existing exclusions, coverage and incomplete-channel classification still apply; read the legacy "stage" spelling from earlier local builds and match both in exclusions. - Order recovery writes by provider edit time first. A strictly newer edit now wins even after projection-only writes advanced updated_at; the observation fence applies only when edit times tie or are absent. - Record each failed member removal with its own event time, so replay never judges a later removal by an earlier one; retries keep the event time. - Clear superseded attributed text parts when a writer without a projection, such as a Desktop import, replaces a message. - Record the landed collector changes in the changelog. * fix: keep stale Gateway observations and newer pending replay work - When newer canonical state is retained over a stale create or update, still append the genuine Gateway or snapshot observation in the same transaction instead of dropping it with the canonical write. - Resolve a replayed metadata failure only for the generation that replay read (failure id plus retry count), so a newer failure recorded for the same identity while replay was in flight stays pending. * fix: keep repair frontiers, thread tombstones and import durability intact - Automatic tail repair recovers individual failed messages without moving the channel history frontier, so ordinary REST repair still fetches gaps. - Message replay resolves only the failure generation it read. - Full catalog scans store only channels Discord returned in that scan; threads rebuilt from stored rows keep their raw payload and tombstones instead of being rewritten (and resurrected) as fresh observations. - Failed member removals use the Gateway receipt time for replay. - Replay of a failed GUILD_DELETE tombstones the guild when access is gone and stores the current guild when access has returned. - Snapshot imports no longer lower the writer's synchronous policy, so durable writer connections stay FULL during and after imports. * fix: merge schema-7 publications and keep metrics and scope durable - Let upgraded readers merge publications that only append the defaulted schema-7 integrity columns, preserving their local rows; any other column change still requires replacement. - Route both metrics writer paths through the durable SQLite adapter (as in the deployed standalone metrics runtime), so metrics writes are FULL too. - Fence replayed guild tombstones against a Gateway rejoin stored while the deletion request was in flight. - Recheck a cached exclusion against the persisted channel revision so the running tail owner picks up scope repaired by metadata replay. * fix: replay member removals against current membership and revalidate scope - Replay a failed GUILD_MEMBER_REMOVE by asking Discord for the member: Unknown Member tombstones the row even when a later full enumeration stamped it with an older snapshot; a current member is stored instead. Without exact member lookup the removal time remains the fence. - Revalidate any cached scope decision (allowance or exclusion) against the persisted channel revision before content-fetch I/O, so the running tail owner never fetches content from a channel replay moved out of scope. * fix: stop crawling a channel the moment it leaves collection scope Recheck the persisted collection scope before each REST page and before attachment extraction, and treat scope loss as a per-channel skip, so a concurrent Gateway move into an excluded category neither fetches more excluded content nor aborts the rest of the repair run. Member-removal replay fixtures now model Discord's Unknown Member response. * fix: recheck collection scope before every attachment download Carry a persisted-scope guard through sync, tail, replay and attachment retry so each attachment request first confirms the channel is still in collection scope; a Gateway move during a message refetch or extraction can no longer reach attachment I/O. Scope loss is a policy outcome: channel work resolves as excluded_by_collection_policy, replay resolves the generation it read, and attachment retry skips without recording a failure, so status does not stay degraded after a successful exclusion. * fix: enforce tombstone admission before attachment I/O and count only actionable failures * fix: treat every live and replay scope rejection as a policy outcome before content I/O * docs: describe tombstone admission and actionable attachment failure health * test: avoid require inside the attachment test handler * test: keep the short handler deadline to the timeout case * test: cover components v2 text, attachment codes, thread lists and guild tombstones * chore: date the 0.17.0 changelog section * Revert "chore: date the 0.17.0 changelog section" This reverts commit 8fa0a18. * fix: keep author and channel names when text repair rewrites lexical indexes * fix: keep channel tombstones through Desktop cache imports * fix: keep authoritative ancestry through cache imports and count policy skips * fix: fence Gateway update refetches against newer recovery snapshots * fix: resolve metadata failures superseded by deletion or Unknown Channel/Member
…#308) Under FULL synchronization each per-channel commit pays a full flush, so a guild with several thousand active threads exceeded the 30s ordered tail handler deadline at startup and left the catalog partially applied. Batch GUILD_CREATE and THREAD_LIST_SYNC catalogs through UpsertChannels.
…able (openclaw#309) * fix: settle corroborated Unknown Channel failures as provider-unavailable A message request answered Unknown Channel stayed a retryable failure forever, degrading status, while public-only snapshots kept publishing the channel from stale permissions. Corroborate it with one exact channel lookup; only a second Unknown Channel resolves the failure as provider_unavailable_404, keeping history, scope and the retry marker. Ordinary repair settles earlier failures superseded by recorded deletions (including ancestors) or exclusions, and public-only snapshots withhold marked channels and their threads until a successful crawl re-admits them. Also fix a nil-map panic in routine syncs when only marked channels remain incomplete. * chore: satisfy lint for unknown-channel reconciliation * fix: rotate the channel failure repair lane past pending rows The lane always read the oldest 200 unresolved rows, so rows that stay pending (non-404 or inconclusive lookups) could starve a later settleable failure. Page by failure_id after a persisted cursor and wrap at the end.
Integrate current main, document bounded discovery, and retain the contributor regression and fix. Co-authored-by: rszrszrsz <rszrszrsz@users.noreply.github.com>
steipete
changed the title
Contributor
|
Merged in 73c2287. The starvation regression failed against the previous main (only message 200) and passed with the fix (200, 250, 300). The original contributor fix and test are preserved, with documentation and credit. CI exposed an existing 40 ms tail-test race. That test now waits for its own repair message and joins the tail before closing the store; 20 consecutive race-enabled runs passed. The full remote Go gate passed at 85.3% coverage, all six snapshot targets built, and independent P0–P3 review was clean. Final-head CI: https://github.com/openclaw/discrawl/actions/runs/39903167284 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
discrawl sync --full --since <time>could repeatedly resume an incomplete old channel and return before discovering the guild's other channels. Bounded scans intentionally never mark older history complete, so that backlog could starve new-message collection indefinitely.Restrict backlog resumption to unwindowed full syncs. Windowed full syncs now discover the normal catalog and retain the existing per-channel permission checks and coverage receipts. Unwindowed batching and explicit channel selection keep their existing behavior. Document the distinction and preserve @rszrszrsz's regression and fix, integrated with current main.
Validation: the synthetic regression fails against current main with only message
200retained, and passes with200,250, and300after the fix. Independent P0–P3 review is clean. The contributor also supplied redacted real CLI evidence showing discovery recovering from 14 repeatedly resumed channels to the full 90-channel catalog. GitHub Actions validates the current head.The first CI run exposed an existing flaky tail-repair test: it canceled after 40 ms and could count a Gateway message without exercising repair. The test now waits for its own repair message and joins the tail goroutine before closing the database, including on failure. A delayed fixture reproduced the old failure; the corrected test passed 20 consecutive runs under the race detector. Independent P0–P3 review of the complete final change is clean. Remote Linux validation passed all code gates with 85.3% coverage, and the six-platform release snapshot passed.