Repository navigation
chore: gitignore cleanup, type fixes, and pending dialer/telemetry/docs work - #66
Conversation
…cs work - .gitignore: add node-compile-cache/, *.dylib, tsx-*/, v8-compile-cache-*/, nx-native-file-cache-*/, jest_dx/, jiti/, xfs-*/, .agent/worktrees/, .tmp.driveupload/ — stops thousands of cache files from cluttering PRs - fix: add 'no_caller_id' to CallErrorReason type - fix: LocalPresenceIndicator theme color (green) and background (transparent.success) - fix: add CallOutcome type alias to queue types - fix: add countWords helper to useCoaching, fix callSid null assertion - fix: DialConfirmationModal useGlobalHotkeys signature - fix: cli/update.ts arrow functions + type import - fix: exclude __stories__ from dialer tsconfig (needs @storybook/test) - dialer: new components, hooks, states, types, utils - telemetry: PostHogInitEffect, posthog lib, posthog server service - docs: user-guide graphql-api section (15 mdx files) - settings: SettingsAIScripts component - misc: consuelo logo svg, cli update command, number-packs test
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
Summary by CodeRabbit
WalkthroughLarge feature addition: extensive dialer frontend (components, hooks, states, sounds), backend dialer/refactored call SQL and Twilio recording APIs, PostHog telemetry (frontend + backend), new CLI update command, many GraphQL API docs/navigation additions, new agent/skill scripts/docs, test additions, and .gitignore/tsconfig adjustments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
@codex can you review this please? |
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:42499 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f7908c2a1
ℹ️ 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".
| twiml, | ||
| statusCallback: statusCallbackUrl, | ||
| statusCallbackEvent: ['initiated', 'ringing', 'answered', 'completed'], | ||
| timeout: PHONE_CALL_TIMEOUT_SECONDS, | ||
| }, |
There was a problem hiding this comment.
This call now omits statusCallbackEvent, so it uses ConferenceService.createCall defaults (['completed']). The phone bridge flow in /v1/voice/phone-status advances only when Twilio sends an in-progress callback, so the lead leg is never added to the conference and calls can remain stuck in initiating until they end.
Useful? React with 👍 / 👎.
| default: | ||
| return 'connected'; |
There was a problem hiding this comment.
The wrap-up UI includes a no-answer option, but this mapper falls through to the default connected outcome for that value. That makes queue analytics and lastCallOutcome inconsistent with the stored list-member disposition when users explicitly choose no-answer.
Useful? React with 👍 / 👎.
| } catch (error: unknown) { | ||
| Sentry.captureException(error, { | ||
| extra: { context: 'startCurrentQueueItem', listId }, | ||
| }); |
There was a problem hiding this comment.
If connect throws here (device state, mic permission, transient SDK error), the catch only logs and leaves autoStartedItemIdRef unchanged. The auto-start effect then refuses to retry this same queue item because it treats it as already started, which can deadlock the active list on a recoverable failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/twenty-front/src/modules/dialer/hooks/useCoaching.ts (1)
176-178:⚠️ Potential issue | 🟡 MinorMissing Sentry tracking in catch block.
Per coding guidelines, caught exceptions must have Sentry tracking. The
fetchCoachingfunction properly usescaptureException(line 126), butrefreshCoachingsilently swallows errors.Add Sentry tracking for observability
} - } catch { - // graceful degradation — coaching still works via initial REST fetch + } catch (err: unknown) { + // graceful degradation — coaching still works via initial REST fetch + captureException(err, { extra: { context: 'refreshCoaching', callSid } }); }As per coding guidelines: "HTTP errors and caught exceptions must have sentry tracking".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/hooks/useCoaching.ts` around lines 176 - 178, The catch block in refreshCoaching silently swallows errors and needs Sentry tracking like fetchCoaching does; update the catch in refreshCoaching to accept the error and call captureException(error) (same Sentry helper used in fetchCoaching) and optionally log a brief contextual message so exceptions are observable for HTTP/caught errors in useCoaching.packages/twenty-front/src/modules/dialer/components/DialConfirmationModal.tsx (1)
220-220:⚠️ Potential issue | 🟡 MinorMissing localization for user-facing strings.
Several strings are not wrapped with the
ttag from Lingui:
- Line 220:
'Unknown Number'- Line 234:
'No caller IDs available'- Line 258:
Call Now🌐 Proposed localization fixes
- <StyledName>{contact?.name ?? 'Unknown Number'}</StyledName> + <StyledName>{contact?.name ?? t`Unknown Number`}</StyledName>- : 'No caller IDs available'} + : t`No caller IDs available`}<IconPhone size={16} /> - Call Now + {t`Call Now`}As per coding guidelines: "Use Lingui for all user-facing strings"
Also applies to: 234-234, 258-258
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialConfirmationModal.tsx` at line 220, The component DialConfirmationModal contains hard-coded user-facing strings (e.g., 'Unknown Number' in StyledName, 'No caller IDs available', and the "Call Now" button label) that must be localized; update the DialConfirmationModal component to wrap these strings with the Lingui `t`/`Trans` utilities (importing from `@lingui/macro` or the project's Lingui helpers), replacing the literals in StyledName, the caller ID empty-state text, and the Call Now button label with calls to `t` or `<Trans>` and ensure any interpolation uses the Lingui API so translations can be extracted.
🟡 Minor comments (10)
packages/twenty-docs/user-guide/graphql-api/troubleshooting/common-ops-issues.mdx-28-29 (1)
28-29:⚠️ Potential issue | 🟡 MinorDelay guidance is inconsistent with the rest of the GraphQL docs.
This page recommends
1 second, while other new pages recommend100mspacing. Please normalize to one clear baseline (or explain when to use each).Also applies to: 148-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-docs/user-guide/graphql-api/troubleshooting/common-ops-issues.mdx` around lines 28 - 29, The delay guidance in this doc is inconsistent: replace the "1 second" recommendation in the bullet "Add delays between requests (1 second recommended)" (and the duplicate guidance at the later bullets) with the project baseline "100ms" or add a short clarifying sentence explaining when to use 100ms vs 1s (e.g., use 100ms as the default pacing for normal workloads and 1s for heavy rate-limited endpoints), and keep the "Implement exponential backoff" bullet as-is; update the nearby text to explicitly state the chosen baseline so it matches other GraphQL docs.packages/twenty-docs/user-guide/graphql-api/general/rate-limits-and-best-practices.mdx-182-192 (1)
182-192:⚠️ Potential issue | 🟡 MinorRequest counter is never incremented in monitoring example.
As written,
Requests in last minutewill stay at0.✅ Suggested fix
client.request = async function(query, variables) { + requestCount += 1; console.log(`API Request: ${new Date().toISOString()}`); return originalRequest(query, variables); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-docs/user-guide/graphql-api/general/rate-limits-and-best-practices.mdx` around lines 182 - 192, The example never increments requestCount, so "Requests in last minute" stays 0; modify client.request (the wrapper around originalRequest) to increment the shared requestCount each time it's invoked (e.g., requestCount++ before/when calling originalRequest) so the setInterval log reflects actual traffic; ensure you increment inside client.request (not in the interval) and do it regardless of request success/failure.packages/twenty-docs/user-guide/graphql-api/general/export-analytics.mdx-85-87 (1)
85-87:⚠️ Potential issue | 🟡 MinorHandle empty datasets before accessing
records[0].If no records are returned, this example crashes while building headers.
🧩 Suggested fix
async function exportToCSV(records, filename) { + if (!records.length) { + fs.writeFileSync(filename, ''); + console.log(`No records to export. Created empty file: ${filename}`); + return; + } const headers = Object.keys(records[0]).join(',');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-docs/user-guide/graphql-api/general/export-analytics.mdx` around lines 85 - 87, The exportToCSV function currently reads headers from records[0] which crashes on empty input; add an early guard in exportToCSV to check if records is falsy or records.length === 0 and handle it (for example return an empty string or a CSV containing only headers if you can determine them) instead of accessing records[0]; update any callers if they expect a non-empty return. Ensure you reference exportToCSV and the records variable when making the change.packages/twenty-docs/user-guide/graphql-api/general/bulk-update-script.mdx-43-43 (1)
43-43:⚠️ Potential issue | 🟡 Minor
batchSizeconfig is ignored during fetch pagination.Line 43 documents batch configurability, but Line 96 hardcodes
first: 60. This makes the template behavior inconsistent and harder to tune.Proposed fix
- first: 60, + first: CONFIG.batchSize,Also applies to: 96-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-docs/user-guide/graphql-api/general/bulk-update-script.mdx` at line 43, The docs and template claim batchSize is configurable but the fetch pagination hardcodes first: 60, so update the pagination call to use the batchSize value (and cap at the API max if needed); specifically replace the literal first: 60 in the GraphQL fetch/query with first: batchSize (or first: Math.min(batchSize, 60)) and ensure the batchSize variable is passed into the query/request where the GraphQL variables are assembled so the documented batchSize actually controls pagination.packages/twenty-docs/user-guide/graphql-api/automation/internal-workflows-overview.mdx-116-120 (1)
116-120:⚠️ Potential issue | 🟡 MinorMinor readability cleanup needed in testing guidance.
Line 117-120 repeats the same sentence opening pattern, and Line 152 (“end result”) is slightly wordy. A quick rephrase would improve flow.
Also applies to: 152-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-docs/user-guide/graphql-api/automation/internal-workflows-overview.mdx` around lines 116 - 120, The four testing-guidance sentences that begin the same way (the repeated opening at lines 117–120) should be rephrased for variety and flow — e.g., turn some into direct questions or short imperatives (“If no records are found, …”, “Handle action failures by …”, “Limit concurrent triggers when …”, “Design for interruptions by …”) or merge closely related items into a single sentence to avoid repetition; also simplify the phrase “end result” (line 152) to a leaner alternative like “result” or “final outcome” to tighten wording. Ensure you update the surrounding sentences so tense and voice remain consistent.packages/twenty-docs/user-guide/graphql-api/automation/slack-notifications.mdx-231-235 (1)
231-235:⚠️ Potential issue | 🟡 MinorAdd official Slack documentation citation for rate limits.
Line 233-234 specifies webhook rate limits (1 message per second steady-state; up to 5 per second burst). The steady-state rate matches official Slack documentation, but cite the source directly. Specifically, reference https://docs.slack.dev/apis/web-api/rate-limits/ or https://api.slack.com/apis/rate-limits to confirm the burst limit is accurate and ensure future maintainers can verify these values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-docs/user-guide/graphql-api/automation/slack-notifications.mdx` around lines 231 - 235, Update the "Slack has rate limits for webhooks:" bullets to include an inline citation to Slack's official rate limit docs by adding a reference link (e.g., to https://docs.slack.dev/apis/web-api/rate-limits/ or https://api.slack.com/apis/rate-limits) immediately after the steady-state and burst items so maintainers can verify the "1 message per second" and "up to 5 messages per second" values; ensure the link is formatted as a Markdown link or footnote next to the bullets under the header text "Slack has rate limits for webhooks:".packages/cli/src/commands/update.ts-59-64 (1)
59-64:⚠️ Potential issue | 🟡 MinorTypeScript errors are suppressed but error message references them.
Using
stdio: 'pipe'suppresses the build output, but the error message at line 99-100 says "check the TypeScript errors above". Users won't see any errors. Either inherit stdio to show errors or adjust the error message.🛠️ Proposed fix to show build output
try { execSync('npx tsc', { cwd: path.resolve(srcDir, '..'), - stdio: 'pipe', + stdio: 'inherit', timeout: 60000, }); return true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/update.ts` around lines 59 - 64, The execSync call that runs 'npx tsc' in update.ts currently uses stdio: 'pipe', which hides TypeScript compiler output while the error message still tells users to "check the TypeScript errors above"; change the execSync invocation (the block that calls execSync('npx tsc', { cwd: path.resolve(srcDir, '..'), stdio: 'pipe', timeout: 60000 })) to either use stdio: 'inherit' so compiler output is printed to the console, or keep 'pipe' but capture stdout/stderr and include them in the thrown/logged error so users can see the TypeScript errors.packages/cli/src/commands/update.ts-88-131 (1)
88-131:⚠️ Potential issue | 🟡 MinorFix indentation inside try block.
The try block content (lines 89-130) is indented at 2 spaces but should be at 4 spaces to be properly nested inside the try block.
🔧 Proposed fix
export const updateCommand = async (): Promise<void> => { try { - const currentVersion = getInstalledVersion(); - log(`current version: ${currentVersion}`); + const currentVersion = getInstalledVersion(); + log(`current version: ${currentVersion}`); - if (isLocalDev()) { + if (isLocalDev()) { // ... rest of content similarly indented🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/update.ts` around lines 88 - 131, The try block in update.ts is mis-indented (2 spaces) causing poor nesting and readability; re-indent the entire block contents between the "try {" and the "catch (err: unknown)" to 4 spaces, ensuring lines referencing getInstalledVersion(), isLocalDev(), rebuildLocal(), getLatestVersion(), updateFromNpm(), and process.exit(...) are all consistently indented and that the closing brace before the catch aligns with the try keyword.packages/twenty-front/src/modules/dialer/components/ContactHeader.tsx-125-129 (1)
125-129:⚠️ Potential issue | 🟡 MinorLocalize the placeholder string with Lingui.
The hardcoded string
'Enter a number to call'violates the coding guideline requiring all user-facing text to be localized with Lingui.Proposed fix
+ import { t } from '@lingui/core/macro'; + // ... in the render: <StyledName> - {rawPhoneNumber - ? formatPhone(rawPhoneNumber) - : 'Enter a number to call'} + {rawPhoneNumber + ? formatPhone(rawPhoneNumber) + : t`Enter a number to call`} </StyledName>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/ContactHeader.tsx` around lines 125 - 129, Replace the hardcoded placeholder in ContactHeader (inside StyledName where rawPhoneNumber is falsy) with a Lingui localized message; import the Lingui helper (e.g., t from '@lingui/macro' or use <Trans>) at the top of the ContactHeader component and swap the literal 'Enter a number to call' for a localized string (e.g., t`Enter a number to call` or <Trans>Enter a number to call</Trans>) so formatPhone, rawPhoneNumber and StyledName remain unchanged.packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx-140-146 (1)
140-146:⚠️ Potential issue | 🟡 MinorDelete action lacks confirmation.
The delete button immediately calls
deleteScriptwithout user confirmation. For destructive actions on user content, consider adding a confirmation dialog to prevent accidental data loss.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx` around lines 140 - 146, The Delete Button currently invokes deleteScript(selectedScript.id) immediately (Button, deleteScript, selectedScript.id), so add a user confirmation step before calling deleteScript: modify the Button's onClick to open a confirmation dialog/modal or browser confirm prompt, and only call deleteScript(selectedScript.id) if the user explicitly confirms; ensure the dialog shows script identifying info (e.g., selectedScript.name or id) and that canceling aborts the deletion.
🧹 Nitpick comments (24)
packages/twenty-front/src/modules/dialer/hooks/useCoaching.ts (2)
25-27: Avoid abbreviationein callback parameter.Per coding guidelines, use full descriptive names. Rename
etoentryfor clarity.Proposed fix
const countWords = (entries: TranscriptEntry[]): number => - entries.reduce((sum, e) => sum + e.text.split(/\s+/).filter(Boolean).length, 0); + entries.reduce((sum, entry) => sum + entry.text.split(/\s+/).filter(Boolean).length, 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/hooks/useCoaching.ts` around lines 25 - 27, Rename the callback parameter in countWords from the single-letter `e` to a descriptive name like `entry` to conform to coding guidelines; update the reducer to use `entry` wherever `e` is referenced (e.g., in the expression currently using `e.text`) so the function signature remains const countWords = (entries: TranscriptEntry[]): number => entries.reduce((sum, entry) => ...).
201-202: Non-null assertion relies on closure timing assumptions.While the guard at line 185 ensures
callState.callSidis truthy when the effect initializes, using!inside thesetIntervalcallback assumes the cleanup always runs before the callback fires with stale state. Consider a defensive runtime check instead:Safer approach with runtime guard
const delta = transcript.slice(lastRefreshIndex.current); if (delta.length === 0) return; - // callSid is guaranteed non-null by the guard at the top of this effect - void refreshCoaching(callState.callSid!, delta); + const { callSid } = callState; + if (!callSid) return; + void refreshCoaching(callSid, delta);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/hooks/useCoaching.ts` around lines 201 - 202, The setInterval callback in useCoaching currently uses a non-null assertion on callState.callSid when calling refreshCoaching, which is unsafe; change it to defensively guard at runtime by either capturing the validated callSid into a local const (e.g., const callSid = callState.callSid) before creating the interval and using that const inside the callback, or check if (callState.callSid) return/skip inside the callback before calling refreshCoaching; update references to refreshCoaching and the setInterval callback in useCoaching to remove the '!' assertion and handle the falsy case gracefully.packages/twenty-front/src/modules/dialer/hooks/useTwilioDevice.ts (1)
144-146: Consider rate-limited error tracking for polling failures.The empty catch block swallows all errors silently. While individual polling failures are non-fatal, persistent failures (e.g., network issues, expired auth) would go unnoticed. Consider tracking after N consecutive failures or using a rate limiter.
💡 Optional: Track persistent polling failures
+ const consecutiveFailuresRef = useRef(0); + const MAX_CONSECUTIVE_FAILURES = 5; + // poll call status for failure detection (DEV-816) const startStatusPolling = useCallback( (callSid: string) => { if (statusPollRef.current) clearInterval(statusPollRef.current); + consecutiveFailuresRef.current = 0; statusPollRef.current = setInterval(async () => { try { const res = await authenticatedFetch( `${REACT_APP_SERVER_BASE_URL}/v1/calls/status/${callSid}`, ); - if (!res.ok) return; + if (!res.ok) { + consecutiveFailuresRef.current++; + if (consecutiveFailuresRef.current >= MAX_CONSECUTIVE_FAILURES) { + captureException(new Error('Status polling persistent failure'), { + extra: { callSid, consecutiveFailures: consecutiveFailuresRef.current }, + }); + } + return; + } + consecutiveFailuresRef.current = 0; // ... rest of handler } catch (err: unknown) { - // polling failure is non-fatal + consecutiveFailuresRef.current++; + if (consecutiveFailuresRef.current >= MAX_CONSECUTIVE_FAILURES) { + captureException(err, { + extra: { context: 'statusPolling', callSid, consecutiveFailures: consecutiveFailuresRef.current }, + }); + } } }, 3000); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/hooks/useTwilioDevice.ts` around lines 144 - 146, In useTwilioDevice.ts, replace the empty catch in the polling block inside useTwilioDevice with a lightweight rate-limited failure tracker: keep a module- or hook-scoped consecutiveFailure counter (ref/state) that you increment on each catch and reset to 0 on successful poll; when the counter reaches a threshold (e.g., 3) call your error-tracking/logger function (e.g., sendErrorEvent/console.error/Sentry.captureException) with the caught error and then suppress further reports for a cooldown period (store a lastReported timestamp) so you don’t flood logs; also detect auth-related errors (401/403) and trigger any refreshAuth/reauth path if available.packages/twenty-front/src/modules/dialer/components/__stories__/DialConfirmationModal.stories.tsx (1)
156-174: Unusedcanvasvariables in hotkey test stories.In
CloseWithEscapeandConfirmWithEnterplay functions, thecanvasvariable is declared but never used since these tests only verify keyboard interactions.🧹 Remove unused variables
export const CloseWithEscape: Story = { - play: async ({ canvasElement, args }) => { - const canvas = within(canvasElement); - + play: async ({ args }) => { await userEvent.keyboard('{Escape}'); await expect(args.onClose).toHaveBeenCalled(); }, }; export const ConfirmWithEnter: Story = { - play: async ({ canvasElement, args }) => { - const canvas = within(canvasElement); - + play: async ({ args }) => { await userEvent.keyboard('{Enter}'); await expect(args.onConfirm).toHaveBeenCalled(); }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/__stories__/DialConfirmationModal.stories.tsx` around lines 156 - 174, Remove the unused canvas variable declarations in the play functions for CloseWithEscape and ConfirmWithEnter: delete the lines that create "const canvas = within(canvasElement);" inside the play blocks of the CloseWithEscape and ConfirmWithEnter Story objects so the functions only perform userEvent.keyboard and the expect assertions; this eliminates the unused-variable lint warning without changing test behavior.packages/twenty-docs/user-guide/graphql-api/automation/data-cleanup-automation.mdx (1)
170-177: Phone normalization example is overly US-specific.Line 173-174 rewrites all numeric inputs to a US 10-digit format, which can damage international values. Consider documenting a guarded or locale-aware normalization path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-docs/user-guide/graphql-api/automation/data-cleanup-automation.mdx` around lines 170 - 177, The current phone normalization uses phone -> normalized with .replace(/\D/g...) and a US-only regex that forces 10-digit formatting; change this to a guarded or locale-aware approach: first preserve the original when input is non-US or not exactly 10 digits (check phone after stripping non-digits), or integrate a proper library (e.g., libphonenumber) to parse/format by region; update the code paths that produce normalizedPhone (variables phone and normalized) so they only apply the US 10-digit transform when the cleaned number length === 10 or when a provided locale indicates US, otherwise return the original or a library-formatted international string.packages/cli/src/commands/update.ts (1)
15-16: Consider adding Sentry tracking to silently caught exceptions.Per coding guidelines, caught exceptions should have Sentry tracking. While returning fallback values is appropriate here, silently swallowing errors loses diagnostic information. The same applies to
getLatestVersion(line 28-29) andisLocalDev(line 42-43).♻️ Proposed fix
const getInstalledVersion = (): string => { try { const pkgPath = require.resolve(`${CLI_PACKAGE_NAME}/package.json`); const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')); return pkg.version; - } catch { + } catch (err: unknown) { + captureError(err, { command: 'update', context: 'getInstalledVersion' }); return 'unknown'; } };Apply similar pattern to
getLatestVersionandisLocalDev.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/update.ts` around lines 15 - 16, The catch blocks that currently swallow errors and return fallback values (the anonymous catch returning 'unknown' and the catch blocks in getLatestVersion and isLocalDev) should log the exception to Sentry before returning the fallback; update each catch to capture the thrown error with Sentry.captureException(err) (or the project's Sentry client) and include minimal context (e.g., function name like getLatestVersion, isLocalDev, or update command) so diagnostics are available, then return the existing fallback value. Ensure the Sentry client is imported/available in this module and use the same error-capture pattern for all three catch locations.packages/twenty-front/src/modules/dialer/utils/notificationSounds.ts (2)
23-25: Consider awaitingresume()or documenting the fire-and-forget behavior.
audioContext.resume()returns a Promise. Not awaiting it means the first sound after resuming a suspended context may not play. This is likely acceptable for optional feedback sounds, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/utils/notificationSounds.ts` around lines 23 - 25, The call to audioContext.resume() is currently fire-and-forget which can cause the first sound to miss because resume() returns a Promise; update the code where audioContext.resume() is invoked (the audioContext !== null && audioContext.state === 'suspended' branch) to await audioContext.resume() (or explicitly handle the returned Promise with .then/.catch) so playback is guaranteed after resume, or add a clear inline comment documenting that this is intentionally not awaited and the potential for the first sound to not play.
1-5: Use short-form//comments instead of JSDoc blocks.Per coding guidelines, prefer
//comments over/** */blocks.Suggested change
-/** - * Notification sounds for dialer feedback. - * Uses Web Audio API to generate pleasant, non-intrusive notification sounds. - * No external audio files required - all sounds are synthesized. - */ +// Notification sounds for dialer feedback. +// Uses Web Audio API to generate pleasant, non-intrusive notification sounds. +// No external audio files required - all sounds are synthesized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/utils/notificationSounds.ts` around lines 1 - 5, Replace the leading JSDoc block in the notificationSounds module with short-form single-line comments: change the /** ... */ block at the top of packages/twenty-front/src/modules/dialer/utils/notificationSounds.ts to concise // comments that state the same intent (e.g., purpose, Web Audio API usage, and no external files required) so the file follows the coding guideline preferring // comments; locate the top-of-file comment block around the notification sounds implementation (e.g., the notificationSounds module or exported functions) and update it accordingly.packages/twenty-front/src/modules/dialer/components/ContactHeader.tsx (1)
22-33: Consider localizing status labels.The
STATUS_CONFIGlabels ('Connecting...','Ringing...','Connected', etc.) are user-facing strings. For full i18n compliance, consider refactoring to use Lingui. This would require a different pattern sincetmacros can't be used in static object initializers.packages/twenty-front/src/modules/dialer/components/CallerIdSelectCard.tsx (1)
1-9: Reorganize imports to follow the standard order.Per coding guidelines, imports should be ordered: external libraries first, then internal (
@/), then relative. Currently the imports are mixed.Suggested order
-import { useAvailableCallerIds } from '@/dialer/hooks/useAvailableCallerIds'; -import { useCallerIdSelection } from '@/dialer/hooks/useCallerIdSelection'; -import { localPresenceEnabledState } from '@/dialer/states/localPresenceEnabledState'; -import { Select } from '@/ui/input/components/Select'; -import styled from '@emotion/styled'; -import { t } from '@lingui/core/macro'; -import { useEffect } from 'react'; -import { useRecoilState } from 'recoil'; -import { IconPhone, IconWorld } from 'twenty-ui/display'; +import styled from '@emotion/styled'; +import { t } from '@lingui/core/macro'; +import { useEffect } from 'react'; +import { useRecoilState } from 'recoil'; +import { IconPhone, IconWorld } from 'twenty-ui/display'; + +import { useAvailableCallerIds } from '@/dialer/hooks/useAvailableCallerIds'; +import { useCallerIdSelection } from '@/dialer/hooks/useCallerIdSelection'; +import { localPresenceEnabledState } from '@/dialer/states/localPresenceEnabledState'; +import { Select } from '@/ui/input/components/Select';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/CallerIdSelectCard.tsx` around lines 1 - 9, Reorder the imports in CallerIdSelectCard so external libraries come first, then internal '@/...' imports, then any relative imports: group React and libs (useEffect, styled, t, useRecoilState, IconPhone, IconWorld) together, then move hooks and state/select from internal paths (useAvailableCallerIds, useCallerIdSelection, localPresenceEnabledState, Select) after them; keep the same imported symbols and do not change any usage or semantics—only adjust the import ordering to follow the standard convention.packages/twenty-front/src/modules/object-record/record-show/components/PageLayoutRecordPageRenderer.tsx (1)
101-115: Redundant non-null assertions afterisDefinedcheck.Since
isDefined(pageLayoutId)is already checked in the condition, TypeScript should narrow the type. The!assertions on lines 107 and 114 are unnecessary.♻️ Remove redundant non-null assertions
{isDefined(pageLayoutId) && !isInRightDrawer && targetRecordIdentifier.targetObjectNameSingular === CoreObjectNameSingular.Opportunity && ( <OpportunityCallingWorkspace listId={targetRecordIdentifier.id} - pageLayoutId={pageLayoutId!} + pageLayoutId={pageLayoutId} /> )} {isDefined(pageLayoutId) && (isInRightDrawer || targetRecordIdentifier.targetObjectNameSingular !== CoreObjectNameSingular.Opportunity) && ( - <PageLayoutRenderer pageLayoutId={pageLayoutId!} /> + <PageLayoutRenderer pageLayoutId={pageLayoutId} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/object-record/record-show/components/PageLayoutRecordPageRenderer.tsx` around lines 101 - 115, The non-null assertions on pageLayoutId in the JSX are redundant because isDefined(pageLayoutId) already narrows its type; remove the trailing ! in the props passed to OpportunityCallingWorkspace and PageLayoutRenderer (references: isDefined, pageLayoutId, OpportunityCallingWorkspace, PageLayoutRenderer, targetRecordIdentifier, CoreObjectNameSingular) so the components receive pageLayoutId without unnecessary assertions.packages/twenty-front/src/modules/dialer/components/DialerHomePageContent.tsx (1)
1-4: Import order should place external libraries first.Per coding guidelines, external libraries should be imported before internal modules.
♻️ Suggested import order fix
+import { useRecoilValue } from 'recoil'; + import { DialerHomeLivePanel } from '@/dialer/components/DialerHomeLivePanel'; import { DialerHomePrep } from '@/dialer/components/DialerHomePrep'; import { callStateAtom } from '@/dialer/states/callStateAtom'; -import { useRecoilValue } from 'recoil';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomePageContent.tsx` around lines 1 - 4, Reorder the imports so external libraries come before internal modules: move the external import "useRecoilValue" (from 'recoil') above the internal imports for DialerHomeLivePanel, DialerHomePrep, and callStateAtom; ensure imports remain grouped (external first: useRecoilValue, then internal: DialerHomeLivePanel, DialerHomePrep, callStateAtom) and preserve existing specifiers and paths.packages/twenty-front/src/pages/home/HomePage.tsx (1)
1-11: Import order should place external libraries first.Per coding guidelines, external libraries should be imported before internal modules (
@/), then relative imports (~/).♻️ Suggested import order fix
+import { t } from '@lingui/core/macro'; +import { FeatureFlagKey } from '~/generated-metadata/graphql'; +import { IconSparkles } from 'twenty-ui/display'; +import { Button } from 'twenty-ui/input'; + import { useOpenAskAIPageInCommandMenu } from '@/command-menu/hooks/useOpenAskAIPageInCommandMenu'; import { DialerHomePageContent } from '@/dialer/components/DialerHomePageContent'; import { MainContainerLayoutWithCommandMenu } from '@/object-record/components/MainContainerLayoutWithCommandMenu'; import { SpreadsheetImportProvider } from '@/spreadsheet-import/provider/components/SpreadsheetImportProvider'; import { PageHeader } from '@/ui/layout/page/components/PageHeader'; import { PageContainer } from '@/ui/layout/page/components/PageContainer'; import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled'; -import { t } from '@lingui/core/macro'; -import { FeatureFlagKey } from '~/generated-metadata/graphql'; -import { IconSparkles } from 'twenty-ui/display'; -import { Button } from 'twenty-ui/input';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/pages/home/HomePage.tsx` around lines 1 - 11, Reorder the imports in HomePage.tsx so external libraries come first, then internal "@/..." modules, then "~/..." relative imports: move the external imports like t from '@lingui/core/macro' and IconSparkles/Button from 'twenty-ui/*' to the top, followed by the internal hooks and components (useOpenAskAIPageInCommandMenu, DialerHomePageContent, MainContainerLayoutWithCommandMenu, SpreadsheetImportProvider, PageHeader, PageContainer, useIsFeatureEnabled) and leave FeatureFlagKey from '~/generated-metadata/graphql' last; keep the existing import specifiers unchanged, just adjust their order to match the guideline.packages/twenty-front/src/modules/dialer/components/OpportunityCallAnalyticsTab.tsx (2)
83-90: Hoist constant array outside component.
answeredDispositionsis recreated on every render. Since it's a static constant, define it outside the component to avoid unnecessary allocations.♻️ Proposed refactor
+const ANSWERED_DISPOSITIONS = [ + { value: 'connected', label: t`Connected` }, + { value: 'follow-up', label: t`Follow up` }, + { value: 'not-interested', label: t`Not interested` }, + { value: 'voicemail', label: t`Voicemail` }, + { value: 'busy', label: t`Busy` }, + { value: 'no-answer', label: t`No answer` }, +] as const; export const OpportunityCallAnalyticsTab = ({ // ... }: OpportunityCallAnalyticsTabProps) => { // ... - const answeredDispositions = [ - { value: 'connected', label: t`Connected` }, - // ... - ] as const;Note: If Lingui's
tmacro requires runtime context, keep it inside the component but memoize withuseMemo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/OpportunityCallAnalyticsTab.tsx` around lines 83 - 90, The array constant answeredDispositions inside OpportunityCallAnalyticsTab is recreated on every render; move its declaration outside the component (define answeredDispositions as a top-level constant) so it’s allocated once, or if Lingui's t macro requires component/runtime context, memoize it inside OpportunityCallAnalyticsTab using useMemo (e.g., const answeredDispositions = useMemo(() => [...], [/* i18n deps if any */])) to prevent unnecessary reallocations.
145-145:onRetryhandler is a no-op.Same pattern as other files—the retry callback does nothing. Consider consolidating this pattern or implementing the functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/OpportunityCallAnalyticsTab.tsx` at line 145, The onRetry prop is currently a no-op; replace onRetry={() => undefined} with a real handler (e.g., onRetry={handleRetry}) that triggers the component's existing data reload logic—implement handleRetry inside OpportunityCallAnalyticsTab to set loading state, call the same fetch function used on mount/refetch (the function that loads analytics data, e.g., fetchCallAnalytics / loadAnalytics / refetchAnalytics), and handle success/error states and retries; if the component has no internal fetch, forward the parent's retry prop instead so retries actually re-run the analytics fetch.packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx (2)
18-20: Minor import order inconsistency.
react-router-dom(external) should be grouped with other external imports beforerecoil, not after.import { useState } from 'react'; -import { useRecoilState } from 'recoil'; import { useNavigate } from 'react-router-dom'; +import { useRecoilState } from 'recoil';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx` around lines 18 - 20, The import order in DialerHomePrep.tsx is inconsistent: external React and react-router-dom imports should be grouped before other libraries like recoil; reorder the imports so that useState and useNavigate (from 'react' and 'react-router-dom') come first, followed by useRecoilState (from 'recoil'), keeping existing named imports (useState, useNavigate, useRecoilState) intact and preserving any other import groups.
132-387: Component exceeds recommended size limit.At ~256 lines of JSX (lines 132-387), this component is approaching the 300-line limit. Consider extracting the source-specific panels (person, list, segment, csv, ai) into separate sub-components for better maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx` around lines 132 - 387, The DialerHomePrep component is too large; extract each source-specific panel (person, list, segment, csv, ai) into small subcomponents to reduce JSX in DialerHomePrep: create e.g. DialerSourcePerson, DialerSourceList, DialerSourceSegment, DialerSourceCSV, DialerSourceAI that accept the minimal props they need (for example DialerSourceList should receive selectedListId, setSelectedListId, listRecords, startSelectedList, navigate; DialerSourcePerson should receive status; DialerSourceCSV needs openObjectRecordsSpreadsheetImportDialog and personLabelLowercase; DialerSourceAI needs openAskAIPage and isAiEnabled; DialerSourceSegment needs navigate and personObjectNamePlural). Replace the inline conditional blocks in DialerHomePrep with these components and keep helper functions like startSelectedList and state (source, assistMode, selectedScriptId) in DialerHomePrep so props flow down; ensure unique JSX ids (dropdownId props) and event handlers (setSource, setSelectedListId, setSelectedScriptId, setAssistMode) are preserved when moving code.packages/twenty-front/src/modules/dialer/components/OpportunityCallCoachingTab.tsx (1)
67-67: TheonRetryhandler is a no-op stub.The
onRetry={() => undefined}callback does nothing when invoked. If retry functionality is expected, this needs implementation; otherwise, consider making the prop optional inCoachingPanelor adding a// STUB:comment per coding guidelines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/OpportunityCallCoachingTab.tsx` at line 67, The onRetry prop passed to CoachingPanel in OpportunityCallCoachingTab is a no-op (onRetry={() => undefined}); either implement the actual retry logic or mark it explicitly as a stub. Locate the CoachingPanel usage in OpportunityCallCoachingTab and replace the no-op with the real retry handler (e.g., call the function that re-fetches coaching data or replays the call), or update CoachingPanel's props to make onRetry optional and remove the prop here, or add a clear "// STUB: implement onRetry" comment to satisfy coding guidelines so reviewers know this is intentionally unimplemented.packages/twenty-front/src/modules/dialer/components/OpportunityCallingWorkspace.tsx (1)
173-179: Conditional rendering for dynamic tabs is verbose but clear.The chain of
selectedTabId !==checks handles the fallback toPageLayoutMainContentfor custom page layout tabs. This works correctly but could be simplified with a Set or array check.💡 Suggested simplification
+const FIXED_TAB_IDS = new Set(['coaching', 'people', 'analytics', 'settings']); -{selectedTabId !== 'coaching' && - selectedTabId !== 'people' && - selectedTabId !== 'analytics' && - selectedTabId !== 'settings' && - currentPageLayout && ( +{!FIXED_TAB_IDS.has(selectedTabId) && currentPageLayout && ( <PageLayoutMainContent tabId={selectedTabId} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/OpportunityCallingWorkspace.tsx` around lines 173 - 179, The long chain of selectedTabId !== checks is verbose; replace it with an inclusion check (e.g., use an array or Set of reserved tab IDs) so the condition reads like: if selectedTabId is not in the reservedTabs AND currentPageLayout then render PageLayoutMainContent; update the condition that currently references selectedTabId, PageLayoutMainContent, and currentPageLayout to use reservedTabs.includes(selectedTabId) (or reservedTabs.has(selectedTabId)) negated for clarity.packages/twenty-front/src/modules/dialer/components/DialerHomeLivePanel.tsx (2)
141-141: MultipleonRetryhandlers are no-ops.Both
CoachingPanel(line 141) andPostCallSummary(line 157) receiveonRetry={() => undefined}. If retry functionality is planned, these need implementation. Consider adding// STUB:comments or making the prop optional.Also applies to: 157-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomeLivePanel.tsx` at line 141, CoachingPanel and PostCallSummary are both passed onRetry={() => undefined}, which is a no-op; replace these with either a real retry handler or mark the prop optional/stubbed: implement and pass a meaningful function (e.g., retryCoaching and retryPostCallSummary) that performs the intended retry logic, or change the component prop types for CoachingPanel.onRetry and PostCallSummary.onRetry to be optional and annotate the call sites with a clear "// STUB: implement retry" comment so future work knows it's intentionally unimplemented; update the corresponding prop definitions in the components to match (remove required or provide default no-op with comment) and ensure names like CoachingPanel and PostCallSummary are used to locate the code.
79-111: Consider extracting session reset logic into a reusable hook.The
resetSessionfunction resets 10 different Recoil atoms. This pattern is duplicated across components (e.g., when starting a new call flow). Extracting this touseResetDialerSessionwould improve maintainability and ensure consistent reset behavior.💡 Suggested hook extraction
// useResetDialerSession.ts export const useResetDialerSession = () => { const setCallState = useSetRecoilState(callStateAtom); const setPhoneNumber = useSetRecoilState(phoneNumberState); // ... other setters return useCallback(() => { setCallState({ status: 'idle', callSid: null, /* ... */ }); setPhoneNumber(''); // ... other resets }, [/* dependencies */]); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomeLivePanel.tsx` around lines 79 - 111, The resetSession function duplicates resetting many Recoil atoms; extract that logic into a new hook named useResetDialerSession that calls useSetRecoilState for callStateAtom, phoneNumberState, selectedContactState, talkingPointsState, coachingErrorState, transcriptState, transcriptConnectedState, postCallAnalysisState, isAnalyzingState, and analysisErrorState and returns a memoized callback (useCallback) that performs the same state resets; replace the local resetSession definition in DialerHomeLivePanel (and other components that duplicate this logic) with an import of useResetDialerSession and call the returned reset function to ensure a single source of truth for session resets and to include the proper setter dependencies in the hook.packages/twenty-front/src/modules/dialer/components/ScriptAssistPanel.tsx (1)
144-152: Consider the useEffect usage for state synchronization.While the coding guidelines prefer event handlers over
useEffectfor state updates, these effects respond to external Recoil state changes (script selection and transcript analysis) rather than direct user events. This is a reasonable pattern for derived/reactive state synchronization. However, ensure the second effect's dependency array is correct—activeSectionIndexis both read and written, which could cause unnecessary re-runs.💡 Alternative: use a ref to track previous value
+import { useRef } from 'react'; + +const previousSuggestedRef = useRef(suggestedSectionIndex); + useEffect(() => { - if (suggestedSectionIndex > activeSectionIndex) { + if (suggestedSectionIndex > previousSuggestedRef.current) { setActiveSectionIndex(suggestedSectionIndex); } -}, [activeSectionIndex, suggestedSectionIndex]); + previousSuggestedRef.current = suggestedSectionIndex; +}, [suggestedSectionIndex]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/ScriptAssistPanel.tsx` around lines 144 - 152, The second useEffect reads and writes activeSectionIndex which causes needless re-runs because activeSectionIndex is included in the dependency array; change it to only depend on suggestedSectionIndex and perform a functional update with setActiveSectionIndex(prev => Math.max(prev, suggestedSectionIndex)) (or alternatively use a ref to track the previous active value) so you compare/update safely without listing activeSectionIndex in the deps; update the useEffect that references useEffect, setActiveSectionIndex, activeSectionIndex, and suggestedSectionIndex accordingly.packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx (1)
172-180: Simplify the IIFE pattern for section count.The immediately-invoked function expression is unnecessary here. A simple inline expression or variable assignment is clearer.
♻️ Proposed simplification
- {(() => { - const sectionCount = selectedScriptSections.length; - - return ( - <StyledScriptMeta> - {t`${sectionCount} live sections detected`} - </StyledScriptMeta> - ); - })()} + <StyledScriptMeta> + {t`${selectedScriptSections.length} live sections detected`} + </StyledScriptMeta>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx` around lines 172 - 180, The IIFE around computing sectionCount is unnecessary; replace the self-invoking function with a simple inline expression or precomputed variable. Use selectedScriptSections.length directly (e.g., compute const sectionCount = selectedScriptSections.length above the JSX or embed it in the template) and render <StyledScriptMeta>{t`${sectionCount} live sections detected`}</StyledScriptMeta> (or {t`${selectedScriptSections.length} live sections detected`}) instead of the IIFE to simplify SettingsAIScripts rendering.packages/api/src/routes/calls.ts (1)
803-822: Prefer a static disposition UPDATE over runtime SQL assembly.The
setClausestemplate changes the query text per request and bypasses the repo rule against SQL string construction in.query(). A single constant withCOALESCEkeeps this path cacheable and easier to audit.🔧 Suggested fix
-const _SQL_SET_DISPOSITION = - 'UPDATE calls SET outcome = $1, notes = $2, updated_at = NOW() WHERE id = $3 AND workspace_id = $4 RETURNING id, outcome, notes'; +const SQL_SET_DISPOSITION = + 'UPDATE calls SET outcome = COALESCE($1, outcome), notes = COALESCE($2, notes), updated_at = NOW() WHERE (id::text = $3 OR call_sid = $3) AND workspace_id = $4 RETURNING id, outcome, notes'; ... - const setClauses: string[] = ['updated_at = NOW()']; - const params: unknown[] = []; - let idx = 1; - - if (body.outcome) { - setClauses.push(`outcome = $${idx}`); - params.push(body.outcome); - idx++; - } - - if (body.notes !== undefined) { - setClauses.push(`notes = $${idx}`); - params.push(body.notes); - idx++; - } - - params.push(callId, auth.workspaceId); - - const sql = `UPDATE calls SET ${setClauses.join(', ')} WHERE (id::text = $${idx} OR call_sid = $${idx}) AND workspace_id = $${idx + 1} RETURNING id, outcome, notes`; - const { rows } = await db.query(sql, params); + const { rows } = await db.query(SQL_SET_DISPOSITION, [ + body.outcome ?? null, + body.notes ?? null, + callId, + auth.workspaceId, + ]);As per coding guidelines: "packages/**/*.ts: no SQL template literals in .query() calls."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/calls.ts` around lines 803 - 822, Replace the runtime SQL assembly with a single constant UPDATE that uses COALESCE to keep columns unchanged when params are null: create a constant SQL like "UPDATE calls SET updated_at = NOW(), outcome = COALESCE($1, outcome), notes = COALESCE($2, notes) WHERE (id::text = $3 OR call_sid = $3) AND workspace_id = $4 RETURNING id, outcome, notes" and call db.query(sql, [body.outcome ?? null, body.notes ?? null, callId, auth.workspaceId]); remove setClauses, params, idx and the dynamic sql variable so the .query() call uses a static string (referencing setClauses, params, idx, sql, db.query in the diff to find and replace).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17cae9dc-090f-434b-adab-efc1e1d6ea97
⛔ Files ignored due to path filters (1)
consuelo-logo-animated.svgis excluded by!**/*.svg
📒 Files selected for processing (59)
.gitignorepackages/api/src/routes/__tests__/number-packs.spec.tspackages/api/src/routes/calls.tspackages/cli/src/commands/update.tspackages/dialer/src/services/conference.tspackages/twenty-docs/docs.jsonpackages/twenty-docs/navigation/navigation.template.jsonpackages/twenty-docs/user-guide/graphql-api/analytics/connecting-external-analytics.mdxpackages/twenty-docs/user-guide/graphql-api/analytics/growth-metrics-dashboards.mdxpackages/twenty-docs/user-guide/graphql-api/analytics/scheduled-reports.mdxpackages/twenty-docs/user-guide/graphql-api/automation/data-cleanup-automation.mdxpackages/twenty-docs/user-guide/graphql-api/automation/internal-workflows-overview.mdxpackages/twenty-docs/user-guide/graphql-api/automation/lead-scoring-setup.mdxpackages/twenty-docs/user-guide/graphql-api/automation/slack-notifications.mdxpackages/twenty-docs/user-guide/graphql-api/general/bulk-update-script.mdxpackages/twenty-docs/user-guide/graphql-api/general/export-analytics.mdxpackages/twenty-docs/user-guide/graphql-api/general/import-automation.mdxpackages/twenty-docs/user-guide/graphql-api/general/internal-api-access.mdxpackages/twenty-docs/user-guide/graphql-api/general/rate-limits-and-best-practices.mdxpackages/twenty-docs/user-guide/graphql-api/getting-started/setting-up-internal-workspace.mdxpackages/twenty-docs/user-guide/graphql-api/overview.mdxpackages/twenty-docs/user-guide/graphql-api/troubleshooting/common-ops-issues.mdxpackages/twenty-front/src/modules/dialer/components/CallerIdSelectCard.tsxpackages/twenty-front/src/modules/dialer/components/ContactHeader.tsxpackages/twenty-front/src/modules/dialer/components/DialConfirmationModal.tsxpackages/twenty-front/src/modules/dialer/components/DialerHomeLivePanel.tsxpackages/twenty-front/src/modules/dialer/components/DialerHomePageContent.tsxpackages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsxpackages/twenty-front/src/modules/dialer/components/LocalPresenceIndicator.tsxpackages/twenty-front/src/modules/dialer/components/OpportunityCallAnalyticsTab.tsxpackages/twenty-front/src/modules/dialer/components/OpportunityCallCoachingTab.tsxpackages/twenty-front/src/modules/dialer/components/OpportunityCallPeopleTab.tsxpackages/twenty-front/src/modules/dialer/components/OpportunityCallingWorkspace.tsxpackages/twenty-front/src/modules/dialer/components/ScriptAssistPanel.tsxpackages/twenty-front/src/modules/dialer/components/__stories__/DialConfirmationModal.stories.tsxpackages/twenty-front/src/modules/dialer/components/__stories__/DialPad.stories.tsxpackages/twenty-front/src/modules/dialer/hooks/useCoaching.tspackages/twenty-front/src/modules/dialer/hooks/useCoachingScripts.tspackages/twenty-front/src/modules/dialer/hooks/useOpportunityQueueWorkspace.tspackages/twenty-front/src/modules/dialer/hooks/useTwilioDevice.tspackages/twenty-front/src/modules/dialer/states/callAssistModeState.tspackages/twenty-front/src/modules/dialer/states/callErrorState.tspackages/twenty-front/src/modules/dialer/states/coachingScriptsState.tspackages/twenty-front/src/modules/dialer/states/selectedCoachingScriptIdState.tspackages/twenty-front/src/modules/dialer/types/coachingScript.tspackages/twenty-front/src/modules/dialer/types/queue.tspackages/twenty-front/src/modules/dialer/utils/notificationSounds.tspackages/twenty-front/src/modules/dialer/utils/parseCoachingScript.tspackages/twenty-front/src/modules/object-record/record-show/components/PageLayoutRecordPageRenderer.tsxpackages/twenty-front/src/modules/telemetry/components/PostHogInitEffect.tsxpackages/twenty-front/src/modules/telemetry/lib/posthog.tspackages/twenty-front/src/pages/home/HomePage.tsxpackages/twenty-front/src/pages/object-record/RecordShowPage.tsxpackages/twenty-front/src/pages/settings/ai/SettingsAI.tsxpackages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsxpackages/twenty-front/src/pages/settings/ai/constants/SettingsAiTabs.tspackages/twenty-front/tsconfig.dialer.jsonpackages/twenty-server/src/engine/core-modules/telemetry/posthog.service.tspackages/twenty-shared/src/constants/DocumentationPaths.ts
| export const registerUpdate = (program: Command): void => { | ||
| program | ||
| .command('update') | ||
| .description('update the CLI to the latest version') | ||
| .action(async () => { | ||
| try { | ||
| await updateCommand(); | ||
| } catch (err: unknown) { | ||
| captureError(err, { command: 'update' }); | ||
| process.exit(1); | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other CLI commands implement --json and --quiet flags
rg -n "isJson|isQuiet" packages/cli/src/Repository: consuelohq/opensaas
Length of output: 7683
🏁 Script executed:
#!/bin/bash
# Search for isQuiet function definition and usage
rg -n "isQuiet|quiet" packages/cli/src/ | head -30Repository: consuelohq/opensaas
Length of output: 1252
🏁 Script executed:
#!/bin/bash
# Check how --json flag is registered in CLI commands - look at a few command files
cat -n packages/cli/src/commands/config.ts | head -50Repository: consuelohq/opensaas
Length of output: 1762
🏁 Script executed:
#!/bin/bash
# Look at the update.ts file in full to understand its current structure
cat -n packages/cli/src/commands/update.tsRepository: consuelohq/opensaas
Length of output: 5018
🏁 Script executed:
#!/bin/bash
# Check how --json is registered in index.ts - look for the json flag setup
cat -n packages/cli/src/index.ts | head -100Repository: consuelohq/opensaas
Length of output: 4263
🏁 Script executed:
#!/bin/bash
# Check if there are any commands that DON'T use isJson() for JSON output
rg -L "isJson" packages/cli/src/commands/*.ts 2>/dev/null | head -20Repository: consuelohq/opensaas
Length of output: 1843
Add JSON output support to the update command.
The update command doesn't support JSON output like other CLI commands do. The global --json flag (registered at the root level) is already available, but this command doesn't implement JSON output. Import isJson from ../output.js and provide JSON output for structured results, matching the pattern used in other commands like config, migrate, queue, etc.
The --quiet flag is already handled automatically by the output functions (log(), success(), error(), info()), which respect the global __consuelo_quiet flag, so no additional quiet handling is needed.
🛠️ Suggested fix
Update updateCommand to check isJson() and output JSON when requested:
-import { log, success, error, info } from '../output.js';
+import { log, success, error, info, json, isJson } from '../output.js';
export const updateCommand = async (): Promise<void> => {
try {
const currentVersion = getInstalledVersion();
- log(`current version: ${currentVersion}`);
+ if (isJson()) { json({ action: 'update', status: 'checking', current: currentVersion }); return; }
+ log(`current version: ${currentVersion}`);Then handle JSON output at other decision points (when update is available, when already latest, when update succeeds/fails).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/update.ts` around lines 138 - 150, The update
command needs to emit structured JSON when the global --json flag is used:
import isJson from ../output.js and update the flow in registerUpdate /
updateCommand to check isJson() at each outcome (update available, already
latest, success, failure) and call the JSON output path instead of plain text;
specifically, locate registerUpdate and updateCommand and where
captureError(err, { command: 'update' }) is used, add conditional branches using
isJson() to output a consistent JSON object with fields like { command:
'update', status: 'updated'|'up-to-date'|'failed', version?: string, error?:
string } and ensure errors passed to captureError are also included in the JSON
error output path so callers receive structured results.
| autoStartedItemIdRef.current = currentQueueItem.id; | ||
| setQueueItems((previousQueueItems) => | ||
| previousQueueItems.map((queueItem) => | ||
| queueItem.id === currentQueueItem.id | ||
| ? { | ||
| ...queueItem, | ||
| status: 'calling', | ||
| attempts: queueItem.attempts + 1, | ||
| lastAttemptAt: new Date().toISOString(), | ||
| } | ||
| : queueItem, | ||
| ), | ||
| ); | ||
| setSelectedContact(currentQueueItem.contact); | ||
| setPhoneNumber(currentQueueItem.contact.phone); | ||
| setCallState((previousCallState) => ({ | ||
| ...previousCallState, | ||
| contact: currentQueueItem.contact, | ||
| fromNumber, | ||
| })); | ||
|
|
||
| try { | ||
| await connect({ To: currentQueueItem.contact.phone, From: fromNumber }); | ||
| } catch (error: unknown) { | ||
| Sentry.captureException(error, { | ||
| extra: { context: 'startCurrentQueueItem', listId }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Reset the auto-start guard when connect() fails.
autoStartedItemIdRef.current is set before the Twilio connect attempt, but the catch path never clears it or reverts the queue item out of calling. After a transient dial failure, the effect at Lines 411-421 will refuse to start this item again, so the workspace gets stuck on a permanently "calling" entry.
🐛 Proposed fix
try {
await connect({ To: currentQueueItem.contact.phone, From: fromNumber });
} catch (error: unknown) {
+ autoStartedItemIdRef.current = null;
+ setQueueItems((previousQueueItems) =>
+ previousQueueItems.map((queueItem) =>
+ queueItem.id === currentQueueItem.id
+ ? {
+ ...queueItem,
+ status: 'pending',
+ }
+ : queueItem,
+ ),
+ );
Sentry.captureException(error, {
extra: { context: 'startCurrentQueueItem', listId },
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/twenty-front/src/modules/dialer/hooks/useOpportunityQueueWorkspace.ts`
around lines 372 - 399, On connect failure you must clear the auto-start guard
and revert the optimistic "calling" update so the item can be retried: inside
the catch block for connect({ To: ..., From: ... }) clear
autoStartedItemIdRef.current (set to undefined/null) and call setQueueItems to
map over previousQueueItems and, for the item with id === currentQueueItem.id,
restore its prior state (e.g., set status back to 'queued' or previous status
and undo the attempts increment or adjust lastAttemptAt as appropriate). Use the
existing identifiers autoStartedItemIdRef.current, currentQueueItem,
setQueueItems and connect so the code location is clear.
| void recordResult(currentQueueItem.id, { | ||
| disposition: 'NO_ANSWER', | ||
| callSid: callState.callSid ?? '', | ||
| duration: callState.duration, | ||
| }); | ||
| markQueueItemCompleted(currentQueueItem.id, 'no-answer'); | ||
|
|
||
| autoAdvanceTimerRef.current = setTimeout(() => { | ||
| if (hasNextQueueItem) { | ||
| void advanceQueue(listId); | ||
| } else { | ||
| void completeQueue(listId); | ||
| } | ||
| setWrapUpState(null); | ||
| autoStartedItemIdRef.current = null; | ||
| }, 1200); |
There was a problem hiding this comment.
Await the no-answer write before auto-advancing.
This branch fire-and-forgets recordResult() and advances after 1.2s regardless of whether the write succeeded. If that request fails, the UI moves on but the backend still considers the member unresolved, which can requeue the same contact on refresh or resume.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/twenty-front/src/modules/dialer/hooks/useOpportunityQueueWorkspace.ts`
around lines 447 - 462, The code currently fire-and-forgets recordResult(...)
then auto-advances; change this so recordResult is awaited and any error is
handled before scheduling or performing the auto-advance. Specifically, await
the Promise returned by recordResult(currentQueueItem.id, {...}) (instead of
calling it void), only call markQueueItemCompleted(currentQueueItem.id,
'no-answer') and set the autoAdvanceTimerRef.current after the await succeeds,
and on failure log/handle the error and avoid advancing (or implement a
retry/backoff). Keep references: recordResult, markQueueItemCompleted,
autoAdvanceTimerRef, advanceQueue, completeQueue.
| useEffect(() => { | ||
| initPostHog({ | ||
| apiKey: POSTHOG_API_KEY, | ||
| host: POSTHOG_HOST, | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
Gate PostHog initialization behind cookie consent.
PostHog is initialized unconditionally on mount. Please guard this call with the app’s consent state so analytics does not start before user opt-in.
Based on learnings "Load PostHog analytics only after user accepts cookies, and track: page views, scroll depth, CTA clicks, section visibility, time on page, and outbound link clicks".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/twenty-front/src/modules/telemetry/components/PostHogInitEffect.tsx`
around lines 20 - 25, The PostHog initialization inside useEffect (initPostHog
with POSTHOG_API_KEY and POSTHOG_HOST) currently runs unconditionally; change it
to run only when the app's cookie consent state indicates opt-in (e.g., check
your ConsentContext/useConsent or consentStore) by reading that consent value
inside the effect and making it a dependency so initPostHog is called only when
consent === 'accepted' (or equivalent), and avoid initializing when consent is
false/declined; ensure the effect also handles consent changes (initialize on
accept, no-op otherwise) and keep initPostHog/POSTHOG_API_KEY/POSTHOG_HOST as
the referenced symbols to locate the code.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/api/src/routes/calls.ts (2)
723-757:⚠️ Potential issue | 🟠 MajorDon't turn Twilio lookup failures into
404 No recording available.Both catch blocks capture the exception and then fall through to the same
404used for “looked successfully and found nothing.” That hides Twilio auth/network failures from the client and sends the UI down the wrong branch. Return502after any lookup exception, and keep404for the true no-recording case.One way to split transport failures from real misses
+ let recordingLookupFailed = false; if (recordingSid) { try { const recording = await getLegacyDialer().getRecording(recordingSid); res .status(200) .json({ url: recording.url, duration: recording.duration }); return; } catch (err: unknown) { + recordingLookupFailed = true; Sentry.captureException( err instanceof Error ? err : new Error(String(err)), { extra: { context: 'getRecording', recordingSid }, }, ); } } if (conferenceName) { try { const recordings = await getLegacyDialer().conference.listRecordings(conferenceName); if (recordings.length > 0) { res.status(200).json({ url: recordings[0].url, duration: recordings[0].duration, }); return; } } catch (err: unknown) { + recordingLookupFailed = true; Sentry.captureException( err instanceof Error ? err : new Error(String(err)), { extra: { context: 'listRecordings', conferenceName }, }, ); } } - res.status(404).json({ - error: { code: 'NOT_FOUND', message: 'No recording available' }, - }); + res.status(recordingLookupFailed ? 502 : 404).json({ + error: recordingLookupFailed + ? { code: 'TWILIO_ERROR', message: 'Failed to fetch recording' } + : { code: 'NOT_FOUND', message: 'No recording available' }, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/calls.ts` around lines 723 - 757, The current catch blocks around getLegacyDialer().getRecording(recordingSid) and getLegacyDialer().conference.listRecordings(conferenceName) swallow Twilio errors and fall through to the generic 404; change them so that after Sentry.captureException(...) you send a 502 (Bad Gateway) response and return immediately to indicate an upstream/transport failure, while keeping the existing 404 response only for the non-exception “no recordings found” path (i.e., when recordings.length === 0 or recording is undefined). Update the try/catch for both getRecording and conference.listRecordings to return res.status(502).json({...}) after logging the error and ensure the code still returns normally on successful lookups.
220-227:⚠️ Potential issue | 🟠 MajorCarry workspace context through the callback TwiML leg.
Line 223 now starts the agent leg with
getDialerForWorkspace(...), but Lines 286-293 still build the customer leg throughgetLegacyDialer()and the callback URL does not carry any workspace token. In a non-default workspace that splits one conference across two dialer contexts, so the second leg can be created against the wrong Twilio account or fail outright. Pass a signed workspace lookup key throughtwimlUrland resolve the same workspace dialer inside/v1/calls/callback/twiml.Also applies to: 286-293
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/calls.ts` around lines 220 - 227, The callback TwiML URL currently lacks workspace context causing the second leg to resolve with getLegacyDialer() instead of the workspace-specific dialer; update the call creation paths (where createCall is invoked with twimlUrl and the customer-leg code that uses getLegacyDialer()) to append a signed workspace lookup key/token to twimlUrl, and then in the /v1/calls/callback/twiml handler read and verify that signed workspace key and use getDialerForWorkspace(workspaceId) (or the same resolver used earlier) to construct the dialer for both agent and customer legs so both legs use the same workspace dialer. Ensure the signing/verifying uses the existing signing utility in the codebase and that fallbacks/error handling are preserved.
♻️ Duplicate comments (1)
packages/api/src/routes/calls.ts (1)
421-427:⚠️ Potential issue | 🟠 MajorVerify the rep leg still emits the intermediate status callbacks.
dialer.createCall()falls back to['completed']whenstatusCallbackEventis omitted. If/v1/voice/phone-statusstill waits forcallStatus === 'in-progress'before dialing the lead, removing the explicit events here stalls the phone-initiation flow after the rep answers.Minimal fix if
voice.tsstill depends onin-progressconst { callSid: repCallSid } = await dialer.createCall( body.repPhone, callerId, { twiml, statusCallback: statusCallbackUrl, + statusCallbackEvent: [ + 'initiated', + 'ringing', + 'in-progress', + 'completed', + ], }, );Run this to confirm the dependency before applying the fix:
#!/bin/bash set -euo pipefail rg -n -C4 'phone-status|callStatus|in-progress' packages/api/src/routes/voice.ts rg -n -C4 'statusCallbackEvent|calls\.create' packages/dialer/src/services/conference.ts rg -n -C4 'initiate-phone|statusCallback' packages/api/src/routes/calls.tsExpected result:
packages/api/src/routes/voice.tsstill branches onin-progress, whilepackages/dialer/src/services/conference.tsstill defaults missingstatusCallbackEventto['completed'].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/calls.ts` around lines 421 - 427, dialer.createCall currently falls back to only ['completed'] when statusCallbackEvent is omitted, which can stall /v1/voice/phone-status if it waits for 'in-progress'; update the createCall call in packages/api/src/routes/calls.ts (the invocation using dialer.createCall and variables repCallSid, twiml, statusCallbackUrl) to pass an explicit statusCallbackEvent array that includes intermediate events (at minimum 'ringing' and 'in-progress', e.g. ['ringing','in-progress','completed']) so the rep leg emits the intermediate callbacks the existing /v1/voice/phone-status (callStatus checks) expects.
🧹 Nitpick comments (12)
packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx (3)
355-367: Script Select shows empty dropdown when no scripts exist.When
scriptsarray is empty andassistMode === 'script', the Select dropdown renders with no options. Consider showing a message prompting the user to create scripts first, or disabling the dropdown.♻️ Suggested improvement
{assistMode === 'script' && ( - <Select - dropdownId="dialer-home-script-select" - fullWidth - label={t`Script`} - value={selectedScriptId ?? ''} - onChange={(value) => setSelectedScriptId(value)} - options={scripts.map((script) => ({ - value: script.id, - label: script.name, - }))} - /> + scripts.length > 0 ? ( + <Select + dropdownId="dialer-home-script-select" + fullWidth + label={t`Script`} + value={selectedScriptId ?? ''} + onChange={(value) => setSelectedScriptId(value)} + options={scripts.map((script) => ({ + value: script.id, + label: script.name, + }))} + /> + ) : ( + <StyledDescription> + {t`No scripts available. Create one in settings.`} + </StyledDescription> + ) )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx` around lines 355 - 367, When assistMode === 'script' and scripts is empty the Select (dropdownId="dialer-home-script-select") renders no options; change the render logic in DialerHomePrep so that when scripts.length === 0 you show a disabled Select with a single placeholder option/label like "No scripts available — create a script" (or alternatively render a small helper message next to the Select) and ensure selectedScriptId is set to '' and onChange is no-op to avoid errors; update handling around selectedScriptId and setSelectedScriptId so the component remains controlled and accessible when no scripts exist.
185-239: Source cards lack accessible labels for screen readers.The
StyledSourceCardbuttons contain icons and text, but the icons don't havearia-labelattributes. While the text content provides context, addingaria-hidden="true"to decorative icons would be cleaner for accessibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx` around lines 185 - 239, The source card buttons (StyledSourceCard) include decorative icon components (IconUser, IconList, IconUpload, IconRobot) that lack accessibility attributes; update each icon usage inside those cards to be marked aria-hidden="true" (e.g., <IconUser aria-hidden="true" ...>) so screen readers ignore the decorative SVGs while the button remains labelled by its text (personLabel or the StyledSourceTitle text) and onClick handlers (setSource). Ensure all instances (IconUser, IconList, IconUpload, IconRobot) are updated consistently.
1-29: External import at line 27 should be grouped with other externals.The
twenty-ui/displayimport on line 27 appears after internal@/imports (lines 11-25). Per coding guidelines, external imports should be grouped together before internal imports.♻️ Move line 27 import up with other externals
import { Button } from 'twenty-ui/input'; import { Card } from 'twenty-ui/layout'; +import { IconList, IconRobot, IconUpload, IconUser } from 'twenty-ui/display'; import { useOpenAskAIPageInCommandMenu } from '@/command-menu/hooks/useOpenAskAIPageInCommandMenu'; ... import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled'; -import { IconList, IconRobot, IconUpload, IconUser } from 'twenty-ui/display'; - import { FeatureFlagKey } from '~/generated-metadata/graphql';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx` around lines 1 - 29, The import from 'twenty-ui/display' is an external import but is placed after internal '@/...' imports in DialerHomePrep.tsx; move the line importing IconList, IconRobot, IconUpload, IconUser (the twenty-ui/display import) up into the external imports block (with other externals like '@emotion/styled', '@lingui/core/macro', 'react', 'react-router-dom', 'recoil', and 'twenty-ui/input'/'twenty-ui/layout') so all third-party imports are grouped together before the internal '@/...' imports.packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx (2)
140-147: Consider adding a confirmation dialog before deleting scripts.Clicking "Delete" immediately calls
deleteScript(selectedScript.id)with no confirmation. Accidental deletions could lose script content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx` around lines 140 - 147, The Delete button currently calls deleteScript(selectedScript.id) immediately; add a confirmation step so users must confirm before deletion. Modify the Button onClick handler in SettingsAIScripts (where Button, StyledActions and deleteScript are used) to first show a confirmation dialog or modal (e.g., window.confirm or the app’s ConfirmModal component) and only call deleteScript(selectedScript.id) when the user confirms; ensure the handler also guards against when selectedScript is null/undefined.
1-8: Import order should be: external libraries first, then internal (@/).Per coding guidelines, imports should be ordered: external libraries first, then internal absolute imports using
@/, then relative imports. Currently, internal@/imports appear before external ones (@emotion/styled,@lingui/core/macro,twenty-ui/*).♻️ Suggested import reordering
-import { useCoachingScripts } from '@/dialer/hooks/useCoachingScripts'; -import { TextArea } from '@/ui/input/components/TextArea'; -import { TextInput } from '@/ui/input/components/TextInput'; import styled from '@emotion/styled'; import { t } from '@lingui/core/macro'; import { H2Title, IconTrash } from 'twenty-ui/display'; import { Button } from 'twenty-ui/input'; import { Card, Section } from 'twenty-ui/layout'; + +import { useCoachingScripts } from '@/dialer/hooks/useCoachingScripts'; +import { TextArea } from '@/ui/input/components/TextArea'; +import { TextInput } from '@/ui/input/components/TextInput';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx` around lines 1 - 8, Reorder the imports so external libraries come first, then internal "@/..." imports: move imports like styled from '@emotion/styled', { t } from '@lingui/core/macro', and twenty-ui imports (H2Title, IconTrash, Button, Card, Section) above the internal imports (useCoachingScripts, TextArea, TextInput); keep the same named symbols (useCoachingScripts, TextArea, TextInput, styled, t, H2Title, IconTrash, Button, Card, Section) and grouping but change their order to follow external → internal → relative convention..agents/skills/monitor-ci/scripts/ci-poll-decide.mjs (1)
126-130: Timeout calculation is a rough approximation, not cumulative time.The
isTimedOut()function usespollCount * avgDelayas a heuristic, but this doesn't account for actual cumulative delays (which vary based on backoff). For example, with 5 polls, actual delays might be 60+60+90+90+120=420s, but this calculates 5*90=450s.This is likely acceptable for a "good enough" timeout gate, but be aware it may trigger slightly earlier or later than the configured timeout. If precision matters, consider tracking elapsed wall-clock time instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs around lines 126 - 130, The current isTimedOut() uses pollCount * avgDelay which is an approximation; instead record a start timestamp when polling begins (e.g., startTime or pollStartTs) and change isTimedOut() to compare elapsed wall-clock time against timeoutSeconds (compute elapsed = (Date.now() - startTime) / 1000 and return elapsed >= timeoutSeconds). Update any initialization point that begins polling to set startTime and keep references to timeoutSeconds, pollCount and backoff unchanged..agents/skills/monitor-ci/SKILL.md (2)
144-160: Add language specifier to fenced code block.The pseudocode block is missing a language specifier.
📝 Proposed fix
-``` +```text cycle_count = 0 # Only incremented for agent-initiated cycles (counted against --max-cycles) start_time = now() ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/monitor-ci/SKILL.md around lines 144 - 160, The fenced pseudocode block in .agents/skills/monitor-ci/SKILL.md lacks a language specifier; update the code fence to include a language (e.g., ```text) so syntax renderers treat it as plain text. Locate the block containing variables like cycle_count, start_time, agent_triggered, prev_status, etc., and change the opening fence from ``` to ```text (or another appropriate language) while leaving the block contents unchanged.
46-48: Add language specifier to fenced code block.The fenced code block is missing a language specifier, which triggers a markdown lint warning.
📝 Proposed fix
- ``` + ```text Nx Cloud not connected. Unlock 70% faster CI and auto-fix broken PRs with https://nx.dev/nx-cloud ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/monitor-ci/SKILL.md around lines 46 - 48, The fenced code block that contains the line "Nx Cloud not connected. Unlock 70% faster CI and auto-fix broken PRs with https://nx.dev/nx-cloud" is missing a language specifier; update that fenced block to include a language token (e.g., change ``` to ```text) so the markdown linter warning is resolved..opencode/skills/monitor-ci/SKILL.md (1)
1-301: Duplicate: Third copy of monitor-ci skill documentation.This file, along with
.opencode/commands/monitor-ci.mdand.agents/skills/monitor-ci/SKILL.md, represents three nearly-identical copies of the same documentation. This significantly increases maintenance burden and risk of drift.Consider consolidating into a single source of truth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/monitor-ci/SKILL.md around lines 1 - 301, This SKILL.md is a duplicate of two other monitor-ci docs (all declare name: monitor-ci); pick one canonical source (e.g., the existing agents skill or a single docs folder) and consolidate content into that file, then remove the extra copies and replace them with redirects or symlinks and update any tooling/config that references the removed files; search for the unique symbol "name: monitor-ci" and ensure the chosen canonical file contains the full config (defaults, scripts, MCP mappings, statuses) and that callers/subagents (ci-poll-decide.mjs, ci-state-update.mjs, ci_information/update_self_healing_fix usage) point to that canonical document..opencode/commands/monitor-ci.md (1)
1-301: Note: Near-duplicate of.agents/skills/monitor-ci/SKILL.md.This file is nearly identical to
.agents/skills/monitor-ci/SKILL.md, differing only in frontmatter format. While this may be intentional for supporting multiple agent frameworks, consider whether a shared source or symlink could reduce maintenance burden if these need to stay in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/commands/monitor-ci.md around lines 1 - 301, The document is a near-duplicate of SKILL.md (same monitor-ci content with only frontmatter format differences); remove duplication by making one canonical source and having the other reference it (either replace this monitor-ci.md with a small frontmatter-only wrapper that imports/includes the canonical SKILL.md content, or turn monitor-ci.md into a symlink to the canonical SKILL.md), ensure the canonical file retains the full body and correct frontmatter format, and update any tooling/CI that expects the alternate frontmatter to parse the wrapper or canonical file; look for the two identifiers "monitor-ci.md" and "SKILL.md" and consolidate so only one authoritative copy of the monitor-ci content is maintained..opencode/skills/monitor-ci/scripts/ci-state-update.mjs (1)
1-160: Consolidate duplicate state management script: The file.opencode/skills/monitor-ci/scripts/ci-state-update.mjsis byte-for-byte identical to.agents/skills/monitor-ci/scripts/ci-state-update.mjs. To avoid maintenance drift and reduce duplication, consider moving the script to a single shared location (e.g.,scripts/monitor-ci/ci-state-update.mjs) and symlinking or importing it from both agent framework directories.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/monitor-ci/scripts/ci-state-update.mjs around lines 1 - 160, The ci-state-update.mjs script is duplicated; consolidate by moving the single authoritative implementation to a shared location (e.g., scripts/monitor-ci/ci-state-update.mjs) and replace the copies with references; update both locations (.opencode/skills/monitor-ci/scripts/ci-state-update.mjs and .agents/skills/monitor-ci/scripts/ci-state-update.mjs) to either be a symlink to the shared file or a tiny loader that imports/executes the shared implementation (ensure gate, postAction, cycleCheck and the dispatch logic remain identical and exported/connected so command dispatch still works)..opencode/skills/monitor-ci/scripts/ci-poll-decide.mjs (1)
1-428: Consolidate duplicate script to single shared location.The script is identical to
.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs. Maintain one copy to avoid desynchronization when the decision engine logic changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/monitor-ci/scripts/ci-poll-decide.mjs around lines 1 - 428, This file is a duplicate of the same CI decision logic — consolidate by removing the duplicated implementation and delegating to the single canonical module that implements classify(), buildOutput(), messages, and related helpers (backoff, isNewCipe, formatMessage, etc.). Replace this file’s full implementation with a thin CLI wrapper that imports/require() the canonical module's exported functions (classify/buildOutput and any needed arg-parsing helper) and forwards the parsed args (pollCount, verbosity, waitMode, prevCipeUrl, expectedSha, timeoutSeconds, newCipeTimeoutSeconds, envRerunCount, inputNoProgressCount, prev* statuses, etc.); ensure noProgressCount computation and the final buildOutput(classify()) call remain behaviorally identical, and update any entrypoints/scripts to point to the single shared file so only one copy of the decision engine exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/nx-import/SKILL.md:
- Line 37: Fix the spacing typo in the SKILL.md sentence by inserting a space
between the backticked fragment and the following word: change the text snippet
"`libs/`but" to "`libs/` but" so the sentence reads correctly and preserves
surrounding punctuation and backticks.
In @.agents/skills/nx-run-tasks/SKILL.md:
- Around line 18-20: The three fenced command blocks in SKILL.md (`nx run
<project>:<task>`, `nx run-many -t build test lint typecheck`, and `nx affected
-t build test lint`) are missing language identifiers and trigger markdownlint
MD040; update each triple-backtick fence to include the bash language tag (i.e.,
change ``` to ```bash) for those code blocks so the commands are fenced as bash
snippets.
In @.opencode/skills/link-workspace-packages/SKILL.md:
- Around line 76-78: Update the example JSON and explanatory text to reflect
npm's actual behavior: change the dependency entry for "@org/ui" in the example
from "*" to a concrete version string (e.g., "1.2.3") and replace the sentence
"npm resolves to local workspace automatically during install." with "npm saves
the exact version of the local workspace package. Replace \"1.2.3\" with the
actual version of `@org/ui`."—look for the documented JSON snippet and the
explanatory sentence around the "@org/ui" example in SKILL.md to make these
edits.
In @.opencode/skills/nx-import/references/JEST.md:
- Around line 62-64: Add the missing shell language tag "bash" to the fenced
code blocks that contain the pnpm install commands so markdownlint MD040 is
satisfied; specifically update the fences wrapping the commands "pnpm add -wD
jest ts-jest `@types/jest` `@nx/jest`", "pnpm add -wD `@testing-library/react`
`@testing-library/jest-dom`", "pnpm add -wD babel-jest `@babel/core`
`@babel/preset-env` `@babel/preset-react` `@babel/preset-typescript`", and "pnpm add
-wD `@vue/test-utils`" by changing the opening ``` to ```bash for each block.
In @.opencode/skills/nx-import/SKILL.md:
- Line 37: In the SKILL.md documentation sentence that reads "`Always prefer the
destination's existing conventions. Source uses `libs/`but dest uses
`packages/`? Import into `packages/` (`nx import <source> packages/foo
--source=libs/foo`).`" fix the spacing typo by inserting a space after the
backticked `libs/` so it becomes "`libs/` but", preserving the rest of the
sentence and formatting.
In @.opencode/skills/nx-run-tasks/SKILL.md:
- Around line 18-20: Update the three fenced code blocks that show command
examples to include a language identifier by changing the opening backticks to
```bash; specifically add "bash" to the blocks containing the commands "nx run
<project>:<task>", "nx run-many -t build test lint typecheck", and "nx affected
-t build test lint" so they become fenced as ```bash for markdownlint MD040
compliance.
In `@packages/api/src/routes/calls.ts`:
- Around line 323-326: Normalize body.repPhone, body.leadPhone, and body.from
using normalizePhone() (from `@consuelo/contacts`) before performing E164_REGEX
validation or using them downstream; replace or assign the normalized values
(e.g., repPhoneNormalized, leadPhoneNormalized, fromNormalized or overwrite
body.repPhone/body.leadPhone/body.from) and then validate with
E164_REGEX.test(...) against those normalized variables so the regex checks
canonicalized input and subsequent logic uses the normalized numbers.
In `@packages/twenty-front/src/modules/dialer/components/CoachingPanel.tsx`:
- Around line 299-301: The Retry button label is hardcoded; replace the literal
"Retry" in the JSX for StyledRetryButton (the element rendered when onRetry is
set) with a Lingui-localized string (e.g., use the t macro or Trans component)
and add the necessary import from '@lingui/macro' or '@lingui/react' at the top
of the file; ensure the localized message id/text is used where
StyledRetryButton currently renders "Retry" so the button text is
internationalized.
In `@packages/twenty-front/src/modules/dialer/components/PostCallSummary.tsx`:
- Around line 252-255: The Retry Analysis label is hardcoded; change it to use
Lingui localization by replacing the literal "Retry Analysis" inside the
StyledRetryButton in PostCallSummary with a localized message (e.g., t`Retry
Analysis` or a <Trans> wrapper) and add the corresponding import from Lingui
(for example import { t } from '@lingui/macro') at the top of the file; keep the
onRetry handler and StyledRetryButton usage unchanged.
---
Outside diff comments:
In `@packages/api/src/routes/calls.ts`:
- Around line 723-757: The current catch blocks around
getLegacyDialer().getRecording(recordingSid) and
getLegacyDialer().conference.listRecordings(conferenceName) swallow Twilio
errors and fall through to the generic 404; change them so that after
Sentry.captureException(...) you send a 502 (Bad Gateway) response and return
immediately to indicate an upstream/transport failure, while keeping the
existing 404 response only for the non-exception “no recordings found” path
(i.e., when recordings.length === 0 or recording is undefined). Update the
try/catch for both getRecording and conference.listRecordings to return
res.status(502).json({...}) after logging the error and ensure the code still
returns normally on successful lookups.
- Around line 220-227: The callback TwiML URL currently lacks workspace context
causing the second leg to resolve with getLegacyDialer() instead of the
workspace-specific dialer; update the call creation paths (where createCall is
invoked with twimlUrl and the customer-leg code that uses getLegacyDialer()) to
append a signed workspace lookup key/token to twimlUrl, and then in the
/v1/calls/callback/twiml handler read and verify that signed workspace key and
use getDialerForWorkspace(workspaceId) (or the same resolver used earlier) to
construct the dialer for both agent and customer legs so both legs use the same
workspace dialer. Ensure the signing/verifying uses the existing signing utility
in the codebase and that fallbacks/error handling are preserved.
---
Duplicate comments:
In `@packages/api/src/routes/calls.ts`:
- Around line 421-427: dialer.createCall currently falls back to only
['completed'] when statusCallbackEvent is omitted, which can stall
/v1/voice/phone-status if it waits for 'in-progress'; update the createCall call
in packages/api/src/routes/calls.ts (the invocation using dialer.createCall and
variables repCallSid, twiml, statusCallbackUrl) to pass an explicit
statusCallbackEvent array that includes intermediate events (at minimum
'ringing' and 'in-progress', e.g. ['ringing','in-progress','completed']) so the
rep leg emits the intermediate callbacks the existing /v1/voice/phone-status
(callStatus checks) expects.
---
Nitpick comments:
In @.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs:
- Around line 126-130: The current isTimedOut() uses pollCount * avgDelay which
is an approximation; instead record a start timestamp when polling begins (e.g.,
startTime or pollStartTs) and change isTimedOut() to compare elapsed wall-clock
time against timeoutSeconds (compute elapsed = (Date.now() - startTime) / 1000
and return elapsed >= timeoutSeconds). Update any initialization point that
begins polling to set startTime and keep references to timeoutSeconds, pollCount
and backoff unchanged.
In @.agents/skills/monitor-ci/SKILL.md:
- Around line 144-160: The fenced pseudocode block in
.agents/skills/monitor-ci/SKILL.md lacks a language specifier; update the code
fence to include a language (e.g., ```text) so syntax renderers treat it as
plain text. Locate the block containing variables like cycle_count, start_time,
agent_triggered, prev_status, etc., and change the opening fence from ``` to
```text (or another appropriate language) while leaving the block contents
unchanged.
- Around line 46-48: The fenced code block that contains the line "Nx Cloud not
connected. Unlock 70% faster CI and auto-fix broken PRs with
https://nx.dev/nx-cloud" is missing a language specifier; update that fenced
block to include a language token (e.g., change ``` to ```text) so the markdown
linter warning is resolved.
In @.opencode/commands/monitor-ci.md:
- Around line 1-301: The document is a near-duplicate of SKILL.md (same
monitor-ci content with only frontmatter format differences); remove duplication
by making one canonical source and having the other reference it (either replace
this monitor-ci.md with a small frontmatter-only wrapper that imports/includes
the canonical SKILL.md content, or turn monitor-ci.md into a symlink to the
canonical SKILL.md), ensure the canonical file retains the full body and correct
frontmatter format, and update any tooling/CI that expects the alternate
frontmatter to parse the wrapper or canonical file; look for the two identifiers
"monitor-ci.md" and "SKILL.md" and consolidate so only one authoritative copy of
the monitor-ci content is maintained.
In @.opencode/skills/monitor-ci/scripts/ci-poll-decide.mjs:
- Around line 1-428: This file is a duplicate of the same CI decision logic —
consolidate by removing the duplicated implementation and delegating to the
single canonical module that implements classify(), buildOutput(), messages, and
related helpers (backoff, isNewCipe, formatMessage, etc.). Replace this file’s
full implementation with a thin CLI wrapper that imports/require() the canonical
module's exported functions (classify/buildOutput and any needed arg-parsing
helper) and forwards the parsed args (pollCount, verbosity, waitMode,
prevCipeUrl, expectedSha, timeoutSeconds, newCipeTimeoutSeconds, envRerunCount,
inputNoProgressCount, prev* statuses, etc.); ensure noProgressCount computation
and the final buildOutput(classify()) call remain behaviorally identical, and
update any entrypoints/scripts to point to the single shared file so only one
copy of the decision engine exists.
In @.opencode/skills/monitor-ci/scripts/ci-state-update.mjs:
- Around line 1-160: The ci-state-update.mjs script is duplicated; consolidate
by moving the single authoritative implementation to a shared location (e.g.,
scripts/monitor-ci/ci-state-update.mjs) and replace the copies with references;
update both locations (.opencode/skills/monitor-ci/scripts/ci-state-update.mjs
and .agents/skills/monitor-ci/scripts/ci-state-update.mjs) to either be a
symlink to the shared file or a tiny loader that imports/executes the shared
implementation (ensure gate, postAction, cycleCheck and the dispatch logic
remain identical and exported/connected so command dispatch still works).
In @.opencode/skills/monitor-ci/SKILL.md:
- Around line 1-301: This SKILL.md is a duplicate of two other monitor-ci docs
(all declare name: monitor-ci); pick one canonical source (e.g., the existing
agents skill or a single docs folder) and consolidate content into that file,
then remove the extra copies and replace them with redirects or symlinks and
update any tooling/config that references the removed files; search for the
unique symbol "name: monitor-ci" and ensure the chosen canonical file contains
the full config (defaults, scripts, MCP mappings, statuses) and that
callers/subagents (ci-poll-decide.mjs, ci-state-update.mjs,
ci_information/update_self_healing_fix usage) point to that canonical document.
In `@packages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsx`:
- Around line 355-367: When assistMode === 'script' and scripts is empty the
Select (dropdownId="dialer-home-script-select") renders no options; change the
render logic in DialerHomePrep so that when scripts.length === 0 you show a
disabled Select with a single placeholder option/label like "No scripts
available — create a script" (or alternatively render a small helper message
next to the Select) and ensure selectedScriptId is set to '' and onChange is
no-op to avoid errors; update handling around selectedScriptId and
setSelectedScriptId so the component remains controlled and accessible when no
scripts exist.
- Around line 185-239: The source card buttons (StyledSourceCard) include
decorative icon components (IconUser, IconList, IconUpload, IconRobot) that lack
accessibility attributes; update each icon usage inside those cards to be marked
aria-hidden="true" (e.g., <IconUser aria-hidden="true" ...>) so screen readers
ignore the decorative SVGs while the button remains labelled by its text
(personLabel or the StyledSourceTitle text) and onClick handlers (setSource).
Ensure all instances (IconUser, IconList, IconUpload, IconRobot) are updated
consistently.
- Around line 1-29: The import from 'twenty-ui/display' is an external import
but is placed after internal '@/...' imports in DialerHomePrep.tsx; move the
line importing IconList, IconRobot, IconUpload, IconUser (the twenty-ui/display
import) up into the external imports block (with other externals like
'@emotion/styled', '@lingui/core/macro', 'react', 'react-router-dom', 'recoil',
and 'twenty-ui/input'/'twenty-ui/layout') so all third-party imports are grouped
together before the internal '@/...' imports.
In
`@packages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx`:
- Around line 140-147: The Delete button currently calls
deleteScript(selectedScript.id) immediately; add a confirmation step so users
must confirm before deletion. Modify the Button onClick handler in
SettingsAIScripts (where Button, StyledActions and deleteScript are used) to
first show a confirmation dialog or modal (e.g., window.confirm or the app’s
ConfirmModal component) and only call deleteScript(selectedScript.id) when the
user confirms; ensure the handler also guards against when selectedScript is
null/undefined.
- Around line 1-8: Reorder the imports so external libraries come first, then
internal "@/..." imports: move imports like styled from '@emotion/styled', { t }
from '@lingui/core/macro', and twenty-ui imports (H2Title, IconTrash, Button,
Card, Section) above the internal imports (useCoachingScripts, TextArea,
TextInput); keep the same named symbols (useCoachingScripts, TextArea,
TextInput, styled, t, H2Title, IconTrash, Button, Card, Section) and grouping
but change their order to follow external → internal → relative convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b04c3a58-f424-46c6-a610-9cbf765d1633
📒 Files selected for processing (58)
.agents/skills/link-workspace-packages/SKILL.md.agents/skills/monitor-ci/SKILL.md.agents/skills/monitor-ci/references/fix-flows.md.agents/skills/monitor-ci/scripts/ci-poll-decide.mjs.agents/skills/monitor-ci/scripts/ci-state-update.mjs.agents/skills/nx-generate/SKILL.md.agents/skills/nx-import/SKILL.md.agents/skills/nx-import/references/ESLINT.md.agents/skills/nx-import/references/GRADLE.md.agents/skills/nx-import/references/JEST.md.agents/skills/nx-import/references/NEXT.md.agents/skills/nx-import/references/TURBOREPO.md.agents/skills/nx-import/references/VITE.md.agents/skills/nx-plugins/SKILL.md.agents/skills/nx-run-tasks/SKILL.md.agents/skills/nx-workspace/SKILL.md.agents/skills/nx-workspace/references/AFFECTED.md.codex/agents/ci-monitor-subagent.toml.codex/config.toml.cursor/rules/nx-rules.mdc.gitignore.opencode/agents/ci-monitor-subagent.md.opencode/commands/monitor-ci.md.opencode/skills/link-workspace-packages/SKILL.md.opencode/skills/monitor-ci/SKILL.md.opencode/skills/monitor-ci/references/fix-flows.md.opencode/skills/monitor-ci/scripts/ci-poll-decide.mjs.opencode/skills/monitor-ci/scripts/ci-state-update.mjs.opencode/skills/nx-generate/SKILL.md.opencode/skills/nx-import/SKILL.md.opencode/skills/nx-import/references/ESLINT.md.opencode/skills/nx-import/references/GRADLE.md.opencode/skills/nx-import/references/JEST.md.opencode/skills/nx-import/references/NEXT.md.opencode/skills/nx-import/references/TURBOREPO.md.opencode/skills/nx-import/references/VITE.md.opencode/skills/nx-plugins/SKILL.md.opencode/skills/nx-run-tasks/SKILL.md.opencode/skills/nx-workspace/SKILL.md.opencode/skills/nx-workspace/references/AFFECTED.mdAGENTS.mdopencode.jsonpackages/api/src/routes/calls.tspackages/cli/src/commands/update.tspackages/twenty-front/src/modules/dialer/components/CallerIdSelectCard.tsxpackages/twenty-front/src/modules/dialer/components/CoachingPanel.tsxpackages/twenty-front/src/modules/dialer/components/DialerHomeLivePanel.tsxpackages/twenty-front/src/modules/dialer/components/DialerHomePageContent.tsxpackages/twenty-front/src/modules/dialer/components/DialerHomePrep.tsxpackages/twenty-front/src/modules/dialer/components/OpportunityCallAnalyticsTab.tsxpackages/twenty-front/src/modules/dialer/components/OpportunityCallCoachingTab.tsxpackages/twenty-front/src/modules/dialer/components/PostCallSummary.tsxpackages/twenty-front/src/modules/dialer/components/__stories__/DialConfirmationModal.stories.tsxpackages/twenty-front/src/modules/dialer/hooks/useCoaching.tspackages/twenty-front/src/modules/dialer/utils/notificationSounds.tspackages/twenty-front/src/modules/object-record/record-show/components/PageLayoutRecordPageRenderer.tsxpackages/twenty-front/src/pages/home/HomePage.tsxpackages/twenty-front/src/pages/settings/ai/components/SettingsAIScripts.tsx
💤 Files with no reviewable changes (1)
- .cursor/rules/nx-rules.mdc
✅ Files skipped from review due to trivial changes (23)
- .agents/skills/nx-plugins/SKILL.md
- .agents/skills/nx-workspace/references/AFFECTED.md
- .opencode/skills/nx-workspace/references/AFFECTED.md
- opencode.json
- .agents/skills/nx-import/references/GRADLE.md
- .opencode/skills/nx-import/references/GRADLE.md
- .agents/skills/nx-generate/SKILL.md
- AGENTS.md
- .codex/agents/ci-monitor-subagent.toml
- .opencode/skills/nx-plugins/SKILL.md
- .agents/skills/nx-workspace/SKILL.md
- .opencode/skills/nx-workspace/SKILL.md
- .opencode/agents/ci-monitor-subagent.md
- .gitignore
- .agents/skills/monitor-ci/references/fix-flows.md
- .agents/skills/nx-import/references/ESLINT.md
- .opencode/skills/nx-import/references/TURBOREPO.md
- .opencode/skills/monitor-ci/references/fix-flows.md
- .agents/skills/link-workspace-packages/SKILL.md
- .agents/skills/nx-import/references/JEST.md
- .agents/skills/nx-import/references/TURBOREPO.md
- .opencode/skills/nx-import/references/NEXT.md
- .opencode/skills/nx-import/references/ESLINT.md
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/twenty-front/src/modules/dialer/components/DialerHomePageContent.tsx
- packages/twenty-front/src/modules/object-record/record-show/components/PageLayoutRecordPageRenderer.tsx
- packages/twenty-front/src/pages/home/HomePage.tsx
- packages/twenty-front/src/modules/dialer/components/OpportunityCallCoachingTab.tsx
- packages/twenty-front/src/modules/dialer/components/CallerIdSelectCard.tsx
- packages/twenty-front/src/modules/dialer/components/DialerHomeLivePanel.tsx
- packages/twenty-front/src/modules/dialer/components/OpportunityCallAnalyticsTab.tsx
- packages/cli/src/commands/update.ts
- packages/twenty-front/src/modules/dialer/components/stories/DialConfirmationModal.stories.tsx
- packages/twenty-front/src/modules/dialer/utils/notificationSounds.ts
|
|
||
| ### Directory Conventions | ||
|
|
||
| - **Always prefer the destination's existing conventions.** Source uses `libs/`but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`). |
There was a problem hiding this comment.
Fix spacing typo in destination-convention sentence.
At Line 37, "libs/but" is missing a space.
💡 Proposed fix
-- **Always prefer the destination's existing conventions.** Source uses `libs/`but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`).
+- **Always prefer the destination's existing conventions.** Source uses `libs/` but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Always prefer the destination's existing conventions.** Source uses `libs/`but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`). | |
| - **Always prefer the destination's existing conventions.** Source uses `libs/` but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nx-import/SKILL.md at line 37, Fix the spacing typo in the
SKILL.md sentence by inserting a space between the backticked fragment and the
following word: change the text snippet "`libs/`but" to "`libs/` but" so the
sentence reads correctly and preserves surrounding punctuation and backticks.
| ``` | ||
| nx run <project>:<task> | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced command blocks.
At Line 18, Line 26, and Line 42, the fenced blocks are missing a language tag, which triggers markdownlint MD040.
💡 Proposed fix
-```
+```bash
nx run <project>:<task>- +bash
nx run-many -t build test lint typecheck
-```
+```bash
nx affected -t build test lint
</details>
Also applies to: 26-28, 42-44
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nx-run-tasks/SKILL.md around lines 18 - 20, The three fenced
command blocks in SKILL.md (nx run <project>:<task>, nx run-many -t build test lint typecheck, and nx affected -t build test lint) are missing language
identifiers and trigger markdownlint MD040; update each triple-backtick fence to
include the bash language tag (i.e., change tobash) for those code
blocks so the commands are fenced as bash snippets.
</details>
<!-- fingerprinting:phantom:triton:hawk:e9d9ff6b-a03d-4e87-b997-235f19a9c39a -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ```json | ||
| { "dependencies": { "@org/ui": "*" } } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In npm workspaces, when running npm install --workspace , what dependency spec is written to the consumer package.json (e.g., exact version, caret range, or *) in current npm docs?
💡 Result:
In npm workspaces, running npm install <workspace-package> --workspace <consumer-workspace> writes the exact version of the workspace package to the consumer's package.json. This follows npm's standard behavior for npm install <package>, which saves the exact version resolved and installed (e.g., "1.2.3") by default, unless overridden by flags like --save-exact (not needed here) or config like save-prefix. For workspace packages, npm resolves to the local workspace's version since it's symlinked, and saves that exact version string. Official npm docs confirm npm install saves dependencies as exact versions by default when no range is specified in the command (docs.npmjs.com/cli/v10/commands/npm-install, docs.npmjs.com/files/package.json). StackOverflow and GitHub discussions (e.g., npm/cli #4588) show examples where cross-workspace dependencies are specified as exact versions like "0.0.0" or the resolved version, and version bumps update dependent ranges only if semver allows. Note: workspace:* protocol is not supported in current npm (as of 2026; see npm/cli #8845, npm/rfcs #765). Docs mentioning it are erroneous.
Citations:
- 1: https://docs.npmjs.com/cli/v9/using-npm/workspaces/?v=true
- 2: https://stackoverflow.com/questions/72851445/how-to-specify-an-npm-workspace-as-a-dependency
- 3: https://www.w3tutorials.net/blog/how-to-specify-an-npm-workspace-as-a-dependency/
- 4: https://npm.github.io/using-pkgs-docs/package-json/types-of-dependencies.html
- 5: https://stackoverflow.com/questions/41003470/npm-install-the-exact-package-version-specified-in-package-json
- 6: https://docs.npmjs.com/specifying-dependencies-and-devdependencies-in-a-package-json-file/
- 7: https://docs.npmjs.com/cli/v10/configuring-npm/package-json
- 8: https://docs.npmjs.com/cli/v9/configuring-npm/package-json/?v=true
- 9: [RRFC] Support for
workspace:protocol (Yarn 2) npm/rfcs#765 - 10: [DOCS] workspace: protocol documented but throws EUNSUPPORTEDPROTOCOL in npm 11.6.4 npm/cli#8845
- 11: https://stackoverflow.com/questions/65237564/npm-7-workspaces-how-to-install-new-package-in-workspace
- 12: https://docs.npmjs.com/cli/v8/commands/npm-install/
- 13: https://github.com/npm/cli/blob/latest/CHANGELOG.md
- 14: feat(version): reify on workspace version change npm/cli#4588
Correct npm workspace dependency spec in documentation.
The file documents "@org/ui": "*" as the result of npm workspace linking, but npm actually writes the exact resolved version of the workspace package (e.g., "@org/ui": "1.2.3"). Update this to show accurate behavior.
Suggested doc correction
-Result in `package.json`:
+Result in `package.json`:
```json
-{ "dependencies": { "@org/ui": "*" } }
+{ "dependencies": { "@org/ui": "1.2.3" } }-npm resolves to local workspace automatically during install.
+npm saves the exact version of the local workspace package. Replace "1.2.3" with the actual version of @org/ui.
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.opencode/skills/link-workspace-packages/SKILL.md around lines 76 - 78,
Update the example JSON and explanatory text to reflect npm's actual behavior:
change the dependency entry for "@org/ui" in the example from "*" to a concrete
version string (e.g., "1.2.3") and replace the sentence "npm resolves to local
workspace automatically during install." with "npm saves the exact version of
the local workspace package. Replace "1.2.3" with the actual version of
@org/ui."—look for the documented JSON snippet and the explanatory sentence
around the "@org/ui" example in SKILL.md to make these edits.
</details>
<!-- fingerprinting:phantom:triton:hawk:54fcd303-8a6b-45fb-8515-0b05d158d207 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ``` | ||
| pnpm add -wD jest ts-jest @types/jest @nx/jest | ||
| ``` |
There was a problem hiding this comment.
Specify shell language for command fences.
At Line 62, Line 73, Line 81, and Line 89, command blocks should use a language tag (bash) to satisfy markdownlint MD040.
💡 Proposed fix
-```
+```bash
pnpm add -wD jest ts-jest `@types/jest` `@nx/jest`- +bash
pnpm add -wD @testing-library/react @testing-library/jest-dom
-```
+```bash
pnpm add -wD babel-jest `@babel/core` `@babel/preset-env` `@babel/preset-react` `@babel/preset-typescript`
- +bash
pnpm add -wD @vue/test-utils
Also applies to: 73-75, 81-83, 89-91
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.opencode/skills/nx-import/references/JEST.md around lines 62 - 64, Add the
missing shell language tag "bash" to the fenced code blocks that contain the
pnpm install commands so markdownlint MD040 is satisfied; specifically update
the fences wrapping the commands "pnpm add -wD jest ts-jest `@types/jest`
`@nx/jest`", "pnpm add -wD `@testing-library/react` `@testing-library/jest-dom`",
"pnpm add -wD babel-jest `@babel/core` `@babel/preset-env` `@babel/preset-react`
`@babel/preset-typescript`", and "pnpm add -wD `@vue/test-utils`" by changing the
opening ``` to ```bash for each block.
|
|
||
| ### Directory Conventions | ||
|
|
||
| - **Always prefer the destination's existing conventions.** Source uses `libs/`but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`). |
There was a problem hiding this comment.
Fix spacing typo in destination-convention sentence.
At Line 37, there is a missing space in "libs/but", which reads as a typo in user-facing docs.
💡 Proposed fix
-- **Always prefer the destination's existing conventions.** Source uses `libs/`but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`).
+- **Always prefer the destination's existing conventions.** Source uses `libs/` but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Always prefer the destination's existing conventions.** Source uses `libs/`but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`). | |
| - **Always prefer the destination's existing conventions.** Source uses `libs/` but dest uses `packages/`? Import into `packages/` (`nx import <source> packages/foo --source=libs/foo`). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.opencode/skills/nx-import/SKILL.md at line 37, In the SKILL.md
documentation sentence that reads "`Always prefer the destination's existing
conventions. Source uses `libs/`but dest uses `packages/`? Import into
`packages/` (`nx import <source> packages/foo --source=libs/foo`).`" fix the
spacing typo by inserting a space after the backticked `libs/` so it becomes
"`libs/` but", preserving the rest of the sentence and formatting.
| ``` | ||
| nx run <project>:<task> | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
Line 18, Line 26, and Line 42 use unfenced-language code blocks, which triggers markdownlint MD040. Please mark them as bash for consistency and lint compliance.
Suggested fix
-```
+```bash
nx run <project>:<task>- +bash
nx run-many -t build test lint typecheck
-```
+```bash
nx affected -t build test lint
</details>
Also applies to: 26-28, 42-44
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.opencode/skills/nx-run-tasks/SKILL.md around lines 18 - 20, Update the
three fenced code blocks that show command examples to include a language
identifier by changing the opening backticks to bash; specifically add "bash" to the blocks containing the commands "nx run <project>:<task>", "nx run-many -t build test lint typecheck", and "nx affected -t build test lint" so they become fenced as bash for markdownlint MD040 compliance.
</details>
<!-- fingerprinting:phantom:triton:hawk:1a73db63-aa6b-4853-89e0-50fd95379886 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| if ( | ||
| !E164_REGEX.test(body.repPhone) || | ||
| !E164_REGEX.test(body.leadPhone) | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Normalize these phone inputs before validating them.
This still regex-checks raw user input, so formatting variants that normalizePhone() could canonicalize get rejected and downstream state depends on caller formatting. Normalize repPhone, leadPhone, and from first, then validate/use the normalized values.
As per coding guidelines, packages/{dialer,contacts,api}/src/**/*.ts: "Always normalize phone numbers using normalizePhone() from @consuelo/contacts."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/api/src/routes/calls.ts` around lines 323 - 326, Normalize
body.repPhone, body.leadPhone, and body.from using normalizePhone() (from
`@consuelo/contacts`) before performing E164_REGEX validation or using them
downstream; replace or assign the normalized values (e.g., repPhoneNormalized,
leadPhoneNormalized, fromNormalized or overwrite
body.repPhone/body.leadPhone/body.from) and then validate with
E164_REGEX.test(...) against those normalized variables so the regex checks
canonicalized input and subsequent logic uses the normalized numbers.
| {onRetry && ( | ||
| <StyledRetryButton onClick={onRetry}>Retry</StyledRetryButton> | ||
| )} |
There was a problem hiding this comment.
Localize the retry button label.
Line 300 uses hardcoded text (Retry). Please internationalize it with Lingui.
As per coding guidelines: "All text must be localized with Lingui".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/twenty-front/src/modules/dialer/components/CoachingPanel.tsx` around
lines 299 - 301, The Retry button label is hardcoded; replace the literal
"Retry" in the JSX for StyledRetryButton (the element rendered when onRetry is
set) with a Lingui-localized string (e.g., use the t macro or Trans component)
and add the necessary import from '@lingui/macro' or '@lingui/react' at the top
of the file; ensure the localized message id/text is used where
StyledRetryButton currently renders "Retry" so the button text is
internationalized.
| {onRetry && ( | ||
| <StyledRetryButton onClick={onRetry}> | ||
| Retry Analysis | ||
| </StyledRetryButton> |
There was a problem hiding this comment.
Localize new retry CTA text.
Line 254 adds a hardcoded user-facing string (Retry Analysis). Please route it through Lingui instead of inline text.
As per coding guidelines: "All text must be localized with Lingui".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/twenty-front/src/modules/dialer/components/PostCallSummary.tsx`
around lines 252 - 255, The Retry Analysis label is hardcoded; change it to use
Lingui localization by replacing the literal "Retry Analysis" inside the
StyledRetryButton in PostCallSummary with a localized message (e.g., t`Retry
Analysis` or a <Trans> wrapper) and add the corresponding import from Lingui
(for example import { t } from '@lingui/macro') at the top of the file; keep the
onRetry handler and StyledRetryButton usage unchanged.
…Extension; move GraphQL API under User Guide PR #66 accidentally deleted Dialer, HighLevel, CLI, Discord, Chrome Extension from the User Guide nav and added GraphQL API as a separate top-level tab. this restores the original nav ordering and adds GraphQL API as a group inside User Guide (after Settings).
what
58 files changed — all 13 code-review checks pass