Skip to content

fix(discord): single-source thread-binding default placement and guard artifact parity#104342

Merged
steipete merged 2 commits into
mainfrom
claude/chanref-p0-artifact-parity
Jul 11, 2026
Merged

fix(discord): single-source thread-binding default placement and guard artifact parity#104342
steipete merged 2 commits into
mainfrom
claude/chanref-p0-artifact-parity

Conversation

@steipete

@steipete steipete commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Related: #104318

What Problem This Solves

Fixes a dual-surface drift hazard where Discord's lightweight thread-binding-api.ts artifact hardcoded defaultTopLevelPlacement = "child" inline while src/channel.ts independently hardcoded the same literal — two hand-maintained copies with nothing keeping them in sync. Matrix's channel.ts carried the same inline-literal hazard. Core genuinely consumes these artifacts on startup-safe paths (thread-bindings-policy, conversation-resolution), so drift would silently change pre-runtime placement behavior.

Why This Change Was Made

Single source of truth per channel (src/thread-binding-api.ts constant re-exported by the root artifact, consumed by the plugin object), matching the pattern matrix's artifact already used, plus a table-driven parity contract test that discovers every bundled channel shipping a thread-binding artifact and asserts artifact export === the real plugin's conversationBindings.defaultTopLevelPlacement. Contract test lanes enumerate files explicitly, so the new test is registered in the contracts lane configs.

User Impact

No behavior change today; prevents a class of silent placement drift between fast-path and runtime behavior.

Evidence

  • Parity contract suite (discord+matrix rows) 9/9 on Testbox tbx_01kx86hrxwddz38dczac6hq4pq; matrix thread-binding + discord channel tests 37/37 on tbx_01kx867d8v3gg6bvg8xh7aq99z; vitest-projects-config 16/16 on tbx_01kx86rb47zbtt0gfd0ncbm3kc; oxfmt clean on tbx_01kx86xq9bxzwhd009m366dyt2.
  • Autoreview (codex gpt-5.6-sol, xhigh): clean, no findings.
  • The old core test's resolveBundledChannelThreadBindingDefaultPlacement("discord") → undefined was a mock-fixture artifact, not vestigiality — fixture renamed; core loader genuinely consumes discord's artifact.

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord channel: matrix Channel integration: matrix scripts Repository scripts size: S maintainer Maintainer-authored PR labels Jul 11, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 11, 2026
@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 11, 2026, 5:18 AM ET / 09:18 UTC.

Summary
Centralizes Discord and Matrix top-level thread-binding placement values in plugin-owned source helpers and adds a bundled artifact-to-runtime-plugin parity contract.

PR surface: Source +8, Tests +68, Other +1. Total +77 across 8 files.

Reproducibility: yes. at source level: current main independently defines Discord's fast-path and runtime placement even though conversation resolution can consume either surface depending on plugin load state.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #104318
Summary: This PR is a bounded Phase 0 behavior-lock candidate within the broader channel-adapter refactor program.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🌊 off-meta tidepool
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Next step before merge

  • [P1] No repair is needed; the protected-label PR should continue through ordinary maintainer review and exact-head merge gates.

Security
Cleared: The patch introduces no dependency, secret, permission, download, publishing, package-resolution, or executable supply-chain changes.

Review details

Best possible solution:

Land the plugin-owned shared constants and public-surface parity contract after the remaining exact-head checks and normal protected-label maintainer review complete.

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

Yes at source level: current main independently defines Discord's fast-path and runtime placement even though conversation resolution can consume either surface depending on plugin load state.

Is this the best way to solve the issue?

Yes. A plugin-owned shared value plus a contract comparing the public artifact with the runtime plugin is the narrowest maintainable fix and follows the repository's existing Matrix and lightweight-artifact patterns.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 72cf43fa8092.

Label changes

Label changes:

  • add P3: This is low-risk preventive internal contract cleanup with no present user-visible behavior change.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The maintainer-labeled PR is exempt from the external-contributor proof gate; its body nevertheless provides exact Testbox IDs for the parity contract, adjacent channel tests, Vitest configuration checks, and formatting.

Label justifications:

  • P3: This is low-risk preventive internal contract cleanup with no present user-visible behavior change.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The maintainer-labeled PR is exempt from the external-contributor proof gate; its body nevertheless provides exact Testbox IDs for the parity contract, adjacent channel tests, Vitest configuration checks, and formatting.
Evidence reviewed

PR surface:

Source +8, Tests +68, Other +1. Total +77 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 4 12 4 +8
Tests 3 70 2 +68
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 1 0 +1
Total 8 83 6 +77

What I checked:

  • Current drift hazard: Current main independently hardcodes Discord's child placement in both the lightweight artifact and the loaded channel plugin, creating the duplication described by the PR. (extensions/discord/src/channel.ts:482, 72cf43fa8092)
  • Production consumer: Conversation resolution prefers the loaded plugin placement and falls back to the lightweight bundled artifact, so divergent values could change behavior according to plugin load state. (src/channels/conversation-resolution.ts:295, 72cf43fa8092)
  • Established sibling pattern: Matrix already owns its placement constant in the source helper and re-exports it through the top-level lightweight artifact; the PR also makes the runtime plugin consume that same value. (extensions/matrix/src/thread-binding-api.ts:5, 72cf43fa8092)
  • Proposed parity contract: The new test enumerates the canonical bundled-channel catalog, discovers artifacts through the shared public-surface loader, validates the closed placement values, and compares them with each loaded plugin contract. (src/channels/plugins/contracts/thread-binding-artifact.contract.test.ts:7, 94b9fb848975)
  • No touched-surface base drift: Current main has no intervening changes from the PR base on the modified artifact, channel, loader-test, contract-test, or lane-registration surfaces; the behind status is therefore advisory for this review. (72cf43fa8092)
  • Feature provenance: Blame and narrow file history attribute the current artifact loader, placement values, and conversation-resolution integration to Vincent Koc's recent commit. (src/channels/plugins/thread-binding-api.ts:1, e413d36054dd)

Likely related people:

  • vincentkoc: Current-main blame and narrow history attribute the lightweight artifact loader, Discord and Matrix placement contracts, and conversation-resolution integration to this recent contributor. (role: introduced behavior; confidence: high; commits: e413d36054dd; files: extensions/discord/src/channel.ts, extensions/discord/thread-binding-api.ts, extensions/matrix/src/channel.ts)
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.

@steipete
steipete merged commit d8bf51b into main Jul 11, 2026
98 checks passed
@steipete
steipete deleted the claude/chanref-p0-artifact-parity branch July 11, 2026 09:50
@steipete

steipete commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

vincentkoc added a commit that referenced this pull request Jul 11, 2026
@vincentkoc
Merge remote-tracking branch 'origin/main'
ba12348 
* origin/main:
  fix(cron): abort superseded reconciliation hooks (#104368)
  ci(release): expose Telegram runtime preflight stage (#104387)
  fix(status): avoid false shell-wrapper audit warnings (#81778)
  fix: retry live Gateway readiness proof (#104374)
  fix(exec-approval): stop misattributing Allow Always unavailability to policy (#97740)
  fix: preserve Mac bundle during live builds (#104376)
  chore(docs): translate with GPT-5.6 xhigh
  test: wire Control UI suite into CI, fix its broken tests, drop dead harness surface (#104361)
  fix(ui): hide group submenu separator when New group is the only entry (#104370)
  fix(discord): reset progress drafts across queued turns (#102341)
  docs(cli): clarify that exec-policy show and approvals get exclude per-session /exec overrides (#94999)
  fix(discord): single-source thread-binding default placement and guard artifact parity (#104342)
  fix(agents): add tool-activity heartbeat to keep subagent alive during tool calls (#95536)
  fix(maint): reuse recent same-PR hosted gates (#104355)
@github-actions github-actions Bot mentioned this pull request Jul 11, 2026
Open
@steipete steipete mentioned this pull request Jul 11, 2026
Merged
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 12, 2026
@steipete
fix(discord): single-source thread-binding default placement and guar…
98a1e4a 
…d artifact parity (openclaw#104342)

* fix(discord): single-source thread-binding default placement and guard artifact parity

* fix(tests): load thread-binding parity artifacts via sanctioned contract test helpers
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 12, 2026
@vincentkoc
Merge remote-tracking branch 'origin/main'
6c0c279 
* origin/main:
  fix(cron): abort superseded reconciliation hooks (openclaw#104368)
  ci(release): expose Telegram runtime preflight stage (openclaw#104387)
  fix(status): avoid false shell-wrapper audit warnings (openclaw#81778)
  fix: retry live Gateway readiness proof (openclaw#104374)
  fix(exec-approval): stop misattributing Allow Always unavailability to policy (openclaw#97740)
  fix: preserve Mac bundle during live builds (openclaw#104376)
  chore(docs): translate with GPT-5.6 xhigh
  test: wire Control UI suite into CI, fix its broken tests, drop dead harness surface (openclaw#104361)
  fix(ui): hide group submenu separator when New group is the only entry (openclaw#104370)
  fix(discord): reset progress drafts across queued turns (openclaw#102341)
  docs(cli): clarify that exec-policy show and approvals get exclude per-session /exec overrides (openclaw#94999)
  fix(discord): single-source thread-binding default placement and guard artifact parity (openclaw#104342)
  fix(agents): add tool-activity heartbeat to keep subagent alive during tool calls (openclaw#95536)
  fix(maint): reuse recent same-PR hosted gates (openclaw#104355)
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

channel: discord Channel integration: discord channel: matrix Channel integration: matrix maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. scripts Repository scripts size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete