Skip to content

feat: recover restart gaps when tail connects - #219

Merged
hannesrudolph merged 3 commits into
mainfrom
feat/tail-startup-repair
Sep 11, 2026
Merged

feat: recover restart gaps when tail connects#219
hannesrudolph merged 3 commits into
mainfrom
feat/tail-startup-repair

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Sep 11, 2026

Copy link
Copy Markdown
Member
Additional instructions

Keep Allow edits from maintainers enabled.

What Problem This Solves

Fixes an issue where users restarting discrawl tail after downtime can wait until the next periodic repair for missed messages to appear. A six-hour repair interval can leave that history gap for hours, requiring an external pre-start sync.

Why This Change Was Made

Add opt-in tail --repair-on-start. After the Gateway connects and the existing ownership-ready callback succeeds, request one immediate latest-only repair through the existing serialized repair lifecycle. Capture remains connected during REST catch-up, and both use the same archive writer owner. Shutdown cancels and joins the repair through the existing bounded cleanup path.

In this opt-in mode, REST alone advances history cursors for the lifetime of the tail; live events still update messages and live freshness immediately. A new live message therefore cannot mark offline history as fetched, and an interrupted owner leaves a recoverable cursor for the next startup. This reuses existing state without another cursor or checkpoint.

User Impact

Service managers can start Discrawl directly and recover restart gaps without a separate sync process. Default behavior is unchanged. Repairs in the new mode may re-fetch already captured messages because history progress belongs to REST. The option also works with --repair-every 0 for startup-only repair and rejects replay-only mode.

Evidence

  • Temporary-archive tests recover an offline message with periodic repair disabled and with a six-hour interval, while a new Gateway message is written during a blocked REST request. Both messages are queued for embedding.
  • Tests verify repair waits for Gateway readiness and the ownership callback, does not overlap periodic repair, and is joined before shutdown returns.
  • CLI coverage verifies explicit selection, default behavior, and replay-only incompatibility.
  • Full Go race suite, lint, module tidy, and unchanged dependency-file checks passed. No live archive is used by tests.
  • Final structured autoreview completed with no actionable findings.
  • The initial ClawSweeper cursor finding was reproduced: a live event arriving before the repair reads channel state caused the offline message to remain missing. The corrected regression passes for both zero and six-hour repair intervals, and a close/reopen test proves an interrupted owner retains recoverable progress.

@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

🦞👀
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 clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. 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. labels Sep 11, 2026
@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 11, 2026, 6:12 PM ET / 22:12 UTC (Revision 2).

ClawSweeper review

What this changes

Adds opt-in startup catch-up for Discord capture, preserving history cursors while live messages arrive and reusing the existing repair lifecycle.

Merge readiness

Ready for maintainer review

The previous cursor-race finding is resolved, and no remaining blocking defect was found. Startup catch-up remains distinct from current main and the latest release; this member-authored PR should remain available for landing.

Priority: P2
Reviewed head: 9fac23d05a03b3e78eb5d85a36435022454ea617

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused implementation resolves the prior blocker, preserves defaults and adds relevant restart and concurrency coverage.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor-proof gate. Supplied evidence exercises RunTail with temporary SQLite archives and fake Discord clients; it supports regression coverage but is not a live Discord run.
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 the ordinary contributor-proof gate. Supplied evidence exercises RunTail with temporary SQLite archives and fake Discord clients; it supports regression coverage but is not a live Discord run.
Evidence reviewed 6 items Previous finding resolved: The pinned earlier-head comparison confirms the new preserveHistoryCursor guard prevents live message creation from advancing history progress. Revised tests cover live arrival before readiness and during REST fetches at both zero and six-hour repair intervals, plus archive close/reopen recovery.
Repair lifecycle and history ownership: Read RunTail, startup regressions, existing cancellation/serialization tests, and REST pagination. Startup waits for the ownership callback; periodic repair is suppressed until readiness and while repair is active. REST advances the existing cursor after persisting fetched pages. Exact-message replay is a separate CLI mode and is rejected with startup repair.
Current main still lacks startup catch-up: The pinned main implementation starts repairs only from its periodic timer and returns directly to Gateway capture when the interval is disabled. Standalone sync is documented, but does not supply the requested connected, same-owner startup lifecycle.
Findings None None.
Security None None.

