Skip to content

fix: prevent live-embedding shutdown delays during storage failures - #218

Merged
hannesrudolph merged 1 commit into
mainfrom
fix/crawlkit-worker-shutdown
Sep 11, 2026
Merged

fix: prevent live-embedding shutdown delays during storage failures#218
hannesrudolph merged 1 commit into
mainfrom
fix/crawlkit-worker-shutdown

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Sep 11, 2026

Copy link
Copy Markdown
Member
Additional instructions

Keep Allow edits from maintainers enabled.

Related: #217 and openclaw/crawlkit#117.

What Problem This Solves

Fixes an issue where users stopping tail --embed-live could wait over five minutes when cancellation interrupted saving retries and cleanup storage was unavailable. Discrawl inherited the worker shutdown defect in crawlkit v0.16.0.

Why This Change Was Made

Consume published crawlkit v0.16.1, which stops retry persistence after cancellation and shares one cleanup deadline across unfinished claims. The patch also bounds interrupted completion and rejected-batch cleanup. This dependency update requires no new database migration or configuration.

User Impact

Live embedding can shut down promptly during storage failures. Claims that cannot be released remain recoverable through their existing leases.

Evidence

  • The upstream regression reproduces 320 seconds before the fix and five seconds after at default limits, using a controlled clock; all 16 cases pass.
  • Crawlkit docs: align release closeout checklist #117 passed full tests, race tests, downstream compatibility, and ClawSweeper review with no outstanding findings.
  • Public Go proxy and module checksum verification passed; the downloaded worker source matches the reviewed fix byte-for-byte.
  • The full Discrawl race suite passed against published v0.16.1. The existing publishing fixture used Git init.defaultBranch=master to match Linux CI; no application workaround was added.
  • Vet and module tidy passed. Structured autoreview completed with no actionable findings.

@hannesrudolph
hannesrudolph requested a review from a team as a code owner September 11, 2026 21:29
@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. 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 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, 5:34 PM ET / 21:34 UTC.

ClawSweeper review

What this changes

Updates CrawlKit from v0.16.0 to v0.16.1 so live embedding shuts down promptly when cancellation interrupts storage operations, and adds an Unreleased note.

Merge readiness

Ready for maintainer review

This remains a useful, focused fix: current main still consumes the affected dependency, and the published upstream patch addresses the shutdown delay without changing APIs or storage formats. No blocking defect was found.

Priority: P2
Reviewed head: 2b8b8590ab162e11a6303ca4051c76e94d693409

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused dependency repair with inspected upstream regression coverage and no identified correctness or security blocker.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Supplied evidence covers the changed CrawlKit runner through controlled-clock storage-fault tests and reported downstream race tests, rather than a live Discrawl fault trace; no material authority change triggers additional proof.
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 external-contributor proof gate. Supplied evidence covers the changed CrawlKit runner through controlled-clock storage-fault tests and reported downstream race tests, rather than a live Discrawl fault trace; no material authority change triggers additional proof.
Evidence reviewed 7 items Verified introduced scope: The pinned delta changes only the CrawlKit version, its two checksums, and one changelog entry; it introduces no application configuration or schema change.
Concrete dependency and shutdown boundary: The store constructs CrawlKit's worker runner, and the CLI cancels it then waits on its completion channel. This directly establishes the dependency contract relevant to the review. Store operations use cancellation-aware database calls; final status persistence has its own one-second timeout.
Merged upstream fix and complete release delta: openclaw/crawlkit#117 is merged. The v0.16.0-to-v0.16.1 comparison contains one commit touching only worker implementation, regression tests, and release notes. Cancellation now stops retry persistence and uses the existing shared batch-cleanup deadline; exported APIs, dependency requirements, and stored schemas are unchanged.
Findings None None.
Security None None.

How this fits together

Discrawl captures Discord messages into a local archive and optionally processes their embeddings in background workers supplied by CrawlKit. Stopping capture cancels those workers and waits for their cleanup before returning.

