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).
This commit is contained in:
Rai Butera 2026-03-11 17:42:44 +00:00
parent 45981e0e82
commit e35a2041fb
3 changed files with 31 additions and 3 deletions

View File

@ -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", () => {

View File

@ -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 (

View File

@ -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>([