(MOT-4073) refactor(workers): adopt engine recoverable triggers — remove hand-rolled pending-bind dances#523
Merged
andersonleal merged 4 commits intoJul 16, 2026
Conversation
…erable triggers The engine now parks a register_trigger for an unregistered trigger type as a pending intent and activates it when the type registers, re-parking and re-activating across harness restarts (recoverable triggers, iii #1962). Delete the web::on-registry-changed watcher, its two engine::*-available event bindings, the engine::triggers::list warm-start probe, the AtomicBool bind claim, and the explicit registry-changed wire schemas; bind the pre-generate guidance hook once at startup. Requires engine >= 0.21.8 — noted in the README (older engines silently drop the bind).
… recoverable triggers Keep the three harness hook binds (turn-completed → wake, pre-trigger → stamp-reply, pre-generate → inject-guidance) but drop the watching/claim machinery around them: the workflow::on-registry-changed watcher, its two engine::*-available event bindings, the engine::triggers::list readiness probe, and the AtomicBool bind claim. The engine parks binds to unregistered trigger types and activates them when the type registers, re-parking across harness restarts (recoverable triggers, iii #1962). Requires engine >= 0.21.8 — noted in the README (older engines silently drop the binds).
…e recoverable triggers Python port of the same cleanup: delete scrapling::on-registry-changed and its two engine::*-available bindings, the engine::triggers::list warm-start probe, the BindOnce claim, try_bind_sync/try_bind_async, and the registry-changed wire schemas; register_trigger the pre-generate hook once at startup. The engine parks the bind until the harness registers the type and re-parks across harness restarts (recoverable triggers, iii #1962). Requires engine >= 0.21.8 — noted in the README.
…e poll The delta-attribution retry loop (snapshot engine::triggers::info counts, re-request, confirm on increase) existed only because binds to not-yet-registered sibling trigger types were silently dropped. The engine now parks them as pending intents and activates them when the owner registers, re-parking across restarts (recoverable triggers, iii #1962), so the one-shot bind pass suffices for all four seams. queue::define is the one guarantee parking does not cover — it is an RPC the queue worker must serve before enqueue works — so keep a minimal poll for just that. Requires engine >= 0.21.8 — noted in the README.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
skill-check — worker0 verified, 44 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughWorkers now use one-shot harness hook bindings instead of readiness polling and registry-event retries. Memory separately retries extraction queue definition until successful, with updated documentation and focused scrapling tests. ChangesOne-shot harness bindings
Memory boot readiness
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scrapling/src/guidance.py`:
- Line 122: Update the log message in the guidance binding setup to state that
binding was requested rather than implying guidance injection is already active.
Preserve the existing log call and context while using wording such as “binding
requested.”
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b2a97211-9aab-4d1d-99a8-0828315a1b03
📒 Files selected for processing (11)
memory/README.mdmemory/src/main.rsscrapling/README.mdscrapling/src/guidance.pyscrapling/tests/test_guidance.pyweb/README.mdweb/src/configuration.rsweb/src/main.rsworkflow/README.mdworkflow/src/configuration.rsworkflow/src/main.rs
| # on_error fail_open is MANDATORY: pre_generate defaults fail-CLOSED, and a | ||
| # missing guidance line must never abort a turn. | ||
| iii.register_trigger({"type": HOOK_TRIGGER_TYPE, "function_id": HOOK_ID, "config": {"on_error": "fail_open"}}) | ||
| log.info("scrapling pre-generate hook bound (guidance injection active)") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Log the binding request rather than immediate activation.
On a cold start, Line 122 executes while the intent may still be parked, so “guidance injection active” is inaccurate until the harness registers its trigger type. Use wording such as “binding requested.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scrapling/src/guidance.py` at line 122, Update the log message in the
guidance binding setup to state that binding was requested rather than implying
guidance injection is already active. Preserve the existing log call and context
while using wording such as “binding requested.”
ytallo
approved these changes
Jul 16, 2026
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
2 participants
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.
Summary
Engine #1962 "recoverable triggers" (
15dc993eb) parks aregister_triggerwhose trigger type isn't registered yet and activates it automatically when the type registers, re-parking and re-activating across owner restarts. That makes every per-worker "watch the registry + probeengine::triggers::list+ AtomicBool claim + retry the bind" dance dead weight — including the latent AtomicBool bug where a harness restart never re-bound the hook.One commit per worker, −659/+97:
web::on-registry-changed+ its twoengine::*-availablebindings, the readiness probe, the claim flag, and the explicit registry-changed wire schemas;setup_harness_hooksis now onebind(...)call.BindOnce,try_bind_sync/async, warm-start probe,scrapling::on-registry-changedall gone); tests rewritten for the one-shot bind.retry_bindingsdelta-attribution loop; the existing one-shotbind_best_effortpass suffices for all four seams. Kept a minimalqueue::definepoll: enqueue rejects undefined queues (queue/src/runtime.rs), and trigger parking doesn't create queues.Audited, no change (legit ongoing registry subscriptions, not deferred binds): rbac-proxy catalog refresh, harness discovery, lsp completions.
Guard
Requires an engine built after
15dc993eb(≥ 0.21.8) — on older engines a one-shot bind regresses to the old silent drop. Noted in each touched worker's README; the worker manifest schema cannot express an engine floor. Review also surfaced an engine-side parity gap (bind dropped instead of parked when the registrator fails on the found-type path) — filed as MOT-4081.Verification
cargo fmt --check+clippy --all-targets -- -D warnings+cargo testgreen for web (102), workflow (137), memory (75);ruff+pytestgreen for scrapling (79).on-registry-changed/try_bindremnants outside legit subscriptions.15dc993ebengine:instance_count2→3).web::inject-guidanceexercised directly.Closes MOT-4073.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation