Skip to content

fix(matrix): respect is_direct: false in isStrictDirectRoom#85172

Open
JulyanXu wants to merge 1 commit into
openclaw:mainfrom
JulyanXu:fix/matrix-strict-direct-is-direct-false
Open

fix(matrix): respect is_direct: false in isStrictDirectRoom#85172
JulyanXu wants to merge 1 commit into
openclaw:mainfrom
JulyanXu:fix/matrix-strict-direct-is-direct-false

Conversation

@JulyanXu

@JulyanXu JulyanXu commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

isStrictDirectRoom in extensions/matrix/src/matrix/direct-room.ts returned evidence.strict directly without checking memberStateFlag. When a 2-person Matrix room has is_direct: false on the membership event, the function still classified it as a strict DM, causing requireMention to be silently bypassed for rooms configured under groups[].

This fix checks evidence.memberStateFlag === false before returning true, ensuring the explicit is_direct: false signal overrides the 2-member heuristic.

Note: There is a related gap in monitor/direct.ts where the m.direct cache hit path (line 232-238) also bypasses the is_direct: false check. This PR addresses only the isStrictDirectRoom function used by verification-events.ts and targets.ts. A follow-up PR should address the isDirectMessage gap.

Closes #85017

Verification

  • Single file change: extensions/matrix/src/matrix/direct-room.ts
  • inspectMatrixDirectRoomEvidence already reads hasDirectMatrixMemberFlag and stores it in memberStateFlag — this fix simply uses the value that was already being fetched
  • The isDirectMessage path in monitor/direct.ts already handles is_direct: false at line 252-254 — this brings isStrictDirectRoom to parity

Real behavior proof

  • Behavior addressed: 2-person Matrix rooms with is_direct: false and requireMention: true in groups[] still get classified as DMs, bypassing mention requirements
  • Real environment tested: Source-level verification against current main (9f2c0a8)
  • Exact steps: Create 2-person Matrix room with is_direct: false, add to groups[] with requireMention: true → after fix, isStrictDirectRoom returns false when memberStateFlag === false
  • Evidence after fix: requireMention config is respected for 2-person rooms that explicitly set is_direct: false
  • What was not tested: Live Matrix gateway with patch applied

@JulyanXu
fix(matrix): respect is_direct: false in isStrictDirectRoom
a5ed4e5 
isStrictDirectRoom returned evidence.strict directly without
considering memberStateFlag. When a 2-person room has
is_direct: false on the membership event, the function still
classified it as a strict DM, causing requireMention to be
silently bypassed for rooms configured under groups[].

Now checks memberStateFlag === false before returning true,
ensuring explicit is_direct: false signals override the
2-member heuristic.

Closes openclaw#85017
@openclaw-barnacle openclaw-barnacle Bot added channel: matrix Channel integration: matrix size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 22, 2026
@clawsweeper

clawsweeper Bot commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR changes extensions/matrix/src/matrix/direct-room.ts so isStrictDirectRoom returns false when Matrix direct-room evidence has memberStateFlag === false.

Reproducibility: yes. source-level: current isStrictDirectRoom returns .strict even when existing evidence records memberStateFlag: false for the bot membership event. I did not run a live Matrix reproduction in this read-only review.

PR rating
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Summary: The implementation is small and plausible, but the PR is not quality-ready because after-fix real behavior proof is missing.

Rank-up moves:

  • Add redacted live Matrix gateway proof, logs, terminal output, or a recording showing the patched is_direct: false behavior; redact IPs, tokens, phone numbers, homeserver-private data, and other private details.
  • Update the PR body after adding proof so ClawSweeper re-runs automatically; if it does not, ask a maintainer to comment @clawsweeper re-review.
  • Add or point to focused regression coverage for isStrictDirectRoom returning false when memberStateFlag === false.
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.

Real behavior proof
Needs real behavior proof before merge: The PR body provides source-level verification only; contributor should add redacted live Matrix output, logs, terminal proof, or a recording and update the PR body to trigger re-review.

Risk before merge

Maintainer options:

  1. Prove and cover the false-veto path (recommended)
    Before merge, add focused regression coverage for isStrictDirectRoom with local is_direct: false and redacted live Matrix proof or logs showing the intended after-fix behavior.
  2. Accept source-only proof
    Maintainers can choose to merge the source-level helper fix without live proof, but that knowingly bypasses the external PR real-behavior proof gate.
  3. Retarget or close as superseded
    If the only intended goal is the already-closed requireMention report, close this PR as superseded by Fix Matrix configured two-person room routing #85137 and track any isStrictDirectRoom cleanup separately.

Next step before merge
The remaining blocker is contributor real-behavior proof and maintainer scope review, not a safe ClawSweeper repair job.

Security
Cleared: The diff narrows Matrix strict-DM classification and adds no dependency, workflow, credential, or supply-chain surface.

Review details

Best possible solution:

Land the narrow helper change only after redacted real Matrix proof and focused regression coverage, while leaving #85137 as the canonical fix for the linked requireMention issue.

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

Yes, source-level: current isStrictDirectRoom returns .strict even when existing evidence records memberStateFlag: false for the bot membership event. I did not run a live Matrix reproduction in this read-only review.

Is this the best way to solve the issue?

Mostly yes: reusing memberStateFlag in isStrictDirectRoom is the narrow owner-boundary fix for this helper. It still needs live proof and regression coverage, and the PR body should not claim to fix the already-landed requireMention path.

Label changes:

  • add P2: This is a focused Matrix routing bug fix with limited affected scope; the broader linked P1 requireMention issue is already fixed on main.
  • add merge-risk: 🚨 compatibility: Merging changes how existing two-person Matrix rooms with local is_direct: false are classified by isStrictDirectRoom, which can alter send and verification routing.
  • add rating: 🧂 unranked krab: Current PR rating is 🧂 unranked krab because proof is 🧂 unranked krab, patch quality is 🐚 platinum hermit, and The implementation is small and plausible, but the PR is not quality-ready because after-fix real behavior proof is missing.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides source-level verification only; contributor should add redacted live Matrix output, logs, terminal proof, or a recording and update the PR body to trigger re-review.

Label justifications:

  • P2: This is a focused Matrix routing bug fix with limited affected scope; the broader linked P1 requireMention issue is already fixed on main.
  • merge-risk: 🚨 compatibility: Merging changes how existing two-person Matrix rooms with local is_direct: false are classified by isStrictDirectRoom, which can alter send and verification routing.
  • rating: 🧂 unranked krab: Current PR rating is 🧂 unranked krab because proof is 🧂 unranked krab, patch quality is 🐚 platinum hermit, and The implementation is small and plausible, but the PR is not quality-ready because after-fix real behavior proof is missing.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides source-level verification only; contributor should add redacted live Matrix output, logs, terminal proof, or a recording and update the PR body to trigger re-review.

Acceptance criteria:

  • node scripts/run-vitest.mjs extensions/matrix/src/matrix/direct-room.test.ts
  • node scripts/run-vitest.mjs extensions/matrix/src/matrix/send/targets.test.ts
  • node scripts/run-vitest.mjs extensions/matrix/src/matrix/monitor/direct.test.ts

What I checked:

Likely related people:

  • joshavant: Authored the current-main Matrix configured two-person room routing fix that closed the linked requireMention issue and added the direct-tracker configured-room veto. (role: recent area contributor; confidence: high; commits: 1f9ebb9dda36; files: extensions/matrix/src/matrix/monitor/direct.ts, extensions/matrix/src/matrix/monitor/index.ts, extensions/matrix/src/matrix/monitor/direct.test.ts)
  • w-sss: Co-authored the merged Matrix direct-room change that introduced the local memberStateFlag evidence now used by this PR. (role: introduced related direct-room behavior; confidence: medium; commits: 2b2edaa01db5; files: extensions/matrix/src/matrix/direct-room.ts, extensions/matrix/src/matrix/direct-management.ts, extensions/matrix/src/matrix/direct-room.test.ts)
  • gumadeiras: Co-authored and reviewed the merged Matrix direct-room classification change that shaped the current is_direct contract. (role: reviewer and adjacent contributor; confidence: medium; commits: 2b2edaa01db5; files: extensions/matrix/src/matrix/direct-room.ts, extensions/matrix/src/matrix/direct-management.ts, extensions/matrix/src/matrix/monitor/direct.ts)

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

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 22, 2026
@clawsweeper

clawsweeper Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

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: matrix Channel integration: matrix merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

1 participant

@JulyanXu