Greptile Summary

This PR hardens Discord reads and message targeting by defaulting readMessages to a compact 20-message payload with diagnostics logging, removing the auto-inferred current-thread fallback for the read action, and rejecting legacy to/channelId params in favour of an explicit target field for all target-taking actions. It also adds a requireExplicitTarget gate in createMessageTool for send-family actions and improves channel-selection error messages when a caller accidentally passes a target ID in the channel field.

The logic is sound and the new test coverage is thorough. Two issues worth addressing:

• Misleading test names (P1): Three tests in message-action-runner.test.ts are named with "blocks ... when ..." language but their assertions confirm the operation succeeds (result.kind === \"send\" or \"action\"). Future developers reading these names will assume the cross-context policy already rejects these cases, which could lead to skipping real guards when tightening the policy.
• Redundant channelId read (P2): In the readMessages case of discord-actions-messaging.ts, resolveChannelId() and requestedTarget both call readStringParam(params, \"channelId\", { required: true }). Extracting the raw read before resolution makes the pre/post-resolution distinction explicit at a glance.

Confidence Score: 4/5

Safe to merge; the P1 concern is in test descriptions only with no runtime regression risk.

Core logic changes are well-structured and covered by tests. The misleading test names create long-term maintainability debt around the cross-context messaging policy but do not affect runtime behaviour.

src/infra/outbound/message-action-runner.test.ts (misleading test names at lines 239, 253, 310); src/agents/tools/discord-actions-messaging.ts (redundant read at lines 340-343).

This is a comment left during a code review.
Path: src/infra/outbound/message-action-runner.test.ts
Line: 239-251

Comment:
**Misleading test names assert success, not rejection**

Three tests use "blocks" in their name but assert the operation succeeds:

- Line 239: `"blocks send when target differs from current channel"` asserts `expect(result.kind).toBe("send")`
- Line 253: `"blocks thread-reply when channelId differs from current channel"` asserts `expect(result.kind).toBe("action")`
- Line 310: `"blocks $name send when target differs from current context"` asserts `expect(result.kind).toBe("send")`

The word "blocks" implies the operation should be rejected, but the assertions confirm it succeeds. A future developer reading these names will assume cross-context guards already exist when they do not. Suggested renames: `"allows same-provider send when target differs from current channel"`, `"allows same-provider thread-reply when target differs from current channel"`, and `"allows $name send when target differs from current context"`.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/tools/discord-actions-messaging.ts
Line: 340-343

Comment:
**`channelId` read twice in `readMessages` case**

`resolveChannelId()` on line 340 internally calls `readStringParam(params, "channelId", { required: true })`. Lines 341-343 call the same function again to populate `requestedTarget`. Consider extracting the raw read before resolution to make the pre/post distinction explicit:

```suggestion
      const requestedTarget = readStringParam(params, "channelId", {
        required: true,
      });
      const channelId = resolveDiscordChannelId(requestedTarget);
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Harden Discord message reads" | Re-trigger Greptile}

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

This PR is superseded by the narrower merged fix in #73521. Current main now addresses the Discord read/search hang and inbound hook session-key fallback without taking this PR's broader target-schema changes, and this PR's diff also carried extension-boundary and compatibility risks called out in review.

Best possible solution:

Close this PR as superseded by the merged #73521 fix. Keep the targeted main implementation for Discord read/search timeout, gateway routing, and session-key fallback; handle any future message-target API tightening separately with compatibility and extension-boundary review.

What I checked:

• Superseding commit on main: Current main contains merged commit e4ff7c1, fix: Discord read/search timeout, session-key fallback, and gateway execution mode (#73521), which is the maintainer-authored follow-up that explicitly superseded this approach in the provided related PR context. (e4ff7c162044)
• Discord REST timeout now lives in the request client: Current main defines a 15 second Discord REST timeout and injects AbortSignal.timeout(DISCORD_REST_TIMEOUT_MS) into the Discord request-client fetch wrapper, covering read/search REST calls at the HTTP layer. (extensions/discord/src/proxy-request-client.ts:6, e4ff7c162044)
• Read/search now route through gateway execution mode: Discord channel actions now return gateway execution mode for read and search, matching the defense-in-depth part of the superseding fix. (extensions/discord/src/channel-actions.ts:162, e4ff7c162044)
• Inbound hook session-key fallback is implemented: dispatchReplyFromConfig now falls back from SessionKey to CommandTargetSessionKey, so internal message:received hook emission is not skipped when Discord's primary session key is empty. (src/auto-reply/reply/dispatch-from-config.ts:295, e4ff7c162044)
• Regression coverage exists for the superseding fix: Current tests assert the Discord request client injects an abort timeout signal, including a hanging-request case. (extensions/discord/src/proxy-request-client.test.ts:5, e4ff7c162044)
• Current main deliberately keeps legacy target compatibility: The main branch still canonicalizes legacy to/channelId fields into target; that means this PR's broader breaking target-schema change was not adopted by the superseding fix. (src/infra/outbound/message-action-normalization.ts:48, c0ec58f4b690)

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Codex review notes: model gpt-5.5, reasoning high; reviewed against c0ec58f4b690; fix evidence: commit e4ff7c162044.