feat(twenty-slack): unfurl Twenty record links pasted in Slack - #24108
feat(twenty-slack): unfurl Twenty record links pasted in Slack#24108abdulrahmancodes wants to merge 8 commits into
Conversation
Subscribe to link_shared events and expand pasted record links (/object/<objectNameSingular>/<recordId>) into compact cards via chat.unfurl: the record name plus a few key fields per object type (people, companies, opportunities, notes, tasks). - Route link_shared through slack-events-resolver to a new slack-link-unfurl logic function running in the resolved workspace - Only unfurl links on the workspace's own base URL, cap lookups at 5 per message, and skip unresolvable or inaccessible records silently - Add links:read / links:write scopes, the link_shared bot event and an unfurl domain placeholder to the Slack app manifest - Grant the app function role read-only access to the CRM objects the Slack Assistant role can read, so unfurls never expose more than the assistant itself - Document setup (unfurl domain, re-authorization for existing installs) and behaviour in SETUP.md / README.md; bump app to 0.5.0 (0.4.0 is claimed by #23985 and 0.3.1 by #24097)
|
👋 Thanks for contributing to Twenty! Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run. Looking forward to your contribution! |
✅ Standard review · no findings
High-level — Additive, reconnect-gated Slack unfurl feature; sub-1000 non-test lines, no migration, and the read-only role widening (human-answered) mirrors the assistant role's existing exposure — no high-level findings. Reviewed against the |
![twenty-ci-bot-public[bot]](https://avatars.githubusercontent.com/u/119600397?s=60&v=4){kind=small}
| } | ||
|
|
||
| try { | ||
| const client = new CoreApiClient() as unknown as CoreRecordQueryClient; |
There was a problem hiding this comment.
🟡 Nit · Low-level · typing — no as casts
new CoreApiClient() as unknown as CoreRecordQueryClient plus Record<string, any>/record: any defeat typing
The standard bans as unknown as and any in favour of a real type, mapper, or Pick<>; the accompanying comment justifies but doesn't exempt it. Consider a thin typed wrapper around the generated client's query surface so the cast and the any records disappear.
There was a problem hiding this comment.
The as unknown as and the any-typed records are gone since 74e47e3 — the fetcher now uses per-object typed record shapes (PersonUnfurlRecord etc.) with typed per-object fetchers. One plain as cast remains, in the single coreQuery helper, and it is irreducible today: the generated CoreApiClient ships as an untyped stub at build time (its schema is generated per workspace on install), so any typed wrapper would contain exactly this cast — moving it, not removing it. The cast site carries a comment saying precisely that.
Generated by Claude Code
Greptile SummaryThe PR adds Slack unfurls for supported Twenty record links, including event routing, bounded record lookup and card formatting, Slack scopes, read-only function-role permissions, setup documentation, and unit coverage.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; both previously reported attachment-construction issues are fixed in the current code. Important Files Changed
Reviews (3): Last reviewed commit: "style(twenty-slack): narrow fetchSlackRe..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Decode Slack's HTML-escaped URLs in a single pass so pre-escaped sequences like &lt; are not unescaped twice (CodeQL) - Truncate record titles and field values before escaping so oversized CRM values cannot push a block past Slack's section/field limits and void the whole chat.unfurl batch - Percent-encode <, > and | in the embedded link so a query string cannot break mrkdwn <url|label> syntax - Replace the untyped CoreApiClient cast and any-typed records with per-object typed fetchers behind one narrow, documented cast - Split the three unfurl formatters into one util per file
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 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/twenty-apps/public/slack/src/logic-functions/utils/unfurl-slack-record-links.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/unfurl-slack-record-links.ts:41">
P2: When loading the Slack connection fails transiently, this branch acknowledges the link event as skipped and prevents the logic-function runtime from retrying it. Throw the connection error for infrastructure failures; reserve `ok: true` skips for valid non-record or intentionally disabled inputs.</violation>
<violation number="2" location="packages/twenty-apps/public/slack/src/logic-functions/utils/unfurl-slack-record-links.ts:81">
P2: When `chat.unfurl` fails transiently, this catch returns a successful Promise with an application-level `ok: false`, so the logic-function runtime has no failure to retry and the record links remain unfurled. Throw the error after logging it so the event execution is retried.</violation>
</file>
<file name="packages/twenty-apps/public/slack/SETUP.md">
<violation number="1" location="packages/twenty-apps/public/slack/SETUP.md:80">
P3: The new 'Record link unfurls' step shifted 'Role' from step 4 to step 5, so the Channel welcome behaviour note's reference to 'the shared-role caveat from step 4 above' is now stale — step 4 is Reconnect. Update that reference to 'step 5'.</violation>
</file>
<file name="packages/twenty-apps/public/slack/src/logic-functions/utils/parse-slack-link-shared-event.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/parse-slack-link-shared-event.ts:41">
P2: Composer previews are detected only by the absence of `message_ts`, while the PR adds a `source` field to the event type (which Slack sets to `"composer"` for composer previews) but never reads it here. Slack identifies link_shared events that have no posted message via `source`/`unfurl_id`; relying on a missing `message_ts` is a proxy that would silently unfurl to a `COMPOSER` channel if Slack ever includes a timestamp on composer events. Check `event.source === 'composer'` (or the presence of `event.unfurl_id`) explicitly.</violation>
</file>
<file name="packages/twenty-apps/public/slack/src/roles/default-function.role.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/roles/default-function.role.ts:48">
P2: The shared default (fallback) role now gains read access to all CRM objects and workspace members, moving the whole app from 'No CRM data access' to full read of person/company/opportunity/note/task plus workspaceMember. Because this role is the manifest-builder default for every logic function and tool, all of them can now read these records, not just the unfurl logic that needs it. Scope the read to a dedicated role bound to the slack-link-unfurl function (or round-trip through the assistant role) instead of widening the shared default, and add workspaceMember to the description.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const slackClientResult = await getSlackClient(); | ||
|
|
||
| if (!slackClientResult.success) { | ||
| return { ok: true, skipped: slackClientResult.error }; |
There was a problem hiding this comment.
P2: When loading the Slack connection fails transiently, this branch acknowledges the link event as skipped and prevents the logic-function runtime from retrying it. Throw the connection error for infrastructure failures; reserve ok: true skips for valid non-record or intentionally disabled inputs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/utils/unfurl-slack-record-links.ts, line 41:
<comment>When loading the Slack connection fails transiently, this branch acknowledges the link event as skipped and prevents the logic-function runtime from retrying it. Throw the connection error for infrastructure failures; reserve `ok: true` skips for valid non-record or intentionally disabled inputs.</comment>
<file context>
@@ -0,0 +1,85 @@
+ const slackClientResult = await getSlackClient();
+
+ if (!slackClientResult.success) {
+ return { ok: true, skipped: slackClientResult.error };
+ }
+
</file context>
There was a problem hiding this comment.
Keeping the ack deliberate here. getSlackConnection folds "Slack is not connected" — a normal, permanent state for installs that never set up the assistant — into the same failure shape as a transient lookup error, so throwing would put every pasted link on unconfigured installs into a retry loop. Unfurls are best-effort decoration and the loss on a rare transient failure is one preview, so the trade-off leans toward the quiet skip. Splitting transient from permanent connection errors would need a change in the shared connection util, which the assistant paths would also want; that's worth doing once, separately.
Generated by Claude Code
|
|
||
| console.warn(`[slack] chat.unfurl failed: ${message}`); | ||
|
|
||
| return { ok: false, error: message }; |
There was a problem hiding this comment.
P2: When chat.unfurl fails transiently, this catch returns a successful Promise with an application-level ok: false, so the logic-function runtime has no failure to retry and the record links remain unfurled. Throw the error after logging it so the event execution is retried.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/utils/unfurl-slack-record-links.ts, line 81:
<comment>When `chat.unfurl` fails transiently, this catch returns a successful Promise with an application-level `ok: false`, so the logic-function runtime has no failure to retry and the record links remain unfurled. Throw the error after logging it so the event execution is retried.</comment>
<file context>
@@ -0,0 +1,85 @@
+
+ console.warn(`[slack] chat.unfurl failed: ${message}`);
+
+ return { ok: false, error: message };
+ }
+
</file context>
There was a problem hiding this comment.
Deliberate as well, for the same reason as the connection branch: chat.unfurl failures skew permanent, not transient — the loudest case is an older install that hasn't re-authorized yet, where every pasted link fails with missing_scope until the admin reconnects. Throwing there would turn each pasted link into a retry loop that can never succeed. The failure is logged and reported in the result (ok: false), and the cost of dropping a genuinely transient failure is one missing preview on one message.
Generated by Claude Code
| ...READABLE_CRM_OBJECT_UNIVERSAL_IDENTIFIERS.map( | ||
| (objectUniversalIdentifier) => ({ | ||
| objectUniversalIdentifier, | ||
| canReadObjectRecords: true, |
There was a problem hiding this comment.
P2: The shared default (fallback) role now gains read access to all CRM objects and workspace members, moving the whole app from 'No CRM data access' to full read of person/company/opportunity/note/task plus workspaceMember. Because this role is the manifest-builder default for every logic function and tool, all of them can now read these records, not just the unfurl logic that needs it. Scope the read to a dedicated role bound to the slack-link-unfurl function (or round-trip through the assistant role) instead of widening the shared default, and add workspaceMember to the description.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/roles/default-function.role.ts, line 48:
<comment>The shared default (fallback) role now gains read access to all CRM objects and workspace members, moving the whole app from 'No CRM data access' to full read of person/company/opportunity/note/task plus workspaceMember. Because this role is the manifest-builder default for every logic function and tool, all of them can now read these records, not just the unfurl logic that needs it. Scope the read to a dedicated role bound to the slack-link-unfurl function (or round-trip through the assistant role) instead of widening the shared default, and add workspaceMember to the description.</comment>
<file context>
@@ -27,6 +42,15 @@ export default defineApplicationRole({
+ ...READABLE_CRM_OBJECT_UNIVERSAL_IDENTIFIERS.map(
+ (objectUniversalIdentifier) => ({
+ objectUniversalIdentifier,
+ canReadObjectRecords: true,
+ canUpdateObjectRecords: false,
+ canSoftDeleteObjectRecords: false,
</file context>
There was a problem hiding this comment.
Took the description part (workspace members are mentioned now, 19ae7c1), but the widening itself is the deliberate trade-off this PR makes, and both suggested alternatives are unavailable today: LogicFunctionManifest has no per-function role field, so a dedicated role cannot be bound to slack-link-unfurl (this is the constraint written down in #24054), and round-tripping through the assistant role means a runAgent call per pasted link, which is the wrong tool for a deterministic single-record read. The widening is read-only, mirrors exactly the object set the assistant role already exposes to anyone who can message the bot, and is called out in SETUP.md. If per-function roles land in the platform, narrowing this back is the natural follow-up.
Generated by Claude Code
- Skip composer previews explicitly on event.source, not only on the missing message timestamp - Fix the stale step number in the SETUP.md channel welcome note - Mention workspace member read access in the function role description
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. Summary
Automated pre-review — human approval still required. |
There was a problem hiding this comment.
3 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="packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts:11">
P3: When a long title or field value contains an astral Unicode character at the truncation boundary, `slice` splits its surrogate pair and corrupts that character. Truncate by code points before appending the ellipsis.</violation>
<violation number="2" location="packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts:38">
P2: When a pasted record URL has a long query string, this section still exceeds Slack’s 3,000-character limit because only the title is capped and delimiter encoding can expand the URL. Bound or remove the query portion of the embedded URL before building the section.</violation>
</file>
<file name="packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-record-link.test.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-record-link.test.ts:40">
P3: The new "should not unescape entities twice" test cannot detect the regression it claims to guard. The entity `&lt;x` is placed in the query string, but `parseSlackRecordLink` only inspects `origin` and `pathname` (then the UUID record id); the decoded query string is never read, and the returned `recordId` comes from the path. So `expect(parsed?.recordId).toBe(RECORD_ID)` passes identically whether `decodeSlackLinkUrl` unescapes once, twice, or not at all. Assert an output the decode actually influences — e.g. put the entity in a path segment and assert the resulting `objectNameSingular`, or expose the decoded URL — otherwise the single-pass guarantee has no test.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| type: 'section', | ||
| text: { | ||
| type: 'mrkdwn', | ||
| text: `*<${encodeSlackLinkUrl(linkUrl)}|${recordTitle}>*`, |
There was a problem hiding this comment.
P2: When a pasted record URL has a long query string, this section still exceeds Slack’s 3,000-character limit because only the title is capped and delimiter encoding can expand the URL. Bound or remove the query portion of the embedded URL before building the section.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts, line 38:
<comment>When a pasted record URL has a long query string, this section still exceeds Slack’s 3,000-character limit because only the title is capped and delimiter encoding can expand the URL. Bound or remove the query portion of the embedded URL before building the section.</comment>
<file context>
@@ -2,22 +2,40 @@ import { type KnownBlock, type MessageAttachment } from '@slack/web-api';
text: {
type: 'mrkdwn',
- text: `*<${linkUrl}|${escapeSlackMrkdwn(card.recordTitle)}>*`,
+ text: `*<${encodeSlackLinkUrl(linkUrl)}|${recordTitle}>*`,
},
},
</file context>
There was a problem hiding this comment.
Addressed in 0271a52 (canonical recordUrl embedded, raw event url kept only as the unfurl key) and 3f42f05 (parameter renamed to make the contract explicit) — see the duplicate thread below for details.
Generated by Claude Code
| const truncateText = (text: string, maxLength: number): string => | ||
| text.length > maxLength ? `${text.slice(0, maxLength - 1)}…` : text; |
There was a problem hiding this comment.
P3: When a long title or field value contains an astral Unicode character at the truncation boundary, slice splits its surrogate pair and corrupts that character. Truncate by code points before appending the ellipsis.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts, line 11:
<comment>When a long title or field value contains an astral Unicode character at the truncation boundary, `slice` splits its surrogate pair and corrupts that character. Truncate by code points before appending the ellipsis.</comment>
<file context>
@@ -2,22 +2,40 @@ import { type KnownBlock, type MessageAttachment } from '@slack/web-api';
+const TITLE_MAX_LENGTH = 250;
+const FIELD_VALUE_MAX_LENGTH = 350;
+
+const truncateText = (text: string, maxLength: number): string =>
+ text.length > maxLength ? `${text.slice(0, maxLength - 1)}…` : text;
+
</file context>
| const truncateText = (text: string, maxLength: number): string => | |
| text.length > maxLength ? `${text.slice(0, maxLength - 1)}…` : text; | |
| const truncateText = (text: string, maxLength: number): string => { | |
| const characters = Array.from(text); | |
| return characters.length > maxLength | |
| ? `${characters.slice(0, maxLength - 1).join('')}…` | |
| : text; | |
| }; |
| workspaceBaseUrl: WORKSPACE_BASE_URL, | ||
| }); | ||
|
|
||
| expect(parsed?.recordId).toBe(RECORD_ID); |
There was a problem hiding this comment.
P3: The new "should not unescape entities twice" test cannot detect the regression it claims to guard. The entity &lt;x is placed in the query string, but parseSlackRecordLink only inspects origin and pathname (then the UUID record id); the decoded query string is never read, and the returned recordId comes from the path. So expect(parsed?.recordId).toBe(RECORD_ID) passes identically whether decodeSlackLinkUrl unescapes once, twice, or not at all. Assert an output the decode actually influences — e.g. put the entity in a path segment and assert the resulting objectNameSingular, or expose the decoded URL — otherwise the single-pass guarantee has no test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-record-link.test.ts, line 40:
<comment>The new "should not unescape entities twice" test cannot detect the regression it claims to guard. The entity `&lt;x` is placed in the query string, but `parseSlackRecordLink` only inspects `origin` and `pathname` (then the UUID record id); the decoded query string is never read, and the returned `recordId` comes from the path. So `expect(parsed?.recordId).toBe(RECORD_ID)` passes identically whether `decodeSlackLinkUrl` unescapes once, twice, or not at all. Assert an output the decode actually influences — e.g. put the entity in a path segment and assert the resulting `objectNameSingular`, or expose the decoded URL — otherwise the single-pass guarantee has no test.</comment>
<file context>
@@ -30,6 +30,16 @@ describe('parseSlackRecordLink', () => {
+ workspaceBaseUrl: WORKSPACE_BASE_URL,
+ });
+
+ expect(parsed?.recordId).toBe(RECORD_ID);
+ });
+
</file context>
… point - Embed a canonical origin + /object/<name>/<id> url in the card so a pasted link's query string cannot push the section past Slack's 3000-char limit; the raw event url stays the chat.unfurl key - Truncate titles and field values by code point so surrogate pairs survive the boundary - Move the link url entity decoding into its own util with direct tests for the single-pass guarantee
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="packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-record-link.test.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-record-link.test.ts:40">
P3: This test does not protect the behavior it describes. `parseSlackRecordLink` only returns the original `linkUrl`, `objectNameSingular`, and `recordId`; none of these reflect the decoded query string, so the double-unescaping regression (`&lt;` → `<`) is unobservable through this test. The assertion `parsed?.recordId` toBe `RECORD_ID` passes whether or not the `<` is preserved, so the test can never fail for the case it claims to cover. To guard the single-decode behavior, assert the decoded output directly (e.g. expose/verify the decoded query retains `<` rather than `<`), or rework the test so it observes actual decoding.</violation>
</file>
<file name="packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts:18">
P2: When an oversized title or field value ends at a surrogate-pair boundary, `truncateText` splits the character and can render `�`. Truncate by code points before appending the ellipsis.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| text: string; | ||
| maxLength: number; | ||
| }): string => | ||
| text.length > maxLength ? `${text.slice(0, maxLength - 1)}…` : text; |
There was a problem hiding this comment.
P2: When an oversized title or field value ends at a surrogate-pair boundary, truncateText splits the character and can render �. Truncate by code points before appending the ellipsis.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/utils/build-slack-record-unfurl-attachment.ts, line 18:
<comment>When an oversized title or field value ends at a surrogate-pair boundary, `truncateText` splits the character and can render `�`. Truncate by code points before appending the ellipsis.</comment>
<file context>
@@ -2,22 +2,46 @@ import { type KnownBlock, type MessageAttachment } from '@slack/web-api';
+ text: string;
+ maxLength: number;
+}): string =>
+ text.length > maxLength ? `${text.slice(0, maxLength - 1)}…` : text;
+
const escapeSlackMrkdwn = (text: string): string =>
</file context>
| text.length > maxLength ? `${text.slice(0, maxLength - 1)}…` : text; | |
| text.length > maxLength | |
| ? `${[...text].slice(0, maxLength - 1).join('')}…` | |
| : text; |
| workspaceBaseUrl: WORKSPACE_BASE_URL, | ||
| }); | ||
|
|
||
| expect(parsed?.recordId).toBe(RECORD_ID); |
There was a problem hiding this comment.
P3: This test does not protect the behavior it describes. parseSlackRecordLink only returns the original linkUrl, objectNameSingular, and recordId; none of these reflect the decoded query string, so the double-unescaping regression (&lt; → <) is unobservable through this test. The assertion parsed?.recordId toBe RECORD_ID passes whether or not the < is preserved, so the test can never fail for the case it claims to cover. To guard the single-decode behavior, assert the decoded output directly (e.g. expose/verify the decoded query retains < rather than <), or rework the test so it observes actual decoding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-record-link.test.ts, line 40:
<comment>This test does not protect the behavior it describes. `parseSlackRecordLink` only returns the original `linkUrl`, `objectNameSingular`, and `recordId`; none of these reflect the decoded query string, so the double-unescaping regression (`&lt;` → `<`) is unobservable through this test. The assertion `parsed?.recordId` toBe `RECORD_ID` passes whether or not the `<` is preserved, so the test can never fail for the case it claims to cover. To guard the single-decode behavior, assert the decoded output directly (e.g. expose/verify the decoded query retains `<` rather than `<`), or rework the test so it observes actual decoding.</comment>
<file context>
@@ -30,6 +30,16 @@ describe('parseSlackRecordLink', () => {
+ workspaceBaseUrl: WORKSPACE_BASE_URL,
+ });
+
+ expect(parsed?.recordId).toBe(RECORD_ID);
+ });
+
</file context>
|
Auto-closed: this PR is draft and has had no update in over 24 hours. Please reopen it once you have the bandwidth to take it forward. |
twenty-eng-sync
Bot
added
the
-PR: stale auto-closed
abdulrahmancodes
removed
the
-PR: stale auto-closed
abdulrahmancodes
added
-PR: awaiting review
and removed
-PR: draft
labels
Claude
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
…rdUrl Makes the builder's contract explicit: it receives the canonical bounded record url from parseSlackRecordLink, never the raw event url with its unbounded query string.
abdulrahmancodes
added
-PR: awaiting review
and removed
-PR: draft
labels
Claude
twenty-eng-sync
Bot
added
the
-PR: stale auto-closed
|
Auto-closed: this PR is draft and has had no update in over 24 hours. Please reopen it once you have the bandwidth to take it forward. |
abdulrahmancodes
removed
the
-PR: stale auto-closed
Combines the link unfurl routing with the install-revocation routing (app_uninstalled / tokens_revoked) and the run-as-workspace-member role changes from main: both resolver cases kept, the function role keeps its slackUserLink and workspaceMember grants alongside the read-only CRM objects for unfurls, and SETUP.md documents both upgrade paths. Version stays 0.5.0 above main's 0.4.1.
2 participants
Why
Pasting a Twenty record URL in Slack shows a plain link: the record page is an auth-gated SPA, so Slack's generic unfurl carries nothing useful, and #24054 turned unfurls off in assistant replies for that reason. Tier-1 CRM integrations expand their record links into rich cards; this brings the same to Twenty links people paste.
Closes twentyhq/core-team-issues#2769.
What it does
The manifest subscribes to
link_shared(withlinks:read/links:writeand an App Unfurl Domain for the workspace). The events resolver routes the event to a newslack-link-unfurllogic function that runs in the resolved workspace: it parses links matching/object/<objectNameSingular>/<recordId>against the workspace's own base URL, fetches each record, and attaches a compact card viachat.unfurl— the linked record name plus a few key fields per object type:Anything that does not resolve — a non-record path, another origin, a non-UUID id, an unsupported object, a deleted or inaccessible record — is skipped silently: no error card, and
chat.unfurlis never called when nothing resolves. Lookups are capped at 5 links per message. Composer previews (typed but not yet sent) are skipped; the posted message triggers its own event. Slack HTML-escapes URLs inlink_sharedpayloads, so the parser decodes for matching but keeps the original string as the unfurl key.Permissions
Logic functions run under the app's default role, which held no CRM access by design, and
LogicFunctionManifesthas no per-function role field (the constraint written down in #24054). The default role therefore gains read-only access to exactly the objects the Slack Assistant role can read (people, companies, opportunities, notes, tasks, plus workspace members for the assignee name), so a card can never expose more than the assistant itself. Anyone in the channel sees the card, so it reads with that shared role, not the poster's Twenty permissions — same trade-off as the assistant, and SETUP.md says so.Not in this PR
unfurl_linksoff; this only covers links people paste.Version
Bumped to 0.5.0 — 0.4.0 is claimed by #23985 and 0.3.1 by #24097; the versions compose in any merge order. Existing installs must add the
link_sharedevent, the unfurl domain and the twolinks:*scopes to their Slack app, then disconnect and reconnect to re-authorize — the feature stays off (events simply don't arrive) until they do.Testing
yarn test:unitinpackages/twenty-apps/public/slack: 22 suites, 143 tests.yarn typecheck,yarn lintandtwenty dev:buildclean.New coverage:
parse-slack-link-shared-event.test.ts(routing, dedupe, composer skip),parse-slack-record-link.test.ts(origin/path/UUID matching, escaped URLs kept as unfurl keys),format-slack-unfurl-field-values.test.ts(select, date, currency micros),build-slack-record-unfurl-attachment.test.ts(block layout, mrkdwn escaping), andunfurl-slack-record-links.test.ts(happy path, silent skips, partial resolution,chat.unfurlfailure).Generated by Claude Code