Skip to content

docs(channel-inbound): document per-inbound cfg re-resolve contract for live bindings#90852

Closed
caster-Q wants to merge 1 commit into
openclaw:mainfrom
caster-Q:docs/channel-routing-cfg-contract
Closed

docs(channel-inbound): document per-inbound cfg re-resolve contract for live bindings#90852
caster-Q wants to merge 1 commit into
openclaw:mainfrom
caster-Q:docs/channel-routing-cfg-contract

Conversation

@caster-Q

@caster-Q caster-Q commented Jun 6, 2026

Copy link
Copy Markdown

Summary

What problem does this PR solve?

The "channel plugins must re-resolve routing per inbound against a fresh runtime cfg" contract is real and load-bearing in current main — bindings/agents/routing are deliberately kind: "none" in the reload plan, and resolve-route.ts caches evaluated bindings in a WeakMap<OpenClawConfig, ...> keyed on cfg object identity. But this contract is not written down anywhere a channel plugin author would find it, so authors keep re-discovering it the hard way (shipped regression → bug report → ~80-line per-event fix).

Why does this matter now?

Three plugins in or adjacent to this tree have independently re-rolled the same fix: telegram inlined it from the start, qqbot shipped without it and recovered via #73567 (closing #69546), and at least one third-party channel plugin is currently broken because of the same omission. Each repeat costs a real regression and a small ActiveCfgProvider-shaped patch. Writing the contract down once breaks the cycle for the next plugin author.

What is the intended outcome?

A channel plugin author reading docs/plugins/sdk-channel-inbound.md learns the per-inbound getRuntimeConfig() contract before shipping, instead of after. Future readers of config-reload-plan.ts:120-140 and resolve-route.ts:205 find inline comments pointing back to the doc, so neither source-level surprise drops a channel author into a stale-routing debug session.

What is intentionally out of scope?

What does success look like?

Merged docs that a channel author can find via the existing sdk-channel-inbound.md reading order, plus two inline source comments that survive future refactors of the reload planner and the routing WeakMap.

What should reviewers focus on?

  • Is docs/plugins/sdk-channel-inbound.md the right home for this section, or would you prefer it in sdk-runtime.md, a new channel-routing.md, or split across both?
  • The inline comments on config-reload-plan.ts:120-140 and resolve-route.ts:205 — phrasing acceptable, or too prescriptive?
  • Naming of getRuntimeConfig import path — used openclaw/plugin-sdk/runtime-config-snapshot per existing sdk-runtime.md guidance; confirm that is the canonical name.

Linked context

Which issue does this close?

Refs #90842 (does not auto-close; this is the docs-only "Option A" direction from that issue, and maintainers may also want the SDK helper "Option B" before closing).

Which issues, PRs, or discussions are related?

Was this requested by a maintainer or owner?

No — this is a contributor proposal in response to ClawSweeper's review on #90842 (fix-shape-clear + needs-maintainer-review). Filed as draft so maintainers can shape the direction (or close in favor of a different placement / wording) before any merge.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: Channel plugin authors capture the start-time cfg reference and reuse it inside long-lived inbound handlers, causing CLI binding edits to be silently ignored until gateway restart.
  • Real environment tested: macOS 15.x (Darwin 25.3.0), OpenClaw main at commit c1ddfccf65c9cf3708771b356090b7331c543f58 at time of investigation.
  • Exact steps or command run after this patch: This is a docs-only patch. The behavior being documented is reproducible at source level per ClawSweeper's own review on [Feature]: Document and/or centralize the per-event cfg re-resolve contract for channel plugins #90842 — they trace bindings/routing reload classification, the WeakMap<OpenClawConfig, EvaluatedBindingsCache> cache, and the qqbot ActiveCfgProvider consumer pattern; that timeline carries the clawsweeper:source-repro label.
  • Evidence after fix: ClawSweeper review on [Feature]: Document and/or centralize the per-event cfg re-resolve contract for channel plugins #90842 confirmed both the bug shape and the docs-fix-shape ("fix-shape-clear" label). The diff in this PR is the doc text plus two short cross-link comments; rendered output not attached because the change is plain Markdown without new MDX components.
  • Observed result after fix: Cross-links between sdk-channel-inbound.md → config-reload-plan.ts:120-140 → resolve-route.ts:205 form a closed loop so a reader landing on either source file is pointed back to the doc.
  • What was not tested: Did not run pnpm docs:check-mdx locally — dependencies are not installed in the fork checkout and the run requires pnpm install --frozen-lockfile first. Relying on CI to catch any MDX render breakage; happy to install deps and re-verify if CI flags anything.
  • Proof limitations or environment constraints: No live runtime repro is feasible for a docs-only change. The source-level repro that motivates the doc is already on the linked issue under clawsweeper:source-repro. If a maintainer wants the live-route repro for their own records, the steps are in [Feature]: Document and/or centralize the per-event cfg re-resolve contract for channel plugins #90842's "Real environment we tested in" section.

Tests and validation

Which commands did you run?

  • git diff --stat — confirmed the change is 3 files, +57 / -0 (docs + two small source comments, no code paths altered).
  • grep for broken cross-links — verified docs/plugins/sdk-channel-inbound.md references resolve (sdk-runtime.md, config-reload-plan.ts, resolve-route.ts, extensions/telegram/..., extensions/qqbot/...) and that the source-side cross-references to docs/plugins/sdk-channel-inbound.md "Routing and live bindings" anchor match the section header.
  • Did not run the project's full test suite — this PR introduces no executable code; CI is the right surface for any docs-render or link-check validation.

