fix: support modern macOS group chat ID format (any;+;) and add watcher checkpoint#43
Conversation
…er checkpoint
On modern macOS (Sequoia/Tahoe), Messages.app uses `any;+;{guid}` for group
chat IDs and `any;-;{phone}` for DMs in the `chat.guid` column. The previous
code hardcoded `iMessage;+;chat{guid}` which does not exist in modern chat.db.
Changes:
- isGroupChatId: recognize `;+;` separator (covers both `any;+;` and legacy
`iMessage;+;chat` formats)
- validateChatId: accept `service;+;guid` groups and `service;-;address` DMs
- extractRecipientFromChatId: handle 3-part `any;-;+phone` DM format
- normalizeChatId: extract core identifier from any format (was dead code,
now consistent)
- buildGroupChatGuid: new utility to reconstruct group guid using discovered
prefix
- database.discoverGroupChatPrefix(): queries chat.db at init to discover the
local guid prefix format (backward compatible across macOS versions)
- database.getMessages: use chat.guid for group chatIds (consistent with
listChats)
- sender: normalize groupId via discovered prefix before AppleScript execution
- sdk: call discoverGroupChatPrefix() at init, pass to sender
- watcher: add onCheckpoint callback for external cursor persistence
Verified on macOS Tahoe 26.3.1:
chat.db groups: guid = "any;+;534ce85d..." chat_identifier = "534ce85d..."
chat.db DMs: guid = "any;-;+1310..." chat_identifier = "+1310..."
osascript `chat id "any;+;534ce85d..."` works, `iMessage;+;chat...` fails
No breaking API changes. All fixes extend existing format recognition.
Made-with: Cursor
Mason Herbel (Artitus)
requested a review
from LingJueYa～ (LingJueYa)
as a code owner
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
📝 WalkthroughWalkthroughAdds discovery and normalization of Apple group-chat GUID prefixes, applies the prefix when sending group messages, extends chat ID parsing/validation utilities for semicolon and legacy formats, and exposes watcher startup lookback and checkpoint callbacks; integrates discovery into SDK startup and updates tests. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/core/database.ts (1)
518-532: Consider extracting shared chatId resolution logic.The chatId resolution logic here duplicates lines 359-372 in
listChats. Extracting to a shared helper would improve maintainability.♻️ Optional: Extract shared helper
// Add as private method private resolveChatId( isGroup: boolean, chatGuid: string, chatIdentifier: string, chatService: string ): string { if (isGroup || !chatIdentifier) { return chatGuid || chatIdentifier } if (chatIdentifier.includes(';')) { return chatIdentifier } if (chatService) { return `${chatService};${chatIdentifier}` } return `iMessage;${chatIdentifier}` }Then use in both
rowToMessageandlistChats.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/database.ts` around lines 518 - 532, The chatId resolution logic duplicated between rowToMessage and listChats should be extracted into a single helper to avoid divergence; add a private helper (e.g., resolveChatId) that accepts isGroup (or bool), chat_guid, chat_id, and chat_service and implements the existing branching (group or missing identifier -> guid||identifier, semicolon-contained identifier, service-prefixed, default iMessage-prefixed) and then replace the inline logic in rowToMessage and listChats with calls to that helper.src/utils/common.ts (1)
142-149: Consider edge case: discoveredPrefix already contains GUID portion.The function assumes
discoveredPrefixends at the GUID boundary (e.g.,any;+;oriMessage;+;chat). IfdiscoverGroupChatPrefix()returns an unexpected format, this could produce malformed GUIDs.Consider adding a defensive check or documenting the expected prefix format contract.
💡 Optional: Add assertion or documentation
export function buildGroupChatGuid(rawChatId: string, discoveredPrefix: string): string { + // discoveredPrefix should end with ';' or 'chat' (e.g., "any;+;" or "iMessage;+;chat") let guid = rawChatId if (guid.includes(';')) { const parts = guid.split(';') guid = parts[parts.length - 1] ?? guid } if (guid.startsWith('chat')) guid = guid.substring(4) return `${discoveredPrefix}${guid}` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/common.ts` around lines 142 - 149, buildGroupChatGuid may duplicate the GUID portion when discoveredPrefix already contains parts of the GUID; update buildGroupChatGuid to defensively strip any trailing GUID fragments from discoveredPrefix (e.g., trim trailing semicolons or an embedded "chat" segment) or detect if discoveredPrefix already ends with the GUID portion and simply return discoveredPrefix+rawGuidSuffix without re-adding GUID characters; reference buildGroupChatGuid and the helper that produces prefixes (discoverGroupChatPrefix) and implement a check that: 1) normalizes discoveredPrefix by removing trailing delimiters like ';' or "chat", 2) extracts only the GUID suffix from rawChatId (as currently done with split and substring), and 3) concatenates only when there is no overlap to avoid duplicated GUID segments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/core/database.ts`:
- Around line 518-532: The chatId resolution logic duplicated between
rowToMessage and listChats should be extracted into a single helper to avoid
divergence; add a private helper (e.g., resolveChatId) that accepts isGroup (or
bool), chat_guid, chat_id, and chat_service and implements the existing
branching (group or missing identifier -> guid||identifier, semicolon-contained
identifier, service-prefixed, default iMessage-prefixed) and then replace the
inline logic in rowToMessage and listChats with calls to that helper.
In `@src/utils/common.ts`:
- Around line 142-149: buildGroupChatGuid may duplicate the GUID portion when
discoveredPrefix already contains parts of the GUID; update buildGroupChatGuid
to defensively strip any trailing GUID fragments from discoveredPrefix (e.g.,
trim trailing semicolons or an embedded "chat" segment) or detect if
discoveredPrefix already ends with the GUID portion and simply return
discoveredPrefix+rawGuidSuffix without re-adding GUID characters; reference
buildGroupChatGuid and the helper that produces prefixes
(discoverGroupChatPrefix) and implement a check that: 1) normalizes
discoveredPrefix by removing trailing delimiters like ';' or "chat", 2) extracts
only the GUID suffix from rawChatId (as currently done with split and
substring), and 3) concatenates only when there is no overlap to avoid
duplicated GUID segments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e010aef1-de36-47c2-a1c8-8a2e82d82cfd
📒 Files selected for processing (5)
src/core/database.tssrc/core/sdk.tssrc/core/sender.tssrc/core/watcher.tssrc/utils/common.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🧰 Additional context used
🧬 Code graph analysis (1)
src/core/sender.ts (2)
src/utils/common.ts (2)
buildGroupChatGuid(142-150)delay(9-11)src/utils/applescript.ts (3)
generateSendWithAttachmentToChat(447-473)generateSendAttachmentToChat(384-405)generateSendTextToChat(206-216)
🔇 Additional comments (14)
src/core/watcher.ts (3)
25-29: LGTM! Well-documented checkpoint API.The
onCheckpointcallback andinitialLookbackMsconfiguration provide a clean mechanism for external checkpoint persistence, enabling reliable restart without missing messages.
57-62: LGTM! Constructor correctly initializes lookback.The default of 10 seconds is reasonable for catching messages sent just before the watcher starts, and the parameter allows customization for restart scenarios.
125-125: LGTM! Checkpoint emitted at the right time.The checkpoint is emitted after
lastCheckTimeis updated but before message processing errors could occur, ensuring the checkpoint reflects successfully fetched messages.src/core/sender.ts (3)
76-77: LGTM! Sensible default prefix.The default
'any;+;'aligns with modern macOS (Sequoia 15+/Tahoe 26) format, ensuring the sender works out-of-the-box even before async discovery completes.
87-92: LGTM! Clean setter for runtime configuration.The setter enables the SDK to inject the discovered prefix without breaking encapsulation.
371-401: LGTM! Consistent normalization across all group send paths.All three branches (text+attachments, text-only, attachments-only) correctly use
normalizedIdfor AppleScript generation and logging.src/utils/common.ts (4)
34-47: LGTM! Simplified normalization logic.Extracting the last part after
;correctly handles all current formats (group, DM, legacy) and is forward-compatible.
56-61: LGTM! Comprehensive group chat detection.The heuristics correctly identify modern (
any;+;), legacy (iMessage;+;chat), and bare GUID formats.
69-79: LGTM! Clean handling of DM formats.Correctly distinguishes 3-part modern DMs (
service;-;address) from 2-part legacy DMs (service;address), with proper null safety.
91-134: LGTM! Thorough validation with clear error messages.The validation covers all documented formats and provides actionable error messages for invalid inputs.
src/core/database.ts (2)
159-160: LGTM! Added necessary columns for chatId resolution.Including
chat.guidandchat.service_nameenables consistent chatId construction across messages and chat listings.
623-650: LGTM! Code is robustly designed with solid fallbacks, though the style = 43 assumption is unverified.The implementation correctly handles prefix discovery with defensive fallbacks that default to
'any;+;'on any failure. However, the assumption thatstyle = 43reliably identifies group chats cannot be verified from the codebase—no tests cover this function, and no schema documentation explains the chat style field. Notably, elsewhere in the codebase (inlistChats), group chats are identified via participant count (COUNT(*) FROM chat_handle_join > 1), suggesting a different classification approach. The code's defensive design mitigates this uncertainty, but validatingstyle = 43against actual Messages databases or macOS documentation would strengthen confidence in the discovery mechanism.src/core/sdk.ts (2)
115-128: LGTM! Non-blocking initialization with graceful fallback.The async discovery pattern ensures SDK initialization isn't blocked by database access. The type guards ensure compatibility with custom sender implementations.
Note: There's a potential race where
sendToGroupcould be called before discovery completes, but the default'any;+;'prefix handles this correctly for modern macOS.
532-534: LGTM! Correctly wires initialLookbackMs to watcher.The optional chaining safely handles undefined
eventsor missinginitialLookbackMs.
There was a problem hiding this comment.
Pull request overview
Updates chat ID handling to support modern macOS Messages chat.guid formats (notably any;+;{guid} for groups) and adds a watcher checkpoint callback to support reliable restarts without missing messages.
Changes:
- Extend chatId parsing/validation utilities and add
buildGroupChatGuid()to normalize group GUIDs across macOS versions. - Add database-driven discovery of the local group-chat GUID prefix and wire it into the sender/SDK.
- Add
WatcherEvents.onCheckpointandinitialLookbackMswiring to enable external checkpoint persistence.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/common.ts | Expands chatId normalization/validation and adds group GUID builder. |
| src/core/database.ts | Makes getMessages group chatIds consistent with listChats; adds prefix discovery helper. |
| src/core/sender.ts | Normalizes group send IDs via discovered prefix; adds setter for prefix. |
| src/core/sdk.ts | Kicks off prefix discovery at construction time and passes watcher lookback through. |
| src/core/watcher.ts | Adds checkpoint callback and configurable initial lookback window. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const row = this.db.prepare('SELECT guid, chat_identifier FROM chat WHERE style = 43 LIMIT 1').get() as | ||
| | { guid: string; chat_identifier: string } | ||
| | undefined | ||
| if (!row) return 'any;+;' | ||
| const guid = str(row.guid) | ||
| const identifier = str(row.chat_identifier) | ||
| // guid is e.g. "any;+;534ce85d..." and identifier is "534ce85d..." | ||
| const idx = guid.indexOf(identifier) | ||
| if (idx > 0) return guid.substring(0, idx) | ||
| // identifier might have extra prefix (e.g., "chat534ce85d...") | ||
| if (identifier.startsWith('chat') && guid.includes(identifier.substring(4))) { | ||
| const rawGuid = identifier.substring(4) | ||
| const rawIdx = guid.indexOf(rawGuid) | ||
| if (rawIdx > 0) return guid.substring(0, rawIdx) | ||
| } |
There was a problem hiding this comment.
discoverGroupChatPrefix() can return the wrong prefix on legacy macOS if chat.chat_identifier already includes the chat prefix (e.g. identifier = chat<guid> and guid = iMessage;+;chat<guid>). In that case guid.indexOf(identifier) is > 0 and the function returns iMessage;+; (missing the required chat), which will later produce AppleScript IDs like iMessage;+;<guid> and break group sends. Consider deriving the prefix by searching for the raw guid (identifier with an optional leading chat stripped) and returning everything before that position (so legacy returns iMessage;+;chat, modern returns any;+;).
| const row = this.db.prepare('SELECT guid, chat_identifier FROM chat WHERE style = 43 LIMIT 1').get() as | |
| | { guid: string; chat_identifier: string } | |
| | undefined | |
| if (!row) return 'any;+;' | |
| const guid = str(row.guid) | |
| const identifier = str(row.chat_identifier) | |
| // guid is e.g. "any;+;534ce85d..." and identifier is "534ce85d..." | |
| const idx = guid.indexOf(identifier) | |
| if (idx > 0) return guid.substring(0, idx) | |
| // identifier might have extra prefix (e.g., "chat534ce85d...") | |
| if (identifier.startsWith('chat') && guid.includes(identifier.substring(4))) { | |
| const rawGuid = identifier.substring(4) | |
| const rawIdx = guid.indexOf(rawGuid) | |
| if (rawIdx > 0) return guid.substring(0, rawIdx) | |
| } | |
| const row = this.db | |
| .prepare('SELECT guid, chat_identifier FROM chat WHERE style = 43 LIMIT 1') | |
| .get() as { guid: string; chat_identifier: string } | undefined | |
| if (!row) return 'any;+;' | |
| const guid = str(row.guid) | |
| const identifier = str(row.chat_identifier) | |
| // On modern macOS: | |
| // guid = "any;+;534ce85d..." | |
| // identifier = "534ce85d..." | |
| // On legacy macOS: | |
| // guid = "iMessage;+;chat534ce85d..." | |
| // identifier = "chat534ce85d..." or "534ce85d..." | |
| // We derive the prefix by locating the raw GUID (identifier without optional "chat"). | |
| let rawGuid = identifier | |
| if (rawGuid.startsWith('chat')) rawGuid = rawGuid.substring(4) | |
| const idx = guid.indexOf(rawGuid) | |
| if (idx > 0) return guid.substring(0, idx) |
| // Discover the local group chat guid prefix (async, non-blocking) | ||
| this.database | ||
| .discoverGroupChatPrefix() | ||
| .then((prefix) => { | ||
| if ('setGroupChatPrefix' in this.sender && typeof this.sender.setGroupChatPrefix === 'function') { | ||
| this.sender.setGroupChatPrefix(prefix) | ||
| } | ||
| if (this.config.debug) { | ||
| console.log(`[SDK] Discovered group chat prefix: "${prefix}"`) | ||
| } | ||
| }) | ||
| .catch(() => { | ||
| // Non-fatal: falls back to default "any;+;" | ||
| }) |
There was a problem hiding this comment.
Group-chat prefix discovery is kicked off in the constructor but not awaited. Since MessageSender defaults groupChatPrefix to any;+; and sendToGroupChat() always rewrites the provided groupId using that prefix, there’s a race on older macOS where group sends can be rewritten to the wrong format until discovery completes (or if discovery fails silently). To avoid intermittent failures, consider awaiting discovery as part of a dedicated async init path, or make sendToGroupChat() avoid rewriting when the caller already supplied a full service;+;... guid (or retry with the original ID on -1728).
| if (chatId.includes(';')) { | ||
| const parts = chatId.split(';') | ||
| // AppleScript group format: iMessage;+;chat... | ||
| if (parts.length >= 3 && parts[0] === 'iMessage' && parts[1] === '+' && parts[2]?.startsWith('chat')) { | ||
| // Validate GUID part length | ||
|
|
||
| // Group format: service;+;guid (e.g., any;+;534ce85d...) | ||
| if (parts.length >= 3 && parts[1] === '+') { | ||
| const guidPart = parts.slice(2).join(';') | ||
| if (guidPart.length < 8) { | ||
| throw new Error('Invalid chatId format: GUID too short') | ||
| } | ||
| return | ||
| } | ||
|
|
||
| // DM format: <service>;<address> | ||
| const service = parts[0] || '' | ||
| const address = parts[1] || '' | ||
| const allowedServices = new Set(['iMessage', 'SMS', 'RCS']) | ||
| if (!allowedServices.has(service) || !address) { | ||
| throw new Error('Invalid chatId format: expected "<service>;<address>" or group GUID') | ||
| // DM format: service;-;address (e.g., any;-;+1234567890) | ||
| if (parts.length === 3 && parts[1] === '-') { | ||
| if (!parts[2]) { | ||
| throw new Error('Invalid chatId format: missing address') | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
validateChatId() currently accepts semicolon patterns with an empty service segment (e.g. ;+;12345678 or ;-;+1234567890) because the service;+;guid and service;-;address branches don’t validate parts[0]. This can allow clearly invalid chatIds through validation and then fail later at send time. Suggest validating that the service part is non-empty (and optionally in the same allowed-service set used for the 2-part legacy DM case).
| /** | ||
| * Normalize chatId format | ||
| * - Extracts GUID from AppleScript group format (e.g., `iMessage;+;chat...` -> `chat...`) | ||
| * - Returns normalized chatId for consistent handling | ||
| * @param chatId Chat identifier (may be in various formats) | ||
| * @returns Normalized chatId | ||
| * Normalize chatId format by extracting the core identifier. | ||
| * - Group: `any;+;534ce85d...` -> `534ce85d...` | ||
| * - Legacy group: `iMessage;+;chat534ce85d...` -> `chat534ce85d...` | ||
| * - DM: `any;-;+1234567890` -> `+1234567890` | ||
| * - Already bare: returned as-is | ||
| */ | ||
| export function normalizeChatId(chatId: string): string { | ||
| // AppleScript group format: iMessage;+;chat... | ||
| // Extract GUID part (chat...) for normalization | ||
| if (chatId.includes(';')) { | ||
| const parts = chatId.split(';') | ||
| // Check if it matches AppleScript group format: iMessage;+;chat... | ||
| if (parts.length >= 3 && parts[0] === 'iMessage' && parts[1] === '+' && parts[2]?.startsWith('chat')) { | ||
| // Extract GUID part (everything after the second semicolon) | ||
| return parts.slice(2).join(';') | ||
| } | ||
| return parts[parts.length - 1] ?? chatId | ||
| } |
There was a problem hiding this comment.
normalizeChatId() now normalizes any semicolon-form chatId by returning only the last segment. This changes behavior for legacy DM inputs like iMessage;+1234567890 (it will now return +1234567890), which will break existing unit tests and may be a breaking change for callers relying on the previous “unchanged for other formats” behavior. Either update tests/callers and document the new semantics clearly, or narrow the normalization to only the formats you intend to normalize.
| /** Called after each successful poll with the current checkpoint time. Persist this externally | ||
| * and pass it back as `initialLookbackMs` on restart to avoid missing messages. */ |
There was a problem hiding this comment.
The onCheckpoint doc suggests persisting a Date and passing it back as initialLookbackMs, but initialLookbackMs is a duration (ms) rather than an absolute timestamp. This is likely to confuse consumers and lead to incorrect restarts. Consider updating the comment to explain that callers should persist lastCheckTime and compute Date.now() - lastCheckTime.getTime() (optionally clamped) to derive initialLookbackMs on restart, or change the API to accept an absolute checkpoint time instead.
| /** Called after each successful poll with the current checkpoint time. Persist this externally | |
| * and pass it back as `initialLookbackMs` on restart to avoid missing messages. */ | |
| /** | |
| * Called after each successful poll with the current checkpoint time. | |
| * Persist this `lastCheckTime` externally and, on restart, compute a lookback | |
| * duration such as: | |
| * | |
| * const initialLookbackMs = Math.max(0, Date.now() - lastCheckTime.getTime()) | |
| * | |
| * (optionally clamped to some maximum), and pass that duration as | |
| * `initialLookbackMs` to the watcher constructor. This helps avoid missing | |
| * messages across restarts. | |
| */ |
…x test - Extract duplicated chatId resolution logic from rowToMessage and listChats into a single private resolveChatId() helper to prevent divergence - Add defensive overlap guard in buildGroupChatGuid to avoid duplicating GUID segments if discoveredPrefix already contains part of the GUID - Update normalizeChatId test to match new behavior (extracts core identifier from all semicolon formats, not just iMessage;+;chat) Made-with: Cursor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
__tests__/02-utils.test.ts (1)
164-186: Tests fornormalizeChatIdlook good.The updated tests properly cover modern macOS formats (
any;+;,any;-;) alongside legacy formats.Consider adding tests for the new
buildGroupChatGuidfunction to verify its GUID construction and deduplication logic.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/02-utils.test.ts` around lines 164 - 186, Add unit tests for buildGroupChatGuid to verify it constructs GUIDs correctly and deduplicates participant lists: write tests that (1) call buildGroupChatGuid with a given group name and participants in different orders and assert the returned GUID is identical (deduplication/order-independence), (2) call it with varying casings and whitespace on participant identifiers to ensure normalization is applied, and (3) assert the GUID format/length is stable for the same inputs; reference the buildGroupChatGuid function from src/utils/common in the new tests alongside the existing normalizeChatId tests.src/utils/common.ts (1)
57-59: Redundant condition on line 58.The check on line 58 (
chatId.startsWith('iMessage;+;chat')) is unreachable because any string starting withiMessage;+;chatwill contain;+;and already returntrueon line 57.♻️ Suggested simplification
export function isGroupChatId(chatId: string): boolean { if (chatId.includes(';+;')) return true - if (chatId.startsWith('iMessage;+;chat')) return true if (!chatId.includes(';') && chatId.startsWith('chat') && chatId.length > 10) return true return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/common.ts` around lines 57 - 59, The second conditional checking chatId.startsWith('iMessage;+;chat') is redundant because any string matching that prefix will already match the earlier chatId.includes(';+;') check; remove the redundant if-statement (the line containing chatId.startsWith('iMessage;+;chat')) from the function in common.ts so the remaining checks (chatId.includes(';+;') and the final chat/... length check) preserve the original behavior.src/core/database.ts (1)
617-644: Implementation is correct with safe fallbacks.The discovery logic properly handles common patterns and gracefully falls back to
any;+;for edge cases or when no group chats exist. Thestyle = 43condition correctly identifies group chats in the Messages database.Consider extracting the
style = 43magic number to a named constant (e.g.,const CHAT_STYLE_GROUP = 43) for improved maintainability and readability, since this macOS Messages database convention may not be immediately obvious to future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/database.ts` around lines 617 - 644, Extract the magic number 43 into a named constant (e.g., CHAT_STYLE_GROUP = 43) and use that constant in discoverGroupChatPrefix() instead of the literal `style = 43`; update the SQL prepare call ('SELECT guid, chat_identifier FROM chat WHERE style = 43 LIMIT 1') to reference the constant and add a short comment above the constant explaining it denotes group chats in the macOS Messages DB to improve readability and maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@__tests__/02-utils.test.ts`:
- Around line 164-186: Add unit tests for buildGroupChatGuid to verify it
constructs GUIDs correctly and deduplicates participant lists: write tests that
(1) call buildGroupChatGuid with a given group name and participants in
different orders and assert the returned GUID is identical
(deduplication/order-independence), (2) call it with varying casings and
whitespace on participant identifiers to ensure normalization is applied, and
(3) assert the GUID format/length is stable for the same inputs; reference the
buildGroupChatGuid function from src/utils/common in the new tests alongside the
existing normalizeChatId tests.
In `@src/core/database.ts`:
- Around line 617-644: Extract the magic number 43 into a named constant (e.g.,
CHAT_STYLE_GROUP = 43) and use that constant in discoverGroupChatPrefix()
instead of the literal `style = 43`; update the SQL prepare call ('SELECT guid,
chat_identifier FROM chat WHERE style = 43 LIMIT 1') to reference the constant
and add a short comment above the constant explaining it denotes group chats in
the macOS Messages DB to improve readability and maintainability.
In `@src/utils/common.ts`:
- Around line 57-59: The second conditional checking
chatId.startsWith('iMessage;+;chat') is redundant because any string matching
that prefix will already match the earlier chatId.includes(';+;') check; remove
the redundant if-statement (the line containing
chatId.startsWith('iMessage;+;chat')) from the function in common.ts so the
remaining checks (chatId.includes(';+;') and the final chat/... length check)
preserve the original behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac3e4b3a-1eb2-415e-b8ad-ba3afbe63da8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.codegraph.agent.txt.codegraph.json__tests__/02-utils.test.tssrc/core/database.tssrc/utils/common.ts
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
__tests__/02-utils.test.ts (1)
src/utils/common.ts (1)
normalizeChatId(41-47)
🪛 LanguageTool
.codegraph.agent.txt
[uncategorized] ~18-~18: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...oducer: 13 function: 181 class: 15 HIGH IMPACT FUNCTIONS (most dependents): loggerPl...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (9)
.codegraph.agent.txt (1)
1-57: LGTM!This is a static analysis artifact documenting codebase metrics and high-impact functions. No functional code changes to review.
src/utils/common.ts (4)
34-47: LGTM!The normalization logic correctly extracts the core identifier from various semicolon-delimited formats and safely handles edge cases with the nullish coalescing fallback.
63-79: LGTM!The extraction logic correctly handles both modern (
service;-;address) and legacy (service;address) DM formats while properly filtering out group chats.
91-134: LGTM!The validation logic comprehensively covers all accepted chatId formats with clear error messages. The use of
Setfor service validation is appropriate.
143-160: LGTM!The function correctly handles various input formats and includes defensive guards against GUID duplication. The early-return optimizations are appropriate.
src/core/database.ts (4)
100-112: LGTM!The
resolveChatIdhelper consolidates chatId resolution logic with a clear fallback chain: groups usechatGuid, DMs prefer existing semicolon-delimited identifiers, and bare identifiers get service-prefixed.
173-174: LGTM!The additional columns (
chat_guid,chat_service) are correctly included to support the newresolveChatIdlogic.
369-374: LGTM!The
listChatsmethod correctly usesresolveChatIdwith appropriate null handling for optional columns.
520-535: LGTM!The
rowToMessagemethod correctly appliesresolveChatIdusing the aliased column names from the query.
Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function buildGroupChatGuid(rawChatId: string, discoveredPrefix: string): string { | ||
| // Extract the bare GUID (no service prefix, no "chat" prefix) | ||
| let guid = rawChatId | ||
| if (guid.includes(';')) { | ||
| const parts = guid.split(';') | ||
| guid = parts[parts.length - 1] ?? guid | ||
| } | ||
| if (guid.startsWith('chat')) guid = guid.substring(4) | ||
|
|
||
| // If the result already matches what we'd produce, return rawChatId as-is | ||
| const expected = `${discoveredPrefix}${guid}` | ||
| if (rawChatId === expected) return rawChatId | ||
|
|
||
| // Guard against a malformed prefix that already contains the GUID | ||
| if (discoveredPrefix.endsWith(guid)) return discoveredPrefix | ||
|
|
||
| return expected | ||
| } |
There was a problem hiding this comment.
buildGroupChatGuid is new exported behavior but there are no unit tests covering its normalization rules (e.g., stripping any;+; / iMessage;+;chat prefixes, handling chat{guid} inputs, and avoiding double-prefixing). Adding focused tests will help prevent regressions, especially across macOS versions where the prefix differs.
| const idx = guid.indexOf(identifier) | ||
| if (idx > 0) return guid.substring(0, idx) | ||
| // identifier might have extra prefix (e.g., "chat534ce85d...") | ||
| if (identifier.startsWith('chat') && guid.includes(identifier.substring(4))) { | ||
| const rawGuid = identifier.substring(4) | ||
| const rawIdx = guid.indexOf(rawGuid) | ||
| if (rawIdx > 0) return guid.substring(0, rawIdx) | ||
| } |
There was a problem hiding this comment.
discoverGroupChatPrefix() returns guid.substring(0, idx) as soon as chat_identifier is found inside guid. If chat_identifier itself includes the legacy chat prefix (e.g., identifier=chat493..., guid=iMessage;+;chat493...), this will return "iMessage;+;" (missing the required "chat" segment), and downstream buildGroupChatGuid() will produce iMessage;+;493... which won't match Messages.app chat ids. The later identifier.startsWith('chat') fallback is currently unreachable in this case because the early idx > 0 return happens first. Consider handling the identifier.startsWith('chat') case before the idx-based return (or compute the prefix using the raw GUID without chat first).
| const idx = guid.indexOf(identifier) | |
| if (idx > 0) return guid.substring(0, idx) | |
| // identifier might have extra prefix (e.g., "chat534ce85d...") | |
| if (identifier.startsWith('chat') && guid.includes(identifier.substring(4))) { | |
| const rawGuid = identifier.substring(4) | |
| const rawIdx = guid.indexOf(rawGuid) | |
| if (rawIdx > 0) return guid.substring(0, rawIdx) | |
| } | |
| // identifier might have extra prefix (e.g., "chat534ce85d...") | |
| if (identifier.startsWith('chat') && guid.includes(identifier.substring(4))) { | |
| const rawGuid = identifier.substring(4) | |
| const rawIdx = guid.indexOf(rawGuid) | |
| if (rawIdx > 0) return guid.substring(0, rawIdx) | |
| } | |
| // Fall back to direct substring match | |
| const idx = guid.indexOf(identifier) | |
| if (idx > 0) return guid.substring(0, idx) |
| const normalizedId = buildGroupChatGuid(groupId, this.groupChatPrefix) | ||
|
|
||
| if (hasText && resolvedPaths.length > 0) { | ||
| // Strategy 1: Text + Attachments | ||
| const firstAttachment = resolvedPaths[0]! | ||
| const { script } = generateSendWithAttachmentToChat(groupId, text!, firstAttachment) | ||
| await this.executeWithRetry(script, `Send text and attachment to group ${groupId}`) | ||
| const { script } = generateSendWithAttachmentToChat(normalizedId, text!, firstAttachment) | ||
| await this.executeWithRetry(script, `Send text and attachment to group ${normalizedId}`) |
There was a problem hiding this comment.
sendToGroupChat() now normalizes groupId to normalizedId for AppleScript, but sendToGroup() still constructs MessagePromise instances using the original groupId. On modern macOS where the database chatId for groups is any;+;{guid}, a caller-provided groupId like chat{guid} will not match the database value in MessagePromise.matchesChatId() (it only strips semicolon prefixes, not the chat prefix). Consider using the same normalized id for both AppleScript routing and outgoing-message tracking to keep confirmation reliable.
Summary
On modern macOS (Sequoia 15+ / Tahoe 26), Messages.app uses
any;+;{guid}for group chat IDs andany;-;{phone}for DMs in thechat.guidcolumn. The existing code hardcodesiMessage;+;chat{guid}which does not exist in modernchat.db, causing all group chat sends to fail with AppleScript error -1728.Verified on macOS Tahoe 26.3.1:
Changes
Chat ID format recognition (
src/utils/common.ts)isGroupChatId: now recognizesany;+;{guid}via;+;check (backward compatible withiMessage;+;chatand barechat{guid})validateChatId: acceptsservice;+;guidgroups,service;-;addressDMs, addedanyto allowed servicesextractRecipientFromChatId: handles 3-partany;-;+phoneDM format (was only 2-partiMessage;+phone)normalizeChatId: updated to extract last part after;(was dead code -- exported but never imported)buildGroupChatGuid(new): strips any existing prefix and reconstructs with the discovered local prefixStartup format discovery (
src/core/database.ts)discoverGroupChatPrefix()(new): queries one group chat fromchat.dbat SDK init to discover the local guid prefix format. Defaults toany;+;if no groups exist. This ensures backward compatibility -- on older macOS withiMessage;+;chatformat, it discovers that prefix instead.Consistent chatId between getMessages and listChats (
src/core/database.ts)getMessagesnow includeschat.guidandchat.service_namein the queryrowToMessageuseschat.guidfor group chats (same logic aslistChats), fixing the inconsistency wheremsg.chatIdandchatInfo.chatIddiffered for the same groupSend normalization (
src/core/sender.ts)sendToGroupChatnormalizesgroupIdviabuildGroupChatGuid(groupId, discoveredPrefix)before passing to AppleScript generatorssetGroupChatPrefix()setter, called by SDK after discoverySDK wiring (
src/core/sdk.ts)database.discoverGroupChatPrefix()async at initsetGroupChatPrefix()Watcher checkpoint (
src/core/watcher.ts)onCheckpointcallback toWatcherEvents, called after each successful poll withlastCheckTimeinitialLookbackMson restart to avoid missing messagesBreaking Changes
None. All changes are backward-compatible extensions:
isGroupChatIdstill recognizes old formatsvalidateChatIdstill accepts old formatsdiscoverGroupChatPrefixfalls back toany;+;(modern default)onCheckpointis optionalbuildGroupChatGuidis additiveTest plan
npx tsc --noEmitpassesnpx biome checkpassesnpm run buildsucceedsMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Tests