Codex review: needs real behavior proof before merge. Reviewed June 27, 2026, 11:10 AM ET / 15:10 UTC.

Summary
Adds a Mattermost channel-kind cache, uses it for inferTargetChatType and outbound session routing, reorders shared target chat-type inference to consult plugin hooks before channel: defaults, and adds regression tests.

PR surface: Source +75, Tests +219. Total +294 across 11 files.

Reproducibility: yes. for source-level reproduction: current main maps Mattermost private channel types P and G to group inbound, while outbound/session paths can still classify the same channel:<id> target as channel. I did not run a live Mattermost server.

Review metrics: 2 noteworthy metrics.

• Shared inference order: 2 call sites changed. Heartbeat target resolution and subagent announcement delivery now let plugin hooks override channel: and thread: defaults before merge.
• Mattermost channel-kind cache: 1 added, 30-minute TTL vs 5-minute source cache. The new synchronous cache can control session namespace selection longer than the authoritative channel lookup that feeds it.

Stored data model
Persistent data-model change detected: serialized state: extensions/mattermost/src/session-route.ts, unknown-data-model-change: extensions/mattermost/src/session-route.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95646
Summary: This PR is a candidate fix for the canonical Mattermost private-channel chat_type/session namespace mismatch; sibling PRs overlap, but none is merged and proof-positive enough to supersede this branch automatically.

Members:

• canonical: Mattermost: chat_type is non-deterministic for private channels (channel-type vs target-string inference); inferTargetChatType hook not implemented #95646 - The PR body closes this open issue, and the issue describes the same Mattermost private-channel target-string versus channel-type mismatch.
• fixed_by_candidate: fix(mattermost): key private channels as group on outbound routing #96645 - This newer open PR targets the same canonical issue with a narrower Mattermost-boundary approach, but it still needs real Mattermost proof.
• fixed_by_candidate: fix(mattermost): key resolved group targets under group: namespace (#95646) #95669 - This open sibling targets the same route namespace problem with a narrower partial fix, but prior review identified a public-channel regression risk.
• superseded: fix(mattermost): implement inferTargetChatType hook to resolve private channel chat_type #95655 - This closed unmerged sibling attempted the same private-channel chat_type/session mismatch and is no longer a viable landing target.
• superseded: fix(mattermost): keep private channels in group sessions across delivery paths #95656 - This closed unmerged sibling attempted the same root-cause area and is no longer a viable landing target.
• 1 more in the report.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

• Use authoritative channel type or current session identity before the warm cache fallback.
• Align or eliminate the channel-kind side cache lifetime mismatch.
• [P2] Add redacted real Mattermost proof for private-channel delivery and public-channel fallback, then update the PR body for re-review.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR body and comments provide unit, CI, and self-contained simulation proof only; contributor redacted logs, terminal output, recording, screenshot, or linked artifact from a real Mattermost private/public channel is still needed before merge, and updating the PR body should trigger a fresh ClawSweeper review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] The PR can still leave private-channel threaded or scheduled delivery in the channel:<id> namespace after process restart or before the monitor warms the new cache, so the central split-session bug is not fully closed.
• [P1] The 30-minute side cache can outlive the five-minute authoritative channel lookup and Mattermost public/private channel conversion, which can stale session namespace selection.
• [P1] The PR changes two shared channel:/thread: inference sites, so plugin hooks can override defaults outside Mattermost unless maintainers accept that compatibility impact.
• [P1] Contributor proof is unit/CI/self-contained simulation only; there is no redacted real Mattermost after-fix route or delivery proof.
• [P1] Multiple open PRs target Mattermost: chat_type is non-deterministic for private channels (channel-type vs target-string inference); inferTargetChatType hook not implemented #95646, so maintainers need to choose one canonical landing path before merging competing variants.

Maintainer options:

1. Repair cache identity before merge (recommended)
   Use authoritative channel type or current session identity before selecting group, align the cache lifetime with its source, and require live private/public Mattermost proof before landing.
2. Choose one canonical candidate
   Maintainers can select this PR or a sibling such as fix(mattermost): key private channels as group on outbound routing #96645, then pause or close the non-selected variants after proof is added.
3. Accept the broader inference change
   Maintainers may explicitly accept the shared hook-before-default behavior after checking sibling channel semantics and upgrade impact.

Next step before merge

• [P1] Human maintainer review is needed to choose the canonical same-issue PR and require contributor real Mattermost proof; the remaining action is not a safe automation-only repair lane.

Security
Cleared: The diff stays within Mattermost/outbound TypeScript and tests, with no dependency, workflow, secret, permission, or package-resolution changes.

Review findings

• [P1] Use current route evidence before the warm-cache fallback — extensions/mattermost/src/session-route.ts:35-36
• [P2] Tie the channel-kind cache to the source lookup lifetime — extensions/mattermost/src/mattermost/channel-kind-store.ts:11

Behavior proof — P1 fix: route resolver now propagates chatType: "group" for private-channel targets

Root cause (addressed in commits cdafc09 + e4234b1)

resolveMattermostOutboundSessionRoute was computing chatType solely from the target string prefix (isUser ? "direct" : "channel"), ignoring resolvedTarget.kind. A group-channel target with resolvedTarget.kind = "group" would produce chatType: "channel", causing the outbound session to land in the channel: namespace instead of the group: namespace used by the inbound classifier.

Corrected logic (session-route.ts):

const isGroup = resolvedKind === "group";

peer: {
  kind: isUser ? "direct" : "channel",   // ← Mattermost channels always use channel IDs
  id: rawId,
},
chatType: isUser ? "direct" : isGroup ? "group" : "channel",  // ← only chatType distinguishes group
from: isUser ? `mattermost:${rawId}` : `mattermost:channel:${rawId}`,
to:   isUser ? `user:${rawId}`       : `channel:${rawId}`,

Regression tests (session-route.test.ts — 8 tests total)

Two new assertions added that would have caught the original bug:

✓ resolves private-channel group target to chatType group [regression]
  → chatType = "group"      ✅
  → peer.kind = "channel"   ✅  (Mattermost channel ID routing unchanged)
  → from = "mattermost:channel:grp999"  ✅
  → to   = "channel:grp999"            ✅

✓ group outbound session uses channel routing but group session namespace [regression]
  → chatType ≠ "channel"    ✅  (no longer falls through to channel namespace)
  → from does not contain "mattermost:group:"  ✅

Before fix: chatType was "channel" for any non-user target regardless of resolvedTarget.kind.
After fix: chatType is "group" when resolvedTarget.kind === "group", keeping Mattermost's channel-ID routing intact.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

[P2] channel-kind-store.ts (commit: "Implement maximum cache size") — MAX_CHANNEL_KIND_CACHE_SIZE = 2_000 + FIFO eviction added.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Behavior proof — 25/25 assertions passed (Node.js self-contained routing simulation,
exit code 0). See script output above covering DM / public-O / private-P / cold-cache /
group-G+thread / thread-recovery scenarios. Cold-cache public channel correctly falls
back to chatType:channel even when directory emits kind:group.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Fixed check-test-types CI (f5a649c): removed excess to/source from resolvedTarget in tests. All 3 files correct. @clawsweeper re-review

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Closing this PR. The root-cause analysis is correct — Mattermost private channels get split across `group:` (inbound) and `channel:` (outbound) session namespaces — but the fix has two unresolved correctness issues and a proof blocker I can't clear without a live Mattermost environment:

1. Cold-restart regression (`session-route.ts:35-36`): After process restart the `peekMattermostChannelKind` cache is empty. Deliveries to existing `mattermost:group:🧵` sessions rebuild as `channel:` base routes, so thread recovery fails and the split-session bug persists. The fix needs to check `currentSessionKey` namespace or another authoritative source before falling back to the cache.

2. Cache lifetime mismatch (`channel-kind-store.ts`): The new 30-minute side cache outlives the 5-minute authoritative `resolveChannelInfo` lookup and can stale session namespace selection after a public/private channel conversion.

3. No live Mattermost proof: ClawSweeper requires redacted real-environment evidence for private/public channel routing before merge. No test environment available here.

For whoever picks this up: the cache-based approach in `session-route.ts` is on the right track — use `currentSessionKey` identity as a secondary signal before the cache fallback, align cache TTL with `resolveChannelInfo`, and validate with a real Mattermost workspace.

Closes #95646 remains open.