How this fits together

Discrawl archives Discord Gateway events and REST history into a local SQLite database. Startup repair fetches missed history while live capture continues, using the same archive owner.

flowchart TD
 A[Tail command options] --> B[Connect Discord Gateway]
 B --> C[Archive ownership ready]
 C --> D[Serialized REST catch-up]
 B --> E[Live message capture]
 D --> F[SQLite archive and history cursor]
 E --> G[Archive messages and live freshness]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth production +57 net lines; tests +246 net lines Production growth is justified by opt-in startup scheduling and cursor preservation, with focused concurrency and restart coverage.

Technical review

Best possible solution:

Keep startup catch-up opt-in, with REST owning history progress and live capture retaining immediate archive writes.

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

Not applicable as a feature request: current main visibly waits for periodic repair. The earlier patch defect is addressed by the revised source and targeted regression tests; this review did not execute them.

Is this the best way to solve the issue?

Yes. Reusing serialized repair and preserving REST history progress addresses the earlier race without a second checkpoint format or changes to existing defaults.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 0f35f710441b.

Labels

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add 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 the ordinary contributor-proof gate. Supplied evidence exercises RunTail with temporary SQLite archives and fake Discord clients; it supports regression coverage but is not a live Discord run.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove merge-risk: 🚨 message-delivery: Current PR review selected no merge-risk labels.

Label justifications:

  • P2: This is a bounded, opt-in improvement that reduces history-recovery delays after capture restarts.
  • 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 the ordinary contributor-proof gate. Supplied evidence exercises RunTail with temporary SQLite archives and fake Discord clients; it supports regression coverage but is not a live Discord run.

Evidence

What I checked:

  • Previous finding resolved: The pinned earlier-head comparison confirms the new preserveHistoryCursor guard prevents live message creation from advancing history progress. Revised tests cover live arrival before readiness and during REST fetches at both zero and six-hour repair intervals, plus archive close/reopen recovery. (internal/syncer/tail.go:332, 9fac23d05a03)
  • Repair lifecycle and history ownership: Read RunTail, startup regressions, existing cancellation/serialization tests, and REST pagination. Startup waits for the ownership callback; periodic repair is suppressed until readiness and while repair is active. REST advances the existing cursor after persisting fetched pages. Exact-message replay is a separate CLI mode and is rejected with startup repair. (internal/syncer/startup_repair_test.go:26, 9fac23d05a03)
  • Current main still lacks startup catch-up: The pinned main implementation starts repairs only from its periodic timer and returns directly to Gateway capture when the interval is disabled. Standalone sync is documented, but does not supply the requested connected, same-owner startup lifecycle. (internal/syncer/tail.go:16, 0f35f710441b)
  • Release check: GitHub identifies v0.14.1, published September 9, as the latest release. Its tail implementation was inspected through the contents API and likewise has periodic-only repair without tailRepairOnStart. (internal/syncer/tail.go:16, 2aef26b2df8a)
  • Area history and routing: Main-branch history and GitHub commit patches connect hannesrudolph to monotonic message checkpoints and steipete to tail-client cleanup. These are routing signals, not claims about introduction of the reviewed behavior. (internal/syncer/tail.go, 0f35f710441b)
  • Scope and validation limits: The introduced diff has net production growth of 57 lines and test growth of 246 lines; dependencies, schemas and workflows are unchanged. Diff whitespace checks passed and the working tree remained clean. Tests were inspected, not executed. Some historical blobs were unavailable locally; pinned GitHub reads supplied the relevant comparisons. Broad PR search was unavailable, so related-item coverage uses the supplied context. (9fac23d05a03)

Likely related people:

  • hannesrudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • steipete: 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 (1 earlier review cycle)
  • reviewed 2026-09-11T21:58:41.693Z sha 4e0a68e :: blocked before merge. :: [P1] Preserve catch-up cursors before starting live capture

@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 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. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. labels Sep 11, 2026
@hannesrudolph
hannesrudolph merged commit 1907d9f into main Sep 11, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. 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.

1 participant