flowchart TD
  A[Discord capture] --> B[Local embedding queue]
  B --> C[CrawlKit workers]
  C --> D[Embedding provider]
  D --> E[Store vectors]
  F[Stop capture] --> G[Cancel and bound cleanup]
  C --> G
  G --> H[Exit with unfinished claims recoverable]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

None.

Technical review

Best possible solution:

Consume the published upstream cancellation fix while retaining Discrawl’s existing lease recovery and bounded status persistence.

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

Yes, from source: v0.16.0 can start a fresh cleanup timeout for each remaining job after retry cancellation, while Discrawl waits for that runner. The upstream controlled-clock regression covers this path; no runtime reproduction was executed here.

Is this the best way to solve the issue?

Yes. Updating the shared worker dependency is the narrowest repair, and the complete upstream release delta preserves existing APIs, queue formats, and normal retry accounting.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 8f0e93b808ae.

Labels

Label changes:

  • add P2: Repairs a shutdown delay in optional live embedding when cancellation coincides with storage failure.
  • 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 external-contributor proof gate. Supplied evidence covers the changed CrawlKit runner through controlled-clock storage-fault tests and reported downstream race tests, rather than a live Discrawl fault trace; no material authority change triggers additional proof.

Label justifications:

  • P2: Repairs a shutdown delay in optional live embedding when cancellation coincides with storage failure.
  • 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 external-contributor proof gate. Supplied evidence covers the changed CrawlKit runner through controlled-clock storage-fault tests and reported downstream race tests, rather than a live Discrawl fault trace; no material authority change triggers additional proof.

Evidence

What I checked:

  • Verified introduced scope: The pinned delta changes only the CrawlKit version, its two checksums, and one changelog entry; it introduces no application configuration or schema change. (go.mod:11, 2b8b8590ab16)
  • Concrete dependency and shutdown boundary: The store constructs CrawlKit's worker runner, and the CLI cancels it then waits on its completion channel. This directly establishes the dependency contract relevant to the review. Store operations use cancellation-aware database calls; final status persistence has its own one-second timeout. (internal/cli/embedding_worker.go:56, 2b8b8590ab16)
  • Merged upstream fix and complete release delta: fix: prevent minute-long worker shutdown during retry cancellation crawlkit#117 is merged. The v0.16.0-to-v0.16.1 comparison contains one commit touching only worker implementation, regression tests, and release notes. Cancellation now stops retry persistence and uses the existing shared batch-cleanup deadline; exported APIs, dependency requirements, and stored schemas are unchanged. (worker/worker.go:357, 0bb18e9865a2)
  • Dependency policy and release availability: Read the complete dependency AGENTS.md: preserve downstream compatibility and consume published patch tags after verification. GitHub confirms v0.16.1 was published at 2026-09-11T21:31:49Z with the bounded-shutdown fix. (AGENTS.md, 0bb18e9865a2)
  • Still necessary on main: Read-only GitHub checks show current main still selects CrawlKit v0.16.0; Discrawl v0.14.1 selects v0.15.0. The upstream release therefore does not itself resolve dependency consumption in Discrawl. (go.mod:11, 8f0e93b808ae)
  • Supplemental regression evidence: The upstream test adds 16 controlled-clock cases for retry/completion cancellation, accepted/superseded/error outcomes, and rejected batches with available or unavailable cleanup storage. The upstream body reports 320 seconds before versus five seconds after for interrupted retry persistence. These are test results, not a live Discrawl storage-fault trace; this review did not execute tests. (worker/worker_test.go:638, 0bb18e9865a2)

Likely related people:

  • Hannes Rudolph: Raw commit 8f0e93b adds internal/cli/embedding_worker.go:53 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 8f0e93b808ae; files: internal/cli/embedding_worker.go)

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.

@hannesrudolph
hannesrudolph merged commit 66fbb8f into main Sep 11, 2026
22 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