feat(agent): route a Slack event to the run that owns the channel - #193
Open
github-actions[bot] wants to merge 21 commits into
Open
feat(agent): route a Slack event to the run that owns the channel#193github-actions[bot] wants to merge 21 commits into
github-actions[bot] wants to merge 21 commits into
Conversation
Proves the load-bearing assumption behind the customer-onboarding flow: an event that arrives from outside the CRM can wake a run that is already parked, rather than starting a second one. The mechanism already existed and was not being used this way. dispatchAgentRun sends with `continuationToken: runToken(run.id)`, and per docs/agent.md eve hands a continuation token back only when the session is parked and will accept another turn. So resuming is the same send with the same token. resumeAgentRun is that send, with the guards a webhook needs, because a Slack event arrives whenever Slack feels like it: - a finished run is never restarted by a late event - a run with no session yet is left alone - an agent that is no longer LIVE is refused - an unknown run is ignored, not thrown - a refused send is an outcome, not an exception - the run's own status is never touched; the runner owns that Nine integration tests cover each. It does not decide which run an event belongs to: AgentRun has no slackChannelId, and adding one is a schema decision rather than spike material. Not wired to anything yet. The Slack Events endpoint, the channel-to-run lookup and the new action types are the next steps, and they are ordinary work now that this holds.
Closes the gap the spike left open. An inbound event knows a channel id; it needs a run id. - AgentRun gains slackChannelId, indexed with status, so the lookup is one query rather than a scan. - runOnSlackChannel returns only a live run, so a finished run's channel stops routing and a late event lands nowhere. - claimSlackChannel writes the channel once. A run cannot be reassigned, so two channels cannot both point at the same run. - The newest live run wins when a channel is genuinely reused. Adds verifySlackSignature in @crm/auth. Slack's events endpoint is a public POST, so the signature is the only thing standing between a stranger and resuming somebody's run. It fails closed with no secret, refuses a body changed by one byte, refuses another secret's signature, and refuses a replay outside the five-minute window in either direction. timingSafeEqual, not ===. Migration written by hand and verified against a throwaway Postgres: every migration applied, then `migrate diff` reports no difference. The local database could not author it because it carries the HubSpot migration from another branch. 25 tests. Still not wired to an HTTP route.
Adds the inbound half. AgentRun gains slackChannelId so an event that knows a channel can find the run that owns it; slackEventInbox is the landing table, keyed on Slack's event_id so a redelivery is a no-op rather than a second resume. @crm/validation/slack-events parses the envelope and answers the only two questions the ingest needs: is this from us, and is it worth waking an agent for. Both matter — a bot_message that woke the agent would have it answering its own post, forever. Migration generated by `prisma migrate dev` against a scratch database, not written by hand. The local crm database carries another branch's migration, which is why migrate dev refused to author against it; a throwaway database is the way round that, not a hand-rolled file. crm_test was rebuilt: it held a failed record from the hand-written migration this replaces. 17 validation tests, 14 resume tests. Still no HTTP route.
POST /webhooks/slack/events. Answers Slack's setup handshake, verifies every other request, writes an inbox row and pokes the agent. The API decides nothing, per the rule in AGENTS.md: it stores the event and lets the agent work out which run it belongs to and what it means. That also happens to be what keeps the handler inside Slack's three second budget. Refuses by default. With no SLACK_SIGNING_SECRET the endpoint rejects everything rather than trusting the caller, because it is a public POST and the signature is the only thing between a stranger and resuming somebody's run. Answers 200 to a payload it cannot parse, to an event type we do not act on, and to a redelivery, so Slack stops retrying instead of hammering a shape we will never handle. Ignores anything from our own bot; without that the agent answers its own posts forever. The raw body is collected by a small middleware on that path alone. The app runs with bodyParser false and express is not one of its declared dependencies, so importing express.raw would have broken createApp at runtime — as it did, until the tracking-collector spec caught it. 10 endpoint tests, driven by signed fixtures. No Slack workspace needed to run them.
Closes the loop. A stored event finds the run that owns its channel and resumes it on that run's own continuation token, so the agent carries on from where it parked rather than starting again. Every event settles, including the ones that go nowhere. An event whose channel owns no live run is marked processed with a reason, not left to be retried forever. A row already processed is skipped, so a second drain after a redelivery does nothing. Both are the difference between an inbox and a backlog. The drain hangs off POST /internal/crm/dispatch, which the API already pokes on every stored event. It is deliberately not routed through the channel's receive: receive must return a session, and an event that resumes nothing has none to give. app_mention joins the actionable set. It arrives alongside message.channels when the bot is in the channel, so it changes nothing today, but a mention in a channel the bot has not joined is exactly how a customer asks for help. describe() is what the agent actually reads. It names the channel, the user and the text, and truncates at 2000 characters so one pasted log cannot fill a turn. 9 integration tests.
open_slack_channel is the first half of the onboarding flow. A deployed agent names the channel in plain words, gets a tidied Slack name, and the run claims the channel so every later message and join wakes that same run. Without this nothing ever sets AgentRun.slackChannelId and the inbox resumes nobody. A name already in use gives back the existing channel instead of an error, so a retried run lands in the channel it made the first time. claimSlackChannel now says which channel the run watches. The claim is write-once, so a run that opens a second channel used to be told it succeeded while its events went elsewhere. The tool reports the truth. app_mention is deduplicated against message. Slack sends both for one human sentence when the bot is in the channel, which resumed the run twice for one thing said once. A unique index on (channelId, messageTs) stops the second at the door, and the agent is told it was mentioned rather than spoken to. 19 tests across validation, the API and the agent.
Closes the standing gap. conversations.connect:write has been requested from every Slack workspace since the connection shipped and nothing used it, so a customer could never reach the channel the agent made. One tool covers both kinds of person. An address Slack already knows is added with conversations.invite. An address it does not know gets a Slack Connect invitation, and the tool hands back the invitation link. The agent does not have to know which somebody is. already_in_channel counts as invited, so re-running the flow is quiet rather than an error. A lookup failure that is not a missing person stops the invitation instead of falling through to Connect, because "reconnect Slack" and "this person is external" need different answers. Every Slack call now goes through one caller in slack-api.ts, which owns the timeout, the rate-limit retry and the parse. Four hand-rolled fetches in slack-membership.ts went with it. 6 tests.
Slack stores its request URL once. A quick cloudflared tunnel invents a new hostname every restart, so the stored URL goes stale and delivery stops with no error anywhere: the endpoint is simply never called again. tunnel:slack runs a named tunnel. It creates the tunnel if it is missing, points the DNS record at it, prints the request URL and runs it. Re-running is safe, and the hostname never changes, so Slack is configured once. The script refuses clearly rather than half-working: no hostname, no cloudflared, and not signed in each say what to do next. docs/setup.md also records that Socket Mode swallows event delivery while still showing the request URL as Verified. That cost an afternoon.
The tools to open a channel and invite people existed with nothing telling an agent the order to use them in, so the one ordering that matters was left to chance: the channel must be opened with open_slack_channel first, because a channel opened any other way is a channel the run does not watch, and every later reply is lost. The skill also says that waiting is the work. An agent that treats a parked run as an unfinished job polls, reschedules, or reports success that has not happened. A Slack Connect invitation takes a person a day to accept, and the run is supposed to sit there. retireExhausted now joins a once-evaluated subquery, the same shape claimDue already used, instead of IN (SELECT ... LIMIT ...). The planner is free to re-execute a sublink, so the row cap was a request rather than a guarantee. Two queries doing the same job now read the same way.
…ustomer The first live test found the real gap: a deployed run executes inside the agent_runner subagent, which has its own sandboxed tool list, so the root-level tools were never reachable. The run identified the deal and the buyer, then wrote a summary because it had nothing to act with. Copying the tools down would have skipped the discipline every other external action follows. Opening a channel and inviting people are now manifest-approved actions with AgentAction rows, claimed and settled by idempotency key, so a retried run rejoins the channel it already made instead of making a second one and inviting the customer twice. Three guards move failure earlier, where somebody can fix it: - A manifest with a Slack action but no slack:workspace resource no longer parses. It used to deploy and then fail on every single run. - AGENT_ACTION_EXECUTORS and AGENT_ACTION_DEPENDENCIES are exhaustive over AgentActionType, so a new action cannot ship without a tool and a connection requirement. Both refused to compile until they were filled in, and the builder's draft schema had to learn the actions too. - DraftAction was a hand-written twin of its own Zod schema and had already drifted. It is now inferred from the schema. The event chain is verified end to end against the running agent: a closed deal queues the task, the EVENT trigger matches, and the run is created. The second run stops at the dependency preflight with "Connect Slack in Settings → Connections", which is correct — Slack has never been connected in this workspace, and the guard refuses before spending a model call. Test cleanup deletes AgentAction rows before runs. Without that the agent definition survived, the user delete failed on its restricted foreign key, and six orphaned users broke an unrelated auth spec. 7 tests.
A customer channel with only the customer in it is not a channel anybody uses. The person who closed the deal has to be there from the start, and asking the model to remember that would make it optional. The run's input already names the record the event fired for, so the deal is known without asking the agent for it. SlackMemberMatch turns the CRM owner into a Slack user id, and the owner is invited with conversations.invite, which works on a free workspace. Slack Connect does not, so the customer half still needs a paid plan. An owner Slack cannot match is reported, not thrown. The channel is already open by then, and losing it to a missing account would leave a real Slack channel with no run watching it. Run input is parsed with a schema rather than read out of the Json column by hand.
…rive Slack refuses to save an app manifest that subscribes to app_mention without this scope: "app_mention event is missing scope(s)". The event was already in the actionable set, so a mention in a channel was meant to wake a parked run and silently could not. Found by creating the staging app from our own scope list. The manifest would not validate.
Collaborator
CRM-13Persist the provider reply on CRM-13 commits: Files
How to verify
Tests (actual)
|
The history contract and the run store both parse the same shape. Naming it in two places lets them drift. One export, used on write and every read.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
17 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/auth/src/slack-scopes.ts">
<violation number="1" location="packages/auth/src/slack-scopes.ts:36">
P3: Adding `app_mentions:read` makes the documented scope count incorrect. Update `docs/connections.md` to describe the current 17-scope catalog.</violation>
</file>
<file name="apps/agent/agent/lib/slack-api.ts">
<violation number="1" location="apps/agent/agent/lib/slack-api.ts:30">
P2: When Slack or an intermediary returns a non-2xx response with a valid-looking success envelope, `read` reports the operation as successful because it ignores `response.ok`. Require an HTTP-success response before returning `ok: true` (while retaining the rate-limit handling).</violation>
</file>
<file name="packages/validation/src/slack-events.ts">
<violation number="1" location="packages/validation/src/slack-events.ts:49">
P2: When a user sends only the app mention, `isActionable` resumes the run despite no request text. Strip Slack mention tokens before checking for remaining text.</violation>
</file>
<file name="apps/agent/agent/lib/slack-events.ts">
<violation number="1" location="apps/agent/agent/lib/slack-events.ts:88">
P2: When a batch contains multiple events for one channel, `Promise.all` resumes the same run concurrently and can deliver a later Slack message before an earlier one. Process events for each run serially in `receivedAt` order.</violation>
</file>
<file name="docs/setup.md">
<violation number="1" location="docs/setup.md:70">
P2: This section implies the named tunnel is all inbound Slack needs, but without SLACK_SIGNING_SECRET in the API process the events endpoint refuses every request, including Slack's initial URL verification. Add a step to set SLACK_SIGNING_SECRET (the Signing Secret from the Slack app's Basic Information page), or the reader reproduces the exact silent-failure failure mode the section warns about.</violation>
</file>
<file name="apps/agent/test/slack-events.integration.spec.ts">
<violation number="1" location="apps/agent/test/slack-events.integration.spec.ts:220">
P3: The "drains every pending event" test asserts only `resumed >= 2`, so it never verifies that its own three events were fully drained or that the C-nobody event was settled (processedAt set). Because drainSlackEvents reads every unprocessed inbox row in the shared DB with no team filter, the count also mixes unrelated rows, forcing the weak inequality. Assert the exact expected resume count and assert the C-nobody event has processedAt set so the test actually proves the drain behavior.</violation>
</file>
<file name="apps/agent/agent/lib/run-runtime.ts">
<violation number="1" location="apps/agent/agent/lib/run-runtime.ts:143">
P2: When an invite call has multiple addresses, replaying its idempotency key drops the `invited` and `refused` arrays and preserves only one selected invite. Persist the complete outcome set or reconstruct those fields on replay.</violation>
<violation number="2" location="apps/agent/agent/lib/run-runtime.ts:1160">
P2: When a live channel is already owned by a newer run, this reports `watching: true` for an older run that cannot receive its events. Compute `watching` from the actual routed owner or reject reuse by another live run.</violation>
<violation number="3" location="apps/agent/agent/lib/run-runtime.ts:1161">
P2: When the automatic deal-owner invitation throws, this catch silently marks channel opening successful without inviting the owner. Preserve the failure or return an explicit failed owner outcome so the run can retry or report it.</violation>
</file>
<file name="apps/agent/test/slack-invite.integration.spec.ts">
<violation number="1" location="apps/agent/test/slack-invite.integration.spec.ts:39">
P2: These tests depend on the spec's own slack account being the only (or the most recently updated) slack account in the DB, because slackAccessToken() picks the newest slack account globally. Test 6 deletes only the fixed ACCOUNT_ID and asserts 'Slack is not connected.', so any other slack account left in a shared test DB (or created in parallel by the other slack `.integration.spec` files in this directory) makes it flaky, and the success-path tests can select the wrong account. Use a unique per-run id (e.g. crypto.randomUUID() suffixed, as slack-channel-actions.integration.spec.ts does) instead of fixed constants.</violation>
</file>
<file name="apps/agent/agent/subagents/agent_builder/lib/draft-input.ts">
<violation number="1" location="apps/agent/agent/subagents/agent_builder/lib/draft-input.ts:59">
P1: When a builder request needs Slack channel onboarding, this schema accepts `slack.channel.open` and `slack.channel.invite`, but the builder instructions still list only three executable action types. Update the builder prompt and integration guidance so the model can emit and save these capabilities.</violation>
</file>
<file name="apps/api/src/slack/slack-events.controller.ts">
<violation number="1" location="apps/api/src/slack/slack-events.controller.ts:61">
P2: When the signed body is not valid JSON, `JSON.parse(body || "null")` throws a SyntaxError and the endpoint returns 500, so Slack retries the delivery. The endpoint elsewhere fails open with 200 on shapes it cannot read. Wrap the parse so malformed JSON also returns `{ ok: true }` (or an explicit 200) instead of throwing.</violation>
</file>
<file name="apps/agent/agent/lib/agent-actions.ts">
<violation number="1" location="apps/agent/agent/lib/agent-actions.ts:11">
P2: When a run declares `slack.channel.open` or `slack.channel.invite` but never calls it, the required-action failure is persisted as a CRM action. Classify all Slack action types as `provider: "slack"` in `requiredActionFailure` so the action ledger retains the correct provider.</violation>
</file>
<file name="scripts/slack-tunnel.sh">
<violation number="1" location="scripts/slack-tunnel.sh:11">
P2: An inline `.env` comment becomes part of `hostname`. Cloudflare rejects the DNS route. Strip comments before routing.</violation>
</file>
<file name="apps/agent/agent/lib/run-resume.ts">
<violation number="1" location="apps/agent/agent/lib/run-resume.ts:124">
P2: When `channelId` is blank, `claimSlackChannel` persists an empty Slack channel ID even though routing treats blank IDs as absent. Reject blank IDs before `updateMany` so malformed channel responses cannot create inconsistent, non-routable ownership.</violation>
</file>
<file name="apps/agent/agent/lib/slack-membership.ts">
<violation number="1" location="apps/agent/agent/lib/slack-membership.ts:203">
P2: When inventory has retired an old row with this name, the `name_taken` fallback can return that stale channel instead of the visible channel. Restrict this lookup to `available: true`.</violation>
<violation number="2" location="apps/agent/agent/lib/slack-membership.ts:240">
P1: When Slack reuses an existing channel, this early return skips joining the bot to it. Join the returned channel and propagate a failed `JoinOutcome` before reporting the channel as opened.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| if (!parsed.success) | ||
| return { error: "Slack sent back something unreadable." }; | ||
| if (!outcome.ok) { | ||
| if (outcome.error === NAME_TAKEN) return channelNamed(name); |
Contributor
There was a problem hiding this comment.
P1: When Slack reuses an existing channel, this early return skips joining the bot to it. Join the returned channel and propagate a failed JoinOutcome before reporting the channel as opened.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/slack-membership.ts, line 240:
<comment>When Slack reuses an existing channel, this early return skips joining the bot to it. Join the returned channel and propagate a failed `JoinOutcome` before reporting the channel as opened.</comment>
<file context>
@@ -238,25 +229,23 @@ export async function createSlackChannel(
- if (!parsed.success)
- return { error: "Slack sent back something unreadable." };
+ if (!outcome.ok) {
+ if (outcome.error === NAME_TAKEN) return channelNamed(name);
+ return { error: explain(outcome.error) };
+ }
</file context>
| }), | ||
| }), | ||
| z.object({ | ||
| type: z.literal(AGENT_ACTION_TYPES.SLACK_CHANNEL_OPEN), |
Contributor
There was a problem hiding this comment.
P1: When a builder request needs Slack channel onboarding, this schema accepts slack.channel.open and slack.channel.invite, but the builder instructions still list only three executable action types. Update the builder prompt and integration guidance so the model can emit and save these capabilities.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/subagents/agent_builder/lib/draft-input.ts, line 59:
<comment>When a builder request needs Slack channel onboarding, this schema accepts `slack.channel.open` and `slack.channel.invite`, but the builder instructions still list only three executable action types. Update the builder prompt and integration guidance so the model can emit and save these capabilities.</comment>
<file context>
@@ -55,8 +55,20 @@ const action = z.discriminatedUnion("type", [
}),
}),
+ z.object({
+ type: z.literal(AGENT_ACTION_TYPES.SLACK_CHANNEL_OPEN),
+ provider: z.literal("slack"),
+ summary: z.string().trim().min(1).max(240),
</file context>
| } | ||
|
|
||
| const envelope = schemas.slackEvents.slackEnvelope.safeParse( | ||
| JSON.parse(body || "null"), |
Contributor
There was a problem hiding this comment.
P2: When the signed body is not valid JSON, JSON.parse(body || "null") throws a SyntaxError and the endpoint returns 500, so Slack retries the delivery. The endpoint elsewhere fails open with 200 on shapes it cannot read. Wrap the parse so malformed JSON also returns { ok: true } (or an explicit 200) instead of throwing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/slack/slack-events.controller.ts, line 61:
<comment>When the signed body is not valid JSON, `JSON.parse(body || "null")` throws a SyntaxError and the endpoint returns 500, so Slack retries the delivery. The endpoint elsewhere fails open with 200 on shapes it cannot read. Wrap the parse so malformed JSON also returns `{ ok: true }` (or an explicit 200) instead of throwing.</comment>
<file context>
@@ -0,0 +1,90 @@
+ }
+
+ const envelope = schemas.slackEvents.slackEnvelope.safeParse(
+ JSON.parse(body || "null"),
+ );
+
</file context>
| sensitive: false, | ||
| }, | ||
| { | ||
| scope: "app_mentions:read", |
Contributor
There was a problem hiding this comment.
P3: Adding app_mentions:read makes the documented scope count incorrect. Update docs/connections.md to describe the current 17-scope catalog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/auth/src/slack-scopes.ts, line 36:
<comment>Adding `app_mentions:read` makes the documented scope count incorrect. Update `docs/connections.md` to describe the current 17-scope catalog.</comment>
<file context>
@@ -32,6 +32,12 @@ export const SLACK_SCOPES: readonly SlackScope[] = [
sensitive: false,
},
+ {
+ scope: "app_mentions:read",
+ group: "read",
+ grant: "See messages that mention it, so somebody can ask it for help",
</file context>
|
|
||
| const resumed = await drainSlackEvents(send); | ||
|
|
||
| expect(resumed).toBeGreaterThanOrEqual(2); |
Contributor
There was a problem hiding this comment.
P3: The "drains every pending event" test asserts only resumed >= 2, so it never verifies that its own three events were fully drained or that the C-nobody event was settled (processedAt set). Because drainSlackEvents reads every unprocessed inbox row in the shared DB with no team filter, the count also mixes unrelated rows, forcing the weak inequality. Assert the exact expected resume count and assert the C-nobody event has processedAt set so the test actually proves the drain behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/test/slack-events.integration.spec.ts, line 220:
<comment>The "drains every pending event" test asserts only `resumed >= 2`, so it never verifies that its own three events were fully drained or that the C-nobody event was settled (processedAt set). Because drainSlackEvents reads every unprocessed inbox row in the shared DB with no team filter, the count also mixes unrelated rows, forcing the weak inequality. Assert the exact expected resume count and assert the C-nobody event has processedAt set so the test actually proves the drain behavior.</comment>
<file context>
@@ -0,0 +1,272 @@
+
+ const resumed = await drainSlackEvents(send);
+
+ expect(resumed).toBeGreaterThanOrEqual(2);
+ });
+});
</file context>
Collaborator
Tex first pass fail (localhost
|
| Case | persisted | invite_id | url | replay |
|---|---|---|---|---|
| Connect | yes, SUCCEEDED |
yes (I0BT7FVJPFH) |
missing | same invite_id |
| Member | yes, SUCCEEDED |
none (expected) | none | n/a |
Slack conversations.inviteShared defaults external_limited=true, which omits url. inviteGuest posts { channel, emails } and never sets external_limited: false. So the provider reply has no url to persist. Unit tests mock a url Slack did not send.
UI
apps/app/components/agent-builder/agent-history.tsx actionReceipt:
- links only when
result.urlexists - otherwise dumps
externalId
Live SSR/history for the new run: invite id in the payload, no join.slack.com / result.url. Row cannot show invite_id and a clickable url.
Old Tripwire invite rows still have result: null (pre-persist). Receipt is the channel id.
Done looks like
- Connect invite stores
invite_idand a url (likelyexternal_limited: falseon inviteShared, unless Slack still withholds it). - Timeline row shows both: invite_id text and a clickable url.
- Member invite still succeeds with no url.
- Replay returns the same ids.
Mac Chrome screenshot of the live page was blocked from this environment (localhost is on the laptop; box desktop cannot see it). Evidence is live REST + SSR, not a pixel shot. Put before/after on this PR once the row actually renders both.
conversations.inviteShared defaults external_limited to true, so Slack omits url. The request now sets external_limited false. The timeline shows invite_id and the url Slack returned.
Collaborator
Tex retest pass (
|
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/agent/agent/lib/slack-invite.ts">
<violation number="1" location="apps/agent/agent/lib/slack-invite.ts:73">
P2: For conversations.inviteShared, external_limited defaults to true (limited invite) and Slack only returns url/conf_code when it is false, so false is the correct value to obtain the invite URL. But Slack's docs state that when the workspace disallows fully-shared invites, external_limited=false returns restricted_action, so any workspace that previously received a working limited invite will now fail the whole action instead. The code already surfaces that as a failure via explain, but consider confirming this hard-fail-on-restricted_action tradeoff is intended for workspaces that only permit limited invites.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| const outcome = await slackPost( | ||
| token, | ||
| "conversations.inviteShared", | ||
| { channel: channelId, emails: [email], external_limited: false }, |
Contributor
There was a problem hiding this comment.
P2: For conversations.inviteShared, external_limited defaults to true (limited invite) and Slack only returns url/conf_code when it is false, so false is the correct value to obtain the invite URL. But Slack's docs state that when the workspace disallows fully-shared invites, external_limited=false returns restricted_action, so any workspace that previously received a working limited invite will now fail the whole action instead. The code already surfaces that as a failure via explain, but consider confirming this hard-fail-on-restricted_action tradeoff is intended for workspaces that only permit limited invites.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/slack-invite.ts, line 73:
<comment>For conversations.inviteShared, external_limited defaults to true (limited invite) and Slack only returns url/conf_code when it is false, so false is the correct value to obtain the invite URL. But Slack's docs state that when the workspace disallows fully-shared invites, external_limited=false returns restricted_action, so any workspace that previously received a working limited invite will now fail the whole action instead. The code already surfaces that as a failure via explain, but consider confirming this hard-fail-on-restricted_action tradeoff is intended for workspaces that only permit limited invites.</comment>
<file context>
@@ -70,7 +70,7 @@ async function inviteGuest(
token,
"conversations.inviteShared",
- { channel: channelId, emails: [email] },
+ { channel: channelId, emails: [email], external_limited: false },
schemas.slack.inviteShared,
);
</file context>
… an event A public Slack POST was unbounded, a failed inbox write looked like a duplicate, and two drains could resume the same row.
The events path now uses Express raw middleware. Without Express as a direct dependency, createApp cannot start in tests.
Collaborator
Tex retest pass (
|
Contributor
There was a problem hiding this comment.
2 issues found across 16 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/test/slack-events.spec.ts">
<violation number="1" location="apps/api/test/slack-events.spec.ts:187">
P2: This test asserts on the source text of create-app.ts (via Bun.file) rather than any runtime behavior, so it passes even when the body cap is misconfigured and breaks on any unrelated formatting or refactor. Exercise the real endpoint instead: build the app, POST an over-limit body to SLACK_EVENTS_PATH, and assert a 413, plus a normal payload that still verifies the signature.</violation>
</file>
<file name="apps/api/src/agent/agent-runs.service.ts">
<violation number="1" location="apps/api/src/agent/agent-runs.service.ts:450">
P2: `listedActionResult` swallows every parse failure and returns null, so a result that fails `readAgentActionResult` (an action type missing from the validation registry, stale/corrupt JSON, or a type mismatch) is silently dropped from run history. The list response cannot then distinguish "no result" from "result we could not parse," which hides exactly the invite URLs and other details this PR is meant to surface, and leaves no trace that data is being lost. Log the caught error (for example a warning) before returning null so malformed stored results are visible instead of silently disappearing.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| }); | ||
|
|
||
| describe("the Slack events body cap", () => { | ||
| it("uses Express raw middleware with an explicit size limit, then verifies", async () => { |
Contributor
There was a problem hiding this comment.
P2: This test asserts on the source text of create-app.ts (via Bun.file) rather than any runtime behavior, so it passes even when the body cap is misconfigured and breaks on any unrelated formatting or refactor. Exercise the real endpoint instead: build the app, POST an over-limit body to SLACK_EVENTS_PATH, and assert a 413, plus a normal payload that still verifies the signature.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/test/slack-events.spec.ts, line 187:
<comment>This test asserts on the source text of create-app.ts (via Bun.file) rather than any runtime behavior, so it passes even when the body cap is misconfigured and breaks on any unrelated formatting or refactor. Exercise the real endpoint instead: build the app, POST an over-limit body to SLACK_EVENTS_PATH, and assert a 413, plus a normal payload that still verifies the signature.</comment>
<file context>
@@ -182,3 +182,17 @@ describe("the Slack events endpoint", () => {
});
+
+describe("the Slack events body cap", () => {
+ it("uses Express raw middleware with an explicit size limit, then verifies", async () => {
+ const source = await Bun.file(
+ new URL("../src/create-app.ts", import.meta.url),
</file context>
| } | ||
| } | ||
|
|
||
| function listedActionResult(type: string, value: Prisma.JsonValue | null) { |
Contributor
There was a problem hiding this comment.
P2: listedActionResult swallows every parse failure and returns null, so a result that fails readAgentActionResult (an action type missing from the validation registry, stale/corrupt JSON, or a type mismatch) is silently dropped from run history. The list response cannot then distinguish "no result" from "result we could not parse," which hides exactly the invite URLs and other details this PR is meant to surface, and leaves no trace that data is being lost. Log the caught error (for example a warning) before returning null so malformed stored results are visible instead of silently disappearing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/agent/agent-runs.service.ts, line 450:
<comment>`listedActionResult` swallows every parse failure and returns null, so a result that fails `readAgentActionResult` (an action type missing from the validation registry, stale/corrupt JSON, or a type mismatch) is silently dropped from run history. The list response cannot then distinguish "no result" from "result we could not parse," which hides exactly the invite URLs and other details this PR is meant to surface, and leaves no trace that data is being lost. Log the caught error (for example a warning) before returning null so malformed stored results are visible instead of silently disappearing.</comment>
<file context>
@@ -446,3 +446,11 @@ export class AgentRunsService {
}
}
+
+function listedActionResult(type: string, value: Prisma.JsonValue | null) {
+ try {
+ return readAgentActionResult(type, value);
</file context>
Collaborator
Tex post-copy pass (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A Slack Connect invitation is stored as a typed AgentAction result, so replay, history, and the timeline keep
invite_idandurl.Problem
The invite existed only as a Slack side-effect. A retried call could not prove Slack already sent the invitation. History returned an untyped JSON blob. The timeline had no invite link.
Solution
AgentAction.resultis a Prisma JSON column.@crm/validation/agent-actionis the only read and write path.The invite result is flat:
type,invite_id,url,email,kind.externalIdis Slack'sinvite_idwhen Slack returns one, otherwise the channel id.Other actions keep the fields they already had:
slack.message.post—channel,tsslack.channel.open—channelIdcrm.activity.create—activityIdFiles
packages/validation/src/agent-action.tspackages/validation/src/index.tspackages/validation/package.jsonpackages/validation/test/agent-action.spec.tspackages/db/prisma/schema.prismapackages/db/prisma/migrations/20260830083000_agent_action_result/migration.sqlapps/agent/agent/lib/run-runtime.tsapps/agent/agent/lib/slack-invite.tsapps/agent/agent/lib/slack-api.tsapps/api/src/agent/agents.contracts.tsapps/api/src/agent/agent-runs.service.tsapps/app/components/agent-builder/agent-history.tsxapps/agent/test/slack-invite.integration.spec.tsapps/agent/test/slack-channel-actions.integration.spec.tsHow to verify
bun run db:testsocrm_testhasagentAction.result.invite_idI1andurl, then replays the same call and keeps both.bun run lint:slop. Oxlint must report nothing.Commands and results
Ran in
packages/validation. 7 pass, 0 fail. 20.00ms.6 pass, 0 fail. Includes
invite_id: "I1". 285.00ms.8 pass, 0 fail. AgentAction row and replay keep
invite_idI1 and url. 349.00ms.$ oxlint— exit 0, no findings.Pre-push on
70da103:bun run check-types— 13 successfulbun run lint— turbo lint successfulbun run lint:slop— oxlint, no findingsbun run test— 10 successful, 10 total