feat(slack): handle global and message shortcuts#94881
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 9:04 AM ET / 13:04 UTC. Summary Reproducibility: yes. for the source-level gap: current main and v2026.6.8 have block-action and modal handlers but no Slack shortcut listener. The PR fix itself still lacks live Slack invocation proof. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land a rebased Slack-focused implementation after falling back to the selected message timestamp for root-message shortcuts, adding regression coverage, and attaching live Slack proof or an explicit maintainer proof override. Do we have a high-confidence way to reproduce the issue? Yes for the source-level gap: current main and v2026.6.8 have block-action and modal handlers but no Slack shortcut listener. The PR fix itself still lacks live Slack invocation proof. Is this the best way to solve the issue? No, not yet: the Slack plugin is the right owner boundary, but the implementation needs selected-message timestamp fallback, focused regression coverage, and maintainer acceptance of the generic matcher scope. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ce7f89916522. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2477da066c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isMessageShortcut && !channelId) { | ||
| params.ctx.runtime.log?.( | ||
| `slack:interaction drop shortcut callback=${callbackId} user=${userId} reason=missing-channel`, | ||
| ); |
There was a problem hiding this comment.
Route message shortcuts to the selected message thread
When a message shortcut is invoked on a normal channel message, Slack’s payload carries the selected message timestamp as message.ts/message_ts, while message.thread_ts is absent; Slack’s own Bolt shortcut guide uses that timestamp as the thread_ts for replies (https://docs.slack.dev/interactivity/implementing-shortcuts/). Because this line only reads message.thread_ts, ordinary message shortcuts enqueue with threadId: undefined and resolve the channel-level session, so any agent reply/tool delivery goes to the whole channel instead of the selected message’s thread. Use the selected message timestamp as the fallback thread id for message shortcuts.
Useful? React with 👍 / 👎.
|
Blacksmith runners detected OOM events on the following jobs:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c94074b2b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| releaseInFinally = false; | ||
| return responseWithRelease(response, release); |
There was a problem hiding this comment.
Release guarded Graph responses for zero-length bodies
When Graph returns a successful non-null status with Content-Length: 0, readOptionalGraphJson returns without reading the body; since releaseInFinally is disabled here and responseWithRelease only calls the SSRF guard's release after the wrapped stream is drained or cancelled, the guarded dispatcher/timeout remains open. The same leak happens for success callers such as deleteGraphRequest that ignore the returned Response; treat zero-length responses like null-body statuses or make void callers drain/cancel the returned body.
Useful? React with 👍 / 👎.
|
Land-ready verification for
Known gap: no live Slack app shortcut invocation; signed/authenticated request, ACK timing, routing, and redaction are covered through the HTTP handler tests. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1483d2fb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let appGroupStateDirURL { | ||
| return appGroupStateDirURL | ||
| } |
There was a problem hiding this comment.
Migrate legacy iOS identity before switching directories
When an existing iOS install upgrades with the new App Group entitlement, containerURL(forSecurityApplicationGroupIdentifier:) will start returning a shared container even though the previous device.json / device-auth.json still live under Application Support. Because this now selects the empty App Group directory before the legacy directory, DeviceIdentityStore.loadOrCreate(profile: .primary) generates a new device id and DeviceAuthStore stops seeing the saved device token, so already-paired apps can no longer reconnect without re-pairing. Prefer migrating/copying the legacy identity/auth files into the App Group on first launch before making this the primary location.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
Summary
Closes #63920.
Release-note context: Slack apps can now expose configured global and message shortcuts to agents as structured interaction events.
Verification
node scripts/run-vitest.mjs extensions/slack/src/monitor/events/interactions.test.ts extensions/slack/src/monitor/context.test.ts— 52 passedpnpm tsgo:extensionspnpm tsgo:extensions:testpnpm check:changed— passed (run_a7002d4ea5b4,cbx_d0ab4caaa906)Real behavior proof
Behavior addressed: Slack global and message shortcut payloads are acknowledged, authorized, routed, sanitized, and queued as structured
Slack interaction:events.Real environment tested: macOS Node test environment plus Azure Crabbox Linux (
Standard_D32ads_v6).Exact steps or command run after this patch:
node scripts/run-vitest.mjs extensions/slack/src/monitor/events/interactions.test.ts extensions/slack/src/monitor/context.test.ts; remoteenv OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 corepack pnpm check:changed.Evidence after fix: 52 focused tests passed; extension production/test typechecks passed; remote changed gate passed all selected extension/docs checks.
Observed result after fix: global shortcuts target the actor's direct delivery/session context; message shortcuts retain channel, selected message, and thread context; policy-rejected or mismatched payloads are acknowledged and dropped; sensitive Slack fields do not reach agent context.
What was not tested: end-to-end invocation from a live Slack app with configured shortcut callback IDs; no suitable live app configuration was available in this task.