What regression coverage was added or updated?

None — docs-only. The behavior being documented already has source-level evidence on #90842 and a merged unit-test analogue at extensions/qqbot/src/engine/gateway/stages/access-stage.test.ts (added in #73567), which already locks in the routing layer's per-event re-resolve invariant.

@caster-Q
docs(channel-inbound): document per-inbound cfg re-resolve contract f…
cf27251 
…or live bindings

CLI binding edits (openclaw agents bind / agents add) must take effect on
the next inbound message without a gateway restart. The reload planner
already classifies `bindings`, `agents`, and `routing` as `kind: "none"`
by design, and resolve-route.ts caches evaluated bindings in a WeakMap
keyed on cfg object identity. The contract is that channel plugins
re-resolve routing per inbound against a fresh runtime cfg.

That contract was load-bearing but undocumented in the channel author
guide. Telegram inlined it from the start; qqbot shipped without it,
regressed in production, and added an ActiveCfgProvider helper in
openclaw#73567 to close openclaw#69546; third-party channel plugins are repeating the
same regression-then-fix cycle.

This commit adds a "Routing and live bindings" section to
sdk-channel-inbound.md spelling out the contract and pointing at the
two in-tree reference implementations, plus inline cross-link comments
on the two source-of-truth declarations (BASE_RELOAD_RULES_TAIL and the
WeakMap cache) so future readers there land on the doc.

Refs: openclaw#90842
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime size: XS labels Jun 6, 2026
@caster-Q caster-Q mentioned this pull request Jun 6, 2026
Open
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label Jun 6, 2026
@clawsweeper

clawsweeper Bot commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 6, 2026, 12:45 AM ET / 04:45 UTC.

Summary
Review failed before ClawSweeper could summarize the requested change.

PR surface: Source +13, Docs +44. Total +57 across 3 files.

Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path.

Review metrics: none identified.

Merge readiness
Overall: 🌊 off-meta tidepool
Proof: 🌊 off-meta tidepool
Patch quality: 🌊 off-meta tidepool
Result: rating does not apply to this item.

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

Risk before merge

  • [P1] No close action taken because the review did not complete.

Maintainer options:

  1. Decide the mitigation before merge
    Retry the Codex review after fixing the execution failure.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Review did not complete, so no work-lane recommendation was made.
Review details

Best possible solution:

Retry the Codex review after fixing the execution failure.

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

Unclear. The review failed before ClawSweeper could establish a reproduction path.

Is this the best way to solve the issue?

Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction.

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

Codex review notes: model gpt-5.5, reasoning high; reviewed against 9313471fa579.

Label changes

Label changes:

  • add rating: 🌊 off-meta tidepool: Overall readiness is 🌊 off-meta tidepool; proof is 🌊 off-meta tidepool and patch quality is 🌊 off-meta tidepool.

Label justifications:

  • rating: 🌊 off-meta tidepool: Overall readiness is 🌊 off-meta tidepool; proof is 🌊 off-meta tidepool and patch quality is 🌊 off-meta tidepool.
Evidence reviewed

PR surface:

Source +13, Docs +44. Total +57 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 13 0 +13
Tests 0 0 0 0
Docs 1 44 0 +44
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 57 0 +57

What I checked:

  • failure reason: codex execution failed.
  • codex failure detail: Codex review failed for this PR with exit 1.
  • codex stdout: Per-item Codex failure; continuing with the rest of the shard.

Likely related people:

  • unknown: Codex failed before it could trace repository history. (role: review did not complete; confidence: low)
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.

@clawsweeper clawsweeper Bot added the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label Jun 6, 2026
@caster-Q

caster-Q commented Jun 6, 2026

Copy link
Copy Markdown
Author

Closing this PR — the docs guidance is based on an incorrect understanding of current runtime semantics.

The fact I missed: in 2026.5.28, getRuntimeConfig() is a literal alias for loadConfig(), which by default returns loadPinnedRuntimeConfig(...) — the same pinned reference until something explicitly calls setRuntimeConfigSnapshot(). Channel plugins calling getRuntimeConfig() per inbound (the pattern this PR documented) therefore receive the same OpenClawConfig ref on every call and hit the same WeakMap<OpenClawConfig, ...> entry in resolve-route.ts:205 — no routing refresh actually happens.

The only path that swaps the snapshot is applySnapshot in server-reload-handlers, which returns early when isNoopReloadPlan(plan) && !followUp.requiresRestart. Because bindings/agents/routing are all kind: "none", any CLI binding-only edit produces a noop plan → applySnapshot returns → setRuntimeConfigSnapshot is never called → routing stays stale until a non-noop config write (e.g. anything under channels.<id>.*) or a gateway restart.

That makes the docs in this PR misleading: it would tell a channel author the per-inbound getRuntimeConfig() call is sufficient, when in practice it does nothing on its own.

Replaced the analysis on #90842 with a corrected root-cause section (## Correction block at the top of the issue body) pointing at the real fix surface in applySnapshot / reload-plan classification. Will open a new PR only after maintainer direction on that.

Sorry for the noise.

@caster-Q caster-Q closed this Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

docs Improvements or additions to documentation gateway Gateway runtime rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@caster-Q