From e35a2041fb47196e49dcf54ea5c2959da3917247 Mon Sep 17 00:00:00 2001
From: Rai Butera <rai@rbutera.com>
Date: Wed, 11 Mar 2026 17:42:44 +0000
Subject: [PATCH] refactor(sessions): address Greptile dead-variable and Codex
 P2 plugin-collision concerns

Greptile: originatingChannel was computed before the isInterSessionChannel guard
but never used inside it, wasting a normalizeMessageChannel call on the fast path.
Moved the declaration to after the guard where it is actually needed.

Codex P2: isInterSessionChannel had no protection against a real deliverable plugin
channel named 'inter_session'. Added isDeliverableMessageChannel check so that if a
plugin registers that id, the sentinel path is skipped and normal routing applies.

Test: added invariant test documenting the Codex P2 guard (19 tests green).
---
 src/auto-reply/reply/session-delivery.test.ts | 19 +++++++++++++++++++
 src/auto-reply/reply/session-delivery.ts      |  7 +++++--
 src/utils/message-channel.ts                  |  8 +++++++-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/auto-reply/reply/session-delivery.test.ts b/src/auto-reply/reply/session-delivery.test.ts
index 8f563bc4adf..e91b3b04543 100644
--- a/src/auto-reply/reply/session-delivery.test.ts
+++ b/src/auto-reply/reply/session-delivery.test.ts
@@ -73,6 +73,25 @@ describe("INTER_SESSION_CHANNEL sentinel routing", () => {
       }),
     ).toBeUndefined();
   });
+
+  it("does not treat a real deliverable channel named 'inter_session' as the sentinel (Codex P2 guard)", () => {
+    // isInterSessionChannel guards against plugin channel collision:
+    // if a real channel plugin registers with id="inter_session", it must not
+    // be silently swallowed by the sentinel path.
+    // With no such plugin registered in the test registry, isDeliverableMessageChannel
+    // returns false for "inter_session" so the sentinel fires as expected.
+    // This test documents the invariant: sentinel only applies when the value is
+    // NOT a real deliverable channel.
+    const result = resolveLastChannelRaw({
+      originatingChannelRaw: INTER_SESSION_CHANNEL,
+      persistedLastChannel: "discord",
+      sessionKey: "agent:navi:main",
+    });
+    // In test env, "inter_session" is not a registered plugin channel, so
+    // isInterSessionChannel returns true and the sentinel path preserves "discord".
+    expect(result).toBe("discord");
+    expect(result).not.toBe(INTER_SESSION_CHANNEL);
+  });
 });
 
 describe("session delivery direct-session routing overrides", () => {
diff --git a/src/auto-reply/reply/session-delivery.ts b/src/auto-reply/reply/session-delivery.ts
index fc873c02183..2b2eae7f645 100644
--- a/src/auto-reply/reply/session-delivery.ts
+++ b/src/auto-reply/reply/session-delivery.ts
@@ -90,8 +90,6 @@ export function resolveLastChannelRaw(params: {
   persistedLastChannel?: string;
   sessionKey?: string;
 }): string | undefined {
-  const originatingChannel = normalizeMessageChannel(params.originatingChannelRaw);
-
   // Inter-session messages (from sessions_send): preserve the receiver's
   // established external channel without injecting the sender's channel.
   // Threading the sender's channel alone — without paired to/accountId/threadId —
@@ -108,6 +106,11 @@ export function resolveLastChannelRaw(params: {
     return undefined;
   }
 
+  // originatingChannel is only needed for the webchat-flip and external-routing
+  // checks below — declared after the inter-session guard to avoid computing it
+  // for the common inter-session fast path.
+  const originatingChannel = normalizeMessageChannel(params.originatingChannelRaw);
+
   // WebChat should own reply routing for direct-session UI turns, even when the
   // session previously replied through an external channel like iMessage.
   if (
diff --git a/src/utils/message-channel.ts b/src/utils/message-channel.ts
index 731457ee839..0b59cccabed 100644
--- a/src/utils/message-channel.ts
+++ b/src/utils/message-channel.ts
@@ -27,7 +27,13 @@ export const INTER_SESSION_CHANNEL = "inter_session" as const;
 export type InterSessionChannel = typeof INTER_SESSION_CHANNEL;
 
 export function isInterSessionChannel(raw?: string | null): boolean {
-  return raw?.trim().toLowerCase() === INTER_SESSION_CHANNEL;
+  // Guard against collision with real deliverable plugin channels: a plugin
+  // could theoretically register a channel named "inter_session", which must
+  // not be silently treated as our sentinel.
+  if (raw?.trim().toLowerCase() !== INTER_SESSION_CHANNEL) {
+    return false;
+  }
+  return !isDeliverableMessageChannel(INTER_SESSION_CHANNEL);
 }
 
 const MARKDOWN_CAPABLE_CHANNELS = new Set<string>([