feat!: v3.0.0-rc.3 — scope reduction, docs audit, sender refactor - #50
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBroad refactor that narrows the public SDK surface (object-form Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant SDK as SDK
participant PluginMgr as PluginManager
participant Sender as MessageSender
participant Probe as MessagesAppProbe
participant OS as osascript
Client->>SDK: sdk.send({ to, text?, attachments? })
SDK->>PluginMgr: callInterruptingHook(onBeforeSend)
alt hook throws
PluginMgr-->>SDK: throws IMessageError
SDK-->>Client: reject
else hook ok
PluginMgr-->>SDK: ok
SDK->>Sender: sender.send(request)
Sender->>Probe: isRunning()
Probe-->>Sender: boolean
Sender->>Sender: inspectAttachment(s)
Sender->>OS: execAppleScript(script, signal)
OS-->>Sender: success / mapped stderr -> error
Sender-->>SDK: resolve / throw SendError
SDK-->>Client: resolve / reject
end
sequenceDiagram
participant DB as MessagesDB
participant Watch as MessageWatchSource
participant Dispatcher as MessageDispatcher
participant PluginMgr as PluginManager
participant Sink as PluginMessageSink
DB->>Watch: getMessagesSinceRowId()
Watch->>Dispatcher: dispatch(batch)
Dispatcher->>Dispatcher: partition incoming / from-me
Dispatcher->>PluginMgr: callHook(onIncomingMessage) / callHook(onFromMe)
PluginMgr->>Sink: invoke plugin hooks (parallel/sequential per hook)
Sink-->>PluginMgr: errors routed to onError (collected)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR prepares v3.0.0-rc.3 by narrowing the SDK to core primitives (send/query/watch/plugins), removing app-level features (scheduling/reminders/chaining/tracking/URL-download), refactoring the send pipeline, and auditing docs/types against source.
Changes:
- Remove scheduling/reminders/chain/tracker/URL attachment download and related tests/examples.
- Refactor outbound send: split AppleScript script construction, add sandbox-safe attachment staging + cleanup, adjust concurrency/cancellation semantics.
- Update domain/types/query/mapping/watcher/plugin system to reflect new contracts and macOS 26 (“Tahoe”) behaviors.
Reviewed changes
Copilot reviewed 95 out of 96 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/async.ts | Retry/semaphore cancellation & FIFO semantics tweaks |
| src/types/send.ts | Simplify public send request DTO |
| src/types/query.ts | Clarify query filter semantics/docs |
| src/types/plugin.ts | Update plugin hook surface + docs |
| src/types/config.ts | Add sendTimeout, bump defaults |
| src/sdk-bounds.ts | Add bounds for sendTimeout, update defaults |
| src/infra/plugin/manager.ts | Remove old plugin manager module |
| src/infra/platform.ts | Document Darwin→service-prefix threshold |
| src/infra/outgoing/tracker.ts | Remove outgoing DB-confirmation tracker |
| src/infra/outgoing/temp-files.ts | Rewrite temp cleanup around temp dirs/constants |
| src/infra/outgoing/downloader.ts | Remove URL downloading/conversion |
| src/infra/outgoing/applescript-builder.ts | New pure AppleScript builder + attachment precheck |
| src/infra/db/watcher.ts | Make stop() await consumer loop; tighten watcher error handling |
| src/infra/db/sqlite-adapter.ts | Tighten query param typing; document readOnly rationale |
| src/infra/db/mapper.ts | Fix chatId fallbacks; Tahoe retract detection; new mapped fields |
| src/infra/db/macos26.ts | Update query builder filters/paging/backfill helpers |
| src/infra/db/contract.ts | Expand DB query contract (maxRowId + chat backfill) |
| src/infra/db/body-decoder.ts | Decode attributedBody via single-root unarchive |
| src/infra/attachments.ts | Reduce attachment helpers to minimal set |
| src/index.ts | Update public exports to new API surface |
| src/domain/validate.ts | Minimal send validation; URL classification only |
| src/domain/timestamp.ts | Document precision limits for ns conversions |
| src/domain/service.ts | Remove 'unknown'; resolve to `Service |
| src/domain/routing.ts | Route DM vs group via ChatId; remove recipient heuristics |
| src/domain/reaction.ts | Tighten visibility of internal reaction maps/meta |
| src/domain/messages-app.ts | New protocol constants (TCC safe dirs, temp prefix/dir) |
| src/domain/message.ts | Make chatId nullable; add hasAttachments; adjust schedule kind mapping |
| src/domain/chat.ts | Minor docs/formatting updates |
| src/domain/chat-id.ts | Trim user input; stricter validation errors; group GUID pattern |
| src/domain/attachment.ts | Rename fields (isFromMe), add unknown transfer status docs |
| src/domain/DOMAIN.md | Remove stale domain layer spec doc |
| src/config.ts | Remove compat facade (BOUNDS/LIMITS) |
| src/application/send-port.ts | Update SendPort to Promise<void> and clarify semantics |
| src/application/reminders.ts | Remove reminders facade |
| src/application/reminder-time.ts | Remove natural language time parsing |
| src/application/message-dispatcher.ts | Partition incoming vs from-me dispatch; remove tracker integration |
| src/application/message-chain.ts | Remove fluent chain API |
| package.json | Bump version to 3.0.0-rc.3 |
| examples/logger-plugin.ts | Move logger to example; update hooks |
| examples/15-smart-reminders.ts | Remove reminders example |
| examples/14-scheduled-messages.ts | Remove scheduler example |
| examples/13-watch-own-messages.ts | Remove old watcher example |
| examples/11-plugin.ts | Remove outdated plugin example |
| examples/11-error-handling.ts | Update send API usage |
| examples/10-plugin.ts | New plugin example using onFromMe + watcher |
| examples/10-get-sent-message.ts | Remove old send-tracker example |
| examples/09-get-sent-message.ts | New “correlate send with DB row” via watcher example |
| examples/09-batch-send.ts | Remove batch send example |
| examples/08-auto-reply.ts | Rewrite without MessageChain; handle nullable chatId |
| examples/07-watch-messages.ts | Remove explicit stopWatching usage |
| examples/05-query-messages.ts | Update unread filter API (isRead) |
| examples/04-send-group.ts | Avoid hand-written chatIds; resolve via listChats() |
| examples/03-send-file.ts | Replace convenience APIs with send({ attachments }) |
| examples/02-send-image.ts | Remove URL attachment example; local-only |
| examples/01-send-text.ts | Update send API usage |
| docs/code-review-checklist.md | Remove checklist doc |
| biome.json | Exclude tests-e2e/ and .claude/ from lint scope |
| tests/setup.ts | Remove unused temp dir + AppleScript mocks |
| tests/25-architecture-boundaries.test.ts | Update boundaries for removed config facade |
| tests/24-messages-db-semantic.test.ts | Add Tahoe retract/edit semantic tests |
| tests/23-messages-db-query-selection.test.ts | Expand query builder assertions; update filter API |
| tests/22-watcher-updates.test.ts | New tests pinning UPDATE observation gap |
| tests/22-reminders.test.ts | Remove reminder parsing tests |
| tests/20-incoming-dispatcher.test.ts | Update dispatcher semantics + new from-me branch tests |
| tests/19-schema-contract.test.ts | Add explicit mapReaction/interface alignment check |
| tests/17-imessage-service-routing.test.ts | Remove old transport routing tests |
| tests/16-tempfile-security.test.ts | Rewrite symlink/TOCTOU tests for new temp dir strategy |
| tests/12-attributed-body.test.ts | Replace fabricated tests with real typedstream fixtures |
| tests/11-outgoing-manager.test.ts | Remove tracker tests |
| tests/10-applescript-transport.test.ts | New real-subprocess osascript tests (darwin-only) |
| tests/09-search-and-attachments.test.ts | Update attachment helpers + unread filter API |
| tests/08-listchats.test.ts | Add coverage for sort/paging/service/archive/search filters |
| tests/07-integration.test.ts | Remove broad integration test suite |
| tests/04-plugins.test.ts | Update to new plugin manager module + hook names |
| tests/03-database.test.ts | Update query filters; add rowid-based reader tests |
| tests/02-utils.test.ts | Update retry/delay cancellation expectations; remove recipient validation tests |
| tests/01-errors.test.ts | Expand coverage for factories + normalization helpers |
| CLAUDE.md | Update repo architecture docs and conventions |
| .gitignore | Ignore tests-e2e/ |
Comments suppressed due to low confidence (1)
src/utils/async.ts:85
retry()throwssignal.reasondirectly on abort.AbortSignal.reasoncan be any value (string, DOMException, etc.), so callers may observe non-Error throws/rejections and lose stack/cause information. Consider normalizing the abort reason to anError(e.g., wrap non-Error reasons withnew Error(String(reason))) before throwing so the function consistently rejects withErrorinstances.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/logger-plugin.ts (1)
77-79:⚠️ Potential issue | 🟡 MinorUpdate the plugin description to match its new example-only status.
descriptionstill says "Built-in logger plugin", which now contradicts both the file header and the PR’s scope reduction.💡 Proposed fix
return { name: 'logger', version: '1.0.0', - description: 'Built-in logger plugin', + description: 'Example logger plugin',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/logger-plugin.ts` around lines 77 - 79, The plugin object's description field still reads "Built-in logger plugin" but this plugin is now example-only; update the description string in the plugin declaration (the object with name: 'logger', version: '1.0.0', description: ...) to something like "Example logger plugin (for demonstration only)" or equivalent wording that clearly indicates it's not built-in.
🧹 Nitpick comments (12)
.gitignore (1)
44-44: Movetests-e2e/to the Testing & Coverage section.The new entry is placed under "# Temp files" but belongs in the "# Testing & Coverage" section (lines 24–26) alongside
coverage/andtest-results/for better organization.📁 Proposed reorganization
# Testing & Coverage coverage/ test-results/ +tests-e2e/ # Editor & IDE .vscode/*and remove it from the temp section:
.tmp/ *.tmp -tests-e2e/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 44, The entry "tests-e2e/" is currently under the "# Temp files" section; remove that line from the temp files block and add it under the "# Testing & Coverage" section next to "coverage/" and "test-results/" so the testing-related ignores are grouped together—update the .gitignore by deleting the tests-e2e/ line from the temp section and inserting it into the testing section.__tests__/15-reactions.test.ts (1)
61-67: Test name is misleading.The test name says "flags isRemoved=true for all remove-range codes even if kind is unmapped" but it actually asserts
isRemoved: falsefor code 3008. The test correctly verifies that 3008 is outside the remove range (3000–3007), so no issue with the assertion—just the description.Consider renaming to clarify intent, e.g.,
'returns {null, false} for codes just outside remove range'.📝 Suggested rename
- it('flags isRemoved=true for all remove-range codes even if kind is unmapped', () => { - // 3008–3999 fall in the isRemove branch but map to no known base kind - // (REACTION_KIND_MAP has no entry for 2008+). Current contract: kind=null - // but isRemoved=false because !isAdd && !isRemove short-circuits first. - // 3008 and above: isRemove is FALSE (range is 3000–3007 inclusive). + it('returns {null, false} for codes just outside remove range (3008+)', () => { + // 3008 and above fall outside the 3000–3007 remove range, so isRemove is + // false and the !isAdd && !isRemove short-circuit returns early. expect(resolveReactionMeta(3008)).toEqual({ kind: null, isRemoved: false }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/15-reactions.test.ts` around lines 61 - 67, Rename the misleading test title for the case that asserts resolveReactionMeta(3008) returns { kind: null, isRemoved: false }; update the it(...) description (the test centered on resolveReactionMeta) from "flags isRemoved=true for all remove-range codes even if kind is unmapped" to a clearer name such as "returns {kind: null, isRemoved: false} for codes just outside remove range" so the title matches the assertion and intent.__tests__/03-database.test.ts (1)
257-262: Consider tightening the limit assertion.At Line 261,
toBeLessThanOrEqual(1)is a bit permissive. Since this test seeds chats and requestslimit: 1, asserting exactly one row would catch regressions earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/03-database.test.ts` around lines 257 - 262, The test "respects the limit option" currently asserts chats.length <= 1 which is too permissive; update the assertion to require exactly one result so the test fails on regressions — locate the test case (it 'respects the limit option') where insertTestMessage(...) seeds two chats and listChats({ limit: 1 }) is called, and replace the expect(chats.length).toBeLessThanOrEqual(1) assertion with an exact equality assertion (expect(...).toBe(1)) to ensure exactly one row is returned.src/domain/messages-app.ts (1)
21-40: Encode the temp-write-dir invariant in the type system.Right now "
must be one of" only lives in the comment. If someone changesMESSAGES_APP_TEMP_WRITE_DIRlater, TypeScript will not stop an invalid value.💡 Proposed fix
export const MESSAGES_APP_SANDBOX_SAFE_DIRS = ['Pictures', 'Downloads', 'Documents'] as const +export type MessagesAppSandboxSafeDir = (typeof MESSAGES_APP_SANDBOX_SAFE_DIRS)[number] @@ -export const MESSAGES_APP_TEMP_WRITE_DIR = 'Pictures' +export const MESSAGES_APP_TEMP_WRITE_DIR: MessagesAppSandboxSafeDir = 'Pictures'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/messages-app.ts` around lines 21 - 40, The constant MESSAGES_APP_TEMP_WRITE_DIR is only constrained by a comment; make that invariant part of the type system by deriving a union type from MESSAGES_APP_SANDBOX_SAFE_DIRS (e.g., type SandboxSafeDir = typeof MESSAGES_APP_SANDBOX_SAFE_DIRS[number]) and change the declaration of MESSAGES_APP_TEMP_WRITE_DIR to be explicitly typed as that union (e.g., const MESSAGES_APP_TEMP_WRITE_DIR: SandboxSafeDir = 'Pictures'), so the compiler will reject any value not listed in MESSAGES_APP_SANDBOX_SAFE_DIRS.__tests__/16-tempfile-security.test.ts (1)
28-33: Use the shared temp-path constants instead of re-declaring them here.Hard-coding both the prefix and
'Pictures'means this test can drift fromsrc/domain/messages-app.tsand still pass against the wrong contract.💡 Proposed fix
import { homedir, tmpdir } from 'node:os' import { join } from 'node:path' +import { + MESSAGES_APP_TEMP_FILE_PREFIX, + MESSAGES_APP_TEMP_WRITE_DIR, +} from '../src/domain/messages-app' import { TempFileManager } from '../src/infra/outgoing/temp-files' -const PREFIX = 'imsg_temp_' -const REAL_TEMP_DIR = join(homedir(), 'Pictures') +const PREFIX = MESSAGES_APP_TEMP_FILE_PREFIX +const REAL_TEMP_DIR = join(homedir(), MESSAGES_APP_TEMP_WRITE_DIR)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/16-tempfile-security.test.ts` around lines 28 - 33, Replace the hard-coded PREFIX and REAL_TEMP_DIR in the test with the shared constants exported by the messages app module: stop declaring PREFIX and REAL_TEMP_DIR in __tests__/16-tempfile-security.test.ts and instead import the temp-path/ prefix and pictures-dir constants from src/domain/messages-app.ts (the same module that defines the contract used by TempFileManager). Update references to use those exported constant names rather than the literal 'imsg_temp_' and join(homedir(), 'Pictures') so the test stays in sync with TempFileManager's expected values.__tests__/10-applescript-transport.test.ts (1)
91-107: Console.log restoration in finally block is correct.The test properly restores
console.login thefinallyblock, ensuring cleanup even if the test throws. However, the pattern of directly reassigningconsole.logcould be fragile if the tested code uses a cached reference.💡 Optional: Consider using Bun's mock utilities
If Bun's test framework provides
mock.methodor similar for mocking object methods, it could provide more robust cleanup. The current approach works but relies on the implementation callingconsole.logdirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/10-applescript-transport.test.ts` around lines 91 - 107, The test mutates console.log directly which can be fragile; replace the direct reassignment with a proper spy/mock for console.log (e.g., use jest.spyOn(console, 'log').mockImplementation(...) or Bun's mock.method if available) so the test captures output into the lines array while allowing safe restoration via mockRestore in the finally block; update the test around execAppleScript to create the spy before calling execAppleScript and restore/mockRestore in finally, referencing console.log and execAppleScript to locate the change.__tests__/21-watcher.test.ts (1)
338-345: Clarify the wait condition purpose.Line 340's condition
onBatchResolved === false && source !== nullappears to be a timing heuristic to ensureonBatchhas started before callingstop(). Thesource !== nullpart is always true (source was just assigned), making it effectively a no-op. Consider adding a comment or using a more explicit "entered" signal.💡 Suggested improvement for clarity
+ let onBatchEntered = false let onBatchResolved = false const onBatch = async () => { + onBatchEntered = true await new Promise((r) => setTimeout(r, 40)) onBatchResolved = true } // ... await source.start() - // Wait until the consumer definitely entered onBatch. - await waitFor(() => onBatchResolved === false && source !== null, 30) + // Wait until the consumer has entered onBatch. + await waitFor(() => onBatchEntered, 100)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/21-watcher.test.ts` around lines 338 - 345, The wait condition uses `source !== null` which is redundant; replace the timing heuristic by adding an explicit "entered" signal in the `onBatch` handler (e.g., set a new `onBatchEntered` boolean when the handler begins) and then change the `waitFor` call to wait for `onBatchEntered === true` (or keep the existing `onBatchResolved` semantics and wait for it to become false via a new entered flag) before calling `source.stop()` so the test deterministically waits until `onBatch` has started; update references to `onBatch`/`onBatchResolved` and `waitFor` accordingly and optionally add a short comment explaining the purpose of the entered flag.__tests__/17-sender.test.ts (1)
289-292: Remove unused variable.
originalMockis assigned but only referenced by a no-opvoidstatement. This appears to be debugging leftover.🧹 Proposed cleanup
// Shadow the module mock with a direct transport spy by wrapping execStub. execStub = async () => '' - const originalMock = execCalls.slice() - void originalMock🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/17-sender.test.ts` around lines 289 - 292, Remove the unused debugging variable by deleting the assignment to originalMock and the no-op "void originalMock" line in the test; specifically remove the const originalMock = execCalls.slice() and its subsequent void usage so only execStub = async () => '' remains (no other behavior changes to execCalls or execStub).src/domain/validate.ts (1)
23-26: Consider edge case: case-insensitive URL scheme.URLs with uppercase schemes like
HTTP://orHTTPS://would returnfalse. While rare, RFC 3986 specifies schemes are case-insensitive.🔧 Optional: case-insensitive prefix check
export function isURL(value: string): boolean { - return value.startsWith('http://') || value.startsWith('https://') + const lower = value.slice(0, 8).toLowerCase() + return lower.startsWith('http://') || lower.startsWith('https://') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/validate.ts` around lines 23 - 26, The isURL function currently checks prefixes case-sensitively so schemes like "HTTP://" or "Https://" will fail; update isURL to perform a case-insensitive check (e.g., normalize value to lowercase before testing or use a case-insensitive regex) and still only accept "http://" or "https://" schemes; adjust the implementation inside the exported function isURL to use the chosen case-insensitive check so valid URLs with uppercase scheme letters return true.src/infra/db/watcher.ts (1)
346-348: Consider a more defensive type guard.The helper works but relies on duck-typing. A slightly stricter check would handle edge cases where
error.codeexists but isn't a string.🔧 Optional: stricter type guard
function isMissingFileError(error: unknown): boolean { - return typeof error === 'object' && error !== null && (error as { code?: string }).code === 'ENOENT' + if (typeof error !== 'object' || error === null) return false + const code = (error as { code?: unknown }).code + return code === 'ENOENT' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/infra/db/watcher.ts` around lines 346 - 348, The isMissingFileError helper currently duck-types error.code; make the guard stricter by first ensuring error is an object and not null, then check that (error as { code?: unknown }).code is a string before comparing to 'ENOENT' so non-string codes don't incorrectly match; update the isMissingFileError function to perform a typeof check on the code property prior to equality comparison.src/infra/outgoing/temp-files.ts (1)
112-149: Sync filesystem operations may block on large temp directories.
removeExpiredFiles()uses synchronousreaddirSync,lstatSync, andrmSync. For typical use (few temp files), this is fine. However, if~/Picturesaccumulates many entries or the filesystem is slow (network mount), the event loop could block during cleanup.Since cleanup runs on an unref'd interval and at destroy, this is unlikely to impact most users, but worth noting for high-throughput scenarios.
💡 Alternative: async cleanup (if blocking becomes an issue)
// Could be refactored to use async fs operations if needed: import { readdir, lstat, rm } from 'node:fs/promises' private async removeExpiredFiles(): Promise<void> { // ... async implementation }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/infra/outgoing/temp-files.ts` around lines 112 - 149, The cleanup uses blocking sync fs calls in removeExpiredFiles causing potential event-loop stalls; refactor removeExpiredFiles to an async method using node:fs/promises (readdir, lstat, rm) and await operations (e.g., for/of with await or Promise.all with map) while keeping the same age check against this.config.maxAge and preserving debug/error logging around each removal and the overall sweep; update any callers (the unref'd interval and destroy logic) to await or handle the returned Promise from removeExpiredFiles so errors are still caught and do not change behavior for typical small directories.__tests__/14-security-injection.test.ts (1)
24-28: ImportMESSAGES_APP_SANDBOX_SAFE_DIRSfromsrc/domain/messages-app.tsinstead of hardcoding.The test hardcodes
['Pictures', 'Downloads', 'Documents']in theSAFE_DIRSconstant, but this exact value is already exported asMESSAGES_APP_SANDBOX_SAFE_DIRSfrom the domain module. If the source constant changes, this test'sattachmentForstub will diverge. Importing it maintains a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/14-security-injection.test.ts` around lines 24 - 28, Replace the hardcoded SAFE_DIRS array in the test with the canonical MESSAGES_APP_SANDBOX_SAFE_DIRS export and derive SAFE_DIRS by mapping that constant through join(homedir(), d) (same mapping currently used); update the attachmentFor helper to use this derived SAFE_DIRS (retain the existing startsWith check and needsBypass logic) and add the import for MESSAGES_APP_SANDBOX_SAFE_DIRS from the domain messages-app module so the test follows the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/03-database.test.ts`:
- Around line 265-277: The test is flaky because it asserts messages[0]?.service
which depends on ordering; change the assertion to target the specific inserted
row instead: have insertTestMessage return or expose a stable identifier (e.g.,
id) or query database.getMessages() and find the message by the unique fields
used in the insert (sender '+1234567890' and/or text 'Unknown'), then assert
that the foundMessage.service is null; update the test to locate the inserted
row via that identifier or by filtering the messages array rather than relying
on index 0, using the existing insertTestMessage, mockDb.db and
database.getMessages helpers.
In `@__tests__/08-listchats.test.ts`:
- Around line 108-113: The test currently only checks that every item is older
than the first item by comparing each c.lastMessageAt against firstTs
(variables: recent, first, rest, firstTs), which doesn't verify full ordering;
change the loop to track the previous timestamp (e.g., prevTs = firstTs) and for
each item in rest compute ts from c.lastMessageAt and assert prevTs >= ts, then
set prevTs = ts so each item is compared to its immediate predecessor to
validate the entire descending order.
In `@README.md`:
- Around line 279-289: The correlation map logic in sends and the watcher is
broken because sends is never populated before sdk.send; change the flow so you
create and store a pending resolver/promise in sends (keyed by the outgoing
text) before calling sdk.send, then in sdk.startWatching's onFromMeMessage
handler (the onFromMeMessage callback) look up the pending entry for msg.text
and resolve that pending promise (or replace it with Promise.resolve(msg)) when
the message arrives; ensure you use the same text key and clean up the map entry
after resolving so functions like sends.get(msg.text), the pending promise you
stored, and sdk.send coordinate correctly.
In `@src/domain/timestamp.ts`:
- Around line 21-23: Update the doc comment for toMacTimestampNs/MAC_EPOCH to
remove the misleading “early 1970s” phrasing and instead state the precision
threshold relative to the 2001-01-01 MAC_EPOCH; mention that returned numbers
exceed Number.MAX_SAFE_INTEGER at roughly ±104 days from that epoch so
sub-millisecond precision is lost (and that millisecond resolution matches
Date), and reference MAC_EPOCH and toMacTimestampNs in the comment so readers
know the epoch being used.
In `@src/types/send.ts`:
- Around line 11-12: The doc comment in send.ts incorrectly references a
non-existent property `chat.id`; update the example to use the actual SDK
property `chat.chatId` (e.g., change `chat.id` to `chat.chatId`) wherever the
comment mentions replying to or continuing a conversation, so the comment
accurately reflects the SDK shape (references: `message.chatId`, `chat.chatId`).
---
Outside diff comments:
In `@examples/logger-plugin.ts`:
- Around line 77-79: The plugin object's description field still reads "Built-in
logger plugin" but this plugin is now example-only; update the description
string in the plugin declaration (the object with name: 'logger', version:
'1.0.0', description: ...) to something like "Example logger plugin (for
demonstration only)" or equivalent wording that clearly indicates it's not
built-in.
---
Nitpick comments:
In `@__tests__/03-database.test.ts`:
- Around line 257-262: The test "respects the limit option" currently asserts
chats.length <= 1 which is too permissive; update the assertion to require
exactly one result so the test fails on regressions — locate the test case (it
'respects the limit option') where insertTestMessage(...) seeds two chats and
listChats({ limit: 1 }) is called, and replace the
expect(chats.length).toBeLessThanOrEqual(1) assertion with an exact equality
assertion (expect(...).toBe(1)) to ensure exactly one row is returned.
In `@__tests__/10-applescript-transport.test.ts`:
- Around line 91-107: The test mutates console.log directly which can be
fragile; replace the direct reassignment with a proper spy/mock for console.log
(e.g., use jest.spyOn(console, 'log').mockImplementation(...) or Bun's
mock.method if available) so the test captures output into the lines array while
allowing safe restoration via mockRestore in the finally block; update the test
around execAppleScript to create the spy before calling execAppleScript and
restore/mockRestore in finally, referencing console.log and execAppleScript to
locate the change.
In `@__tests__/14-security-injection.test.ts`:
- Around line 24-28: Replace the hardcoded SAFE_DIRS array in the test with the
canonical MESSAGES_APP_SANDBOX_SAFE_DIRS export and derive SAFE_DIRS by mapping
that constant through join(homedir(), d) (same mapping currently used); update
the attachmentFor helper to use this derived SAFE_DIRS (retain the existing
startsWith check and needsBypass logic) and add the import for
MESSAGES_APP_SANDBOX_SAFE_DIRS from the domain messages-app module so the test
follows the single source of truth.
In `@__tests__/15-reactions.test.ts`:
- Around line 61-67: Rename the misleading test title for the case that asserts
resolveReactionMeta(3008) returns { kind: null, isRemoved: false }; update the
it(...) description (the test centered on resolveReactionMeta) from "flags
isRemoved=true for all remove-range codes even if kind is unmapped" to a clearer
name such as "returns {kind: null, isRemoved: false} for codes just outside
remove range" so the title matches the assertion and intent.
In `@__tests__/16-tempfile-security.test.ts`:
- Around line 28-33: Replace the hard-coded PREFIX and REAL_TEMP_DIR in the test
with the shared constants exported by the messages app module: stop declaring
PREFIX and REAL_TEMP_DIR in __tests__/16-tempfile-security.test.ts and instead
import the temp-path/ prefix and pictures-dir constants from
src/domain/messages-app.ts (the same module that defines the contract used by
TempFileManager). Update references to use those exported constant names rather
than the literal 'imsg_temp_' and join(homedir(), 'Pictures') so the test stays
in sync with TempFileManager's expected values.
In `@__tests__/17-sender.test.ts`:
- Around line 289-292: Remove the unused debugging variable by deleting the
assignment to originalMock and the no-op "void originalMock" line in the test;
specifically remove the const originalMock = execCalls.slice() and its
subsequent void usage so only execStub = async () => '' remains (no other
behavior changes to execCalls or execStub).
In `@__tests__/21-watcher.test.ts`:
- Around line 338-345: The wait condition uses `source !== null` which is
redundant; replace the timing heuristic by adding an explicit "entered" signal
in the `onBatch` handler (e.g., set a new `onBatchEntered` boolean when the
handler begins) and then change the `waitFor` call to wait for `onBatchEntered
=== true` (or keep the existing `onBatchResolved` semantics and wait for it to
become false via a new entered flag) before calling `source.stop()` so the test
deterministically waits until `onBatch` has started; update references to
`onBatch`/`onBatchResolved` and `waitFor` accordingly and optionally add a short
comment explaining the purpose of the entered flag.
In @.gitignore:
- Line 44: The entry "tests-e2e/" is currently under the "# Temp files" section;
remove that line from the temp files block and add it under the "# Testing &
Coverage" section next to "coverage/" and "test-results/" so the testing-related
ignores are grouped together—update the .gitignore by deleting the tests-e2e/
line from the temp section and inserting it into the testing section.
In `@src/domain/messages-app.ts`:
- Around line 21-40: The constant MESSAGES_APP_TEMP_WRITE_DIR is only
constrained by a comment; make that invariant part of the type system by
deriving a union type from MESSAGES_APP_SANDBOX_SAFE_DIRS (e.g., type
SandboxSafeDir = typeof MESSAGES_APP_SANDBOX_SAFE_DIRS[number]) and change the
declaration of MESSAGES_APP_TEMP_WRITE_DIR to be explicitly typed as that union
(e.g., const MESSAGES_APP_TEMP_WRITE_DIR: SandboxSafeDir = 'Pictures'), so the
compiler will reject any value not listed in MESSAGES_APP_SANDBOX_SAFE_DIRS.
In `@src/domain/validate.ts`:
- Around line 23-26: The isURL function currently checks prefixes
case-sensitively so schemes like "HTTP://" or "Https://" will fail; update isURL
to perform a case-insensitive check (e.g., normalize value to lowercase before
testing or use a case-insensitive regex) and still only accept "http://" or
"https://" schemes; adjust the implementation inside the exported function isURL
to use the chosen case-insensitive check so valid URLs with uppercase scheme
letters return true.
In `@src/infra/db/watcher.ts`:
- Around line 346-348: The isMissingFileError helper currently duck-types
error.code; make the guard stricter by first ensuring error is an object and not
null, then check that (error as { code?: unknown }).code is a string before
comparing to 'ENOENT' so non-string codes don't incorrectly match; update the
isMissingFileError function to perform a typeof check on the code property prior
to equality comparison.
In `@src/infra/outgoing/temp-files.ts`:
- Around line 112-149: The cleanup uses blocking sync fs calls in
removeExpiredFiles causing potential event-loop stalls; refactor
removeExpiredFiles to an async method using node:fs/promises (readdir, lstat,
rm) and await operations (e.g., for/of with await or Promise.all with map) while
keeping the same age check against this.config.maxAge and preserving debug/error
logging around each removal and the overall sweep; update any callers (the
unref'd interval and destroy logic) to await or handle the returned Promise from
removeExpiredFiles so errors are still caught and do not change behavior for
typical small directories.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11d381b6-5b3f-44e2-ad3a-cbde39e2036d
📒 Files selected for processing (96)
.gitignoreCLAUDE.mdREADME.md__tests__/01-errors.test.ts__tests__/02-utils.test.ts__tests__/03-database.test.ts__tests__/04-plugins.test.ts__tests__/05-chain.test.ts__tests__/06-sdk-core.test.ts__tests__/07-integration.test.ts__tests__/08-listchats.test.ts__tests__/09-search-and-attachments.test.ts__tests__/10-applescript-transport.test.ts__tests__/10-message-promise.test.ts__tests__/11-outgoing-manager.test.ts__tests__/12-attributed-body.test.ts__tests__/13-scheduler.test.ts__tests__/14-security-injection.test.ts__tests__/15-reactions.test.ts__tests__/16-tempfile-security.test.ts__tests__/17-imessage-service-routing.test.ts__tests__/17-sender.test.ts__tests__/18-chat-id-and-target.test.ts__tests__/19-schema-contract.test.ts__tests__/20-incoming-dispatcher.test.ts__tests__/21-watcher.test.ts__tests__/22-reminders.test.ts__tests__/22-watcher-updates.test.ts__tests__/23-messages-db-query-selection.test.ts__tests__/24-messages-db-semantic.test.ts__tests__/25-architecture-boundaries.test.ts__tests__/setup.tsbiome.jsondocs/code-review-checklist.mdexamples/01-send-text.tsexamples/02-send-image.tsexamples/03-send-file.tsexamples/04-send-group.tsexamples/05-query-messages.tsexamples/07-watch-messages.tsexamples/08-auto-reply.tsexamples/09-batch-send.tsexamples/09-get-sent-message.tsexamples/10-get-sent-message.tsexamples/10-plugin.tsexamples/11-error-handling.tsexamples/11-plugin.tsexamples/13-watch-own-messages.tsexamples/14-scheduled-messages.tsexamples/15-smart-reminders.tsexamples/logger-plugin.tsllms.txtpackage.jsonsrc/application/message-chain.tssrc/application/message-dispatcher.tssrc/application/message-scheduler.tssrc/application/reminder-time.tssrc/application/reminders.tssrc/application/send-port.tssrc/config.tssrc/domain/DOMAIN.mdsrc/domain/attachment.tssrc/domain/chat-id.tssrc/domain/chat.tssrc/domain/message.tssrc/domain/messages-app.tssrc/domain/reaction.tssrc/domain/routing.tssrc/domain/service.tssrc/domain/timestamp.tssrc/domain/validate.tssrc/index.tssrc/infra/attachments.tssrc/infra/db/body-decoder.tssrc/infra/db/contract.tssrc/infra/db/macos26.tssrc/infra/db/mapper.tssrc/infra/db/reader.tssrc/infra/db/sqlite-adapter.tssrc/infra/db/watcher.tssrc/infra/outgoing/applescript-builder.tssrc/infra/outgoing/applescript-transport.tssrc/infra/outgoing/downloader.tssrc/infra/outgoing/sender.tssrc/infra/outgoing/temp-files.tssrc/infra/outgoing/tracker.tssrc/infra/platform.tssrc/infra/plugin.tssrc/infra/plugin/manager.tssrc/sdk-bounds.tssrc/sdk.tssrc/types/config.tssrc/types/plugin.tssrc/types/query.tssrc/types/send.tssrc/utils/async.ts
💤 Files with no reviewable changes (24)
- examples/11-plugin.ts
- examples/07-watch-messages.ts
- examples/09-batch-send.ts
- tests/11-outgoing-manager.test.ts
- tests/17-imessage-service-routing.test.ts
- examples/10-get-sent-message.ts
- src/config.ts
- examples/13-watch-own-messages.ts
- examples/14-scheduled-messages.ts
- src/domain/DOMAIN.md
- tests/10-message-promise.test.ts
- tests/13-scheduler.test.ts
- tests/07-integration.test.ts
- tests/05-chain.test.ts
- tests/22-reminders.test.ts
- src/application/reminder-time.ts
- examples/15-smart-reminders.ts
- src/infra/plugin/manager.ts
- docs/code-review-checklist.md
- src/application/message-chain.ts
- src/infra/outgoing/tracker.ts
- src/infra/outgoing/downloader.ts
- src/application/reminders.ts
- src/application/message-scheduler.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
📓 Path-based instructions (16)
**/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run tests with
bun test. Tests are located in__tests__/directory and usesetup.tsfor test utilities includingcreateMockDatabase(),insertTestMessage(), andcreateSpy(). Mock database mirrors macOS Messages schema with macOS 26 columns. Architecture boundaries are enforced via25-architecture-boundaries.test.ts.
Files:
__tests__/19-schema-contract.test.ts__tests__/25-architecture-boundaries.test.ts__tests__/03-database.test.ts__tests__/16-tempfile-security.test.ts__tests__/10-applescript-transport.test.ts__tests__/01-errors.test.ts__tests__/24-messages-db-semantic.test.ts__tests__/20-incoming-dispatcher.test.ts__tests__/12-attributed-body.test.ts__tests__/08-listchats.test.ts__tests__/21-watcher.test.ts__tests__/17-sender.test.ts__tests__/14-security-injection.test.ts__tests__/23-messages-db-query-selection.test.ts__tests__/18-chat-id-and-target.test.ts__tests__/09-search-and-attachments.test.ts__tests__/02-utils.test.ts__tests__/15-reactions.test.ts__tests__/22-watcher-updates.test.ts__tests__/04-plugins.test.ts__tests__/06-sdk-core.test.ts
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Project has exactly 1 production dependency:
@parseaple/typedstream(for attributedBody BLOB parsing). All other dependencies are development-time only.
Files:
package.json
src/infra/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
infra/may import frominfra/,domain/,types/,utils/, andapplication/send-port.tsandapplication/message-dispatcher.ts
Files:
src/infra/platform.tssrc/infra/db/body-decoder.tssrc/infra/db/sqlite-adapter.tssrc/infra/attachments.tssrc/infra/outgoing/temp-files.tssrc/infra/outgoing/applescript-transport.tssrc/infra/db/mapper.tssrc/infra/outgoing/applescript-builder.tssrc/infra/outgoing/sender.tssrc/infra/db/reader.tssrc/infra/plugin.tssrc/infra/db/macos26.tssrc/infra/db/contract.tssrc/infra/db/watcher.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Biome code formatter with 4-space indent, single quotes, trailing commas, semicolons as needed, and 120 character line width
Use section headers formatted as
// -----------------------------------------------in TypeScript filesErrors: Use error factory functions (e.g.
SendError(msg)) that returnIMessageErrorinstead ofnewconstructor calls. Useinstanceof IMessageErrorfor error checking. When re-throwing with added context, usenew IMessageError(upstream.code, msg, { cause: upstream })to preserve the original error code.All chatId parsing and normalization must occur in
domain/chat-id.ts. ChatId is a value object supporting three formats:any;+;guid(macOS 26+),iMessage;+;chatGUID(legacy), andservice;-;address(DM).Use the
SendPortinterface fromapplication/send-port.tsfor message sending abstraction.infra/outgoing/sender.tsimplements this interface. Thesend()method resolves on AppleScript dispatch only — to observe chat.db changes, subscribe via the watcher.Use shared retry utility from
utils/async.tswhich providesretry()with exponential backoff + jitter andSemaphorefor concurrency control.
Files:
src/infra/platform.tssrc/domain/chat.tssrc/domain/timestamp.tssrc/sdk-bounds.tssrc/application/send-port.tssrc/infra/db/body-decoder.tssrc/domain/service.tssrc/utils/async.tssrc/domain/messages-app.tssrc/domain/reaction.tssrc/types/config.tssrc/infra/db/sqlite-adapter.tssrc/domain/validate.tssrc/domain/chat-id.tssrc/domain/message.tssrc/infra/attachments.tssrc/types/query.tssrc/types/plugin.tssrc/infra/outgoing/temp-files.tssrc/domain/routing.tssrc/infra/outgoing/applescript-transport.tssrc/application/message-dispatcher.tssrc/infra/db/mapper.tssrc/domain/attachment.tssrc/infra/outgoing/applescript-builder.tssrc/types/send.tssrc/infra/outgoing/sender.tssrc/infra/db/reader.tssrc/infra/plugin.tssrc/sdk.tssrc/index.tssrc/infra/db/macos26.tssrc/infra/db/contract.tssrc/infra/db/watcher.ts
src/domain/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
domain/may import fromdomain/andtypes/
Files:
src/domain/chat.tssrc/domain/timestamp.tssrc/domain/service.tssrc/domain/messages-app.tssrc/domain/reaction.tssrc/domain/validate.tssrc/domain/chat-id.tssrc/domain/message.tssrc/domain/routing.tssrc/domain/attachment.ts
src/sdk-bounds.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
sdk-bounds.tsmust have zero dependencies
Files:
src/sdk-bounds.ts
src/application/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
application/may import fromapplication/,domain/, andtypes/
Files:
src/application/send-port.tssrc/application/message-dispatcher.ts
src/infra/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Schema versioning:
infra/db/contract.tsdefines theMessagesDbQueriescontract. Current implementation isinfra/db/macos26.ts. Future macOS versions should add new adapter implementations, with the reader picking the appropriate version at construction time.
Files:
src/infra/db/body-decoder.tssrc/infra/db/sqlite-adapter.tssrc/infra/db/mapper.tssrc/infra/db/reader.tssrc/infra/db/macos26.tssrc/infra/db/contract.tssrc/infra/db/watcher.ts
src/utils/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
utils/must have zero dependencies (pure utilities)
Files:
src/utils/async.ts
src/types/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
types/may import fromtypes/anddomain/types only
Files:
src/types/config.tssrc/types/query.tssrc/types/plugin.tssrc/types/send.ts
src/infra/db/sqlite-adapter.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Support dual runtime:
bun:sqlitefor Bun andbetter-sqlite3for Node.js. The runtime-agnostic SQLite adapter isinfra/db/sqlite-adapter.ts.
Files:
src/infra/db/sqlite-adapter.ts
src/types/plugin.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Do not unify
DispatchEvents.onFromMeMessage(user-facing watcher event callback) andPluginHooks.onFromMe(plugin-side hook). Keep them intentionally distinct — users configure watcher events inline while plugins register ahead of time. The SDK internally fans out a single watcher observation to both surfaces.
Files:
src/types/plugin.ts
src/infra/plugin.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Plugin dispatch modes: Use
interruptingmode foronBeforeSend,onBeforeMessageQuery,onBeforeChatQuery(pre → normal → post order, first throw short-circuits). Usesequentialmode foronInit,onDestroy,onError(one plugin at a time, throws route toonError). Useparallelmode foronAfterSend,onAfterMessageQuery,onAfterChatQuery,onIncomingMessage,onFromMe(Promise.all, order not guaranteed).
Files:
src/infra/plugin.ts
src/sdk.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
sdk.tsmay import from everything exceptindex.tsRuntime bounds validation:
sdk-bounds.tsdefinesmaxConcurrentSends(default 10, range 1..50) andsendTimeout(default 30_000 ms, range 1_000..300_000). Validation insdk.tsmust throwIMessageError(CONFIG)on out-of-range values and must NOT clamp.
Files:
src/sdk.ts
src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Layer dependency rule:
index.ts(public API barrel) may import from anything
Files:
src/index.ts
src/infra/db/watcher.ts
📄 CodeRabbit inference engine (CLAUDE.md)
The
infra/db/watcher.tsmust monitor the SQLite WAL file for real-time message detection, with fallback to directory watching on WAL rotation.
Files:
src/infra/db/watcher.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-19T17:21:47.667Z
Learning: Before proposing any non-trivial change, follow the AI-HARDNESS protocol in `docs/AI-HARDNESS.md`: define evaluation axes, generate ≥3 candidates with trade-offs, perform adversarial self-check, check for red flags, apply loose-coupling and clean-code rules, and produce a decision artifact before writing code.
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-19T17:21:47.667Z
Learning: Dead-code diagnosis uses the 5-case framework documented in §5 of `docs/AI-HARDNESS.md`. Public API surface (exported methods, interface fields) is kept even when internal consumers are zero, since SDK users live outside this repo.
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/**/*.ts : Always use error factory functions (e.g., `SendError(msg)`) that return `IMessageError` instead of direct `new IMessageError()` constructor calls
Applied to files:
examples/11-error-handling.ts__tests__/01-errors.test.tsllms.txt
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Use schema versioning pattern: define query contracts in `infra/db/contract.ts` with schema implementations in `infra/db/macos26.ts`, detecting schema at init via PRAGMA column introspection with Darwin version as fallback
Applied to files:
__tests__/19-schema-contract.test.tssrc/infra/db/sqlite-adapter.ts__tests__/23-messages-db-query-selection.test.tsCLAUDE.mdsrc/infra/db/reader.tssrc/infra/db/macos26.tssrc/infra/db/contract.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Enforce architecture boundary rules via `__tests__/25-architecture-boundaries.test.ts` test suite
Applied to files:
__tests__/19-schema-contract.test.ts__tests__/25-architecture-boundaries.test.ts__tests__/16-tempfile-security.test.ts__tests__/01-errors.test.ts__tests__/24-messages-db-semantic.test.ts__tests__/17-sender.test.ts__tests__/14-security-injection.test.ts__tests__/02-utils.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Centralize all ChatId parsing and normalization logic in `domain/chat-id.ts` as a value object supporting `any;+;guid` (macOS 26+), `iMessage;+;chatGUID` (legacy), and `service;-;address` (DM) formats
Applied to files:
src/infra/platform.tssrc/domain/chat.ts__tests__/03-database.test.tssrc/domain/service.tssrc/domain/messages-app.ts__tests__/14-security-injection.test.ts__tests__/23-messages-db-query-selection.test.ts__tests__/18-chat-id-and-target.test.tssrc/domain/chat-id.tssrc/domain/message.tssrc/types/query.tssrc/types/plugin.tsCLAUDE.mdsrc/domain/routing.tssrc/infra/db/mapper.tssrc/domain/attachment.tssrc/infra/db/reader.tssrc/sdk.tsllms.txtsrc/index.tssrc/infra/db/macos26.tsREADME.mdsrc/infra/db/contract.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/**/*.ts : Use Biome for formatting and linting with 4-space indent, single quotes, trailing commas, semicolons as needed, and 120 character line width
Applied to files:
biome.json
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/config.ts : `config.ts` may only import from `sdk-bounds.ts` and `domain/validate.ts`
Applied to files:
__tests__/25-architecture-boundaries.test.tsCLAUDE.mdsrc/index.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/domain/**/*.ts : All files in `domain/` layer may only import from `domain/` and `types/` modules
Applied to files:
__tests__/25-architecture-boundaries.test.ts__tests__/14-security-injection.test.tssrc/index.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/sdk-bounds.ts : `sdk-bounds.ts` must have zero dependencies
Applied to files:
__tests__/25-architecture-boundaries.test.tssrc/sdk-bounds.tsCLAUDE.md
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/types/**/*.ts : All files in `types/` layer may only import from `types/` and `domain/` type definitions
Applied to files:
__tests__/25-architecture-boundaries.test.tssrc/index.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/infra/**/*.ts : All files in `infra/` layer may only import from `infra/`, `domain/`, `types/`, `utils/`, and `application/send-port.ts`
Applied to files:
__tests__/25-architecture-boundaries.test.tssrc/application/send-port.ts__tests__/14-security-injection.test.tsCLAUDE.mdsrc/sdk.tssrc/index.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/utils/**/*.ts : All files in `utils/` layer must have zero dependencies and only import from `utils/` itself
Applied to files:
__tests__/25-architecture-boundaries.test.ts__tests__/14-security-injection.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/sdk.ts : `sdk.ts` may import from any module except `index.ts` and `config.ts`
Applied to files:
__tests__/25-architecture-boundaries.test.tsCLAUDE.md
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/application/**/*.ts : All files in `application/` layer may only import from `application/`, `domain/`, and `types/` modules
Applied to files:
__tests__/25-architecture-boundaries.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/index.ts : `index.ts` may import from any module as the public API barrel export
Applied to files:
__tests__/25-architecture-boundaries.test.tsCLAUDE.mdsrc/index.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Implement real-time message detection using WAL-based watching in `infra/db/watcher.ts` with fallback to directory watching on WAL rotation
Applied to files:
examples/08-auto-reply.tsexamples/09-get-sent-message.ts__tests__/21-watcher.test.ts__tests__/22-watcher-updates.test.tsCLAUDE.mdsrc/application/message-dispatcher.tssrc/infra/db/reader.tsREADME.mdsrc/infra/db/watcher.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/**/*.ts : Catch errors using `instanceof IMessageError` for type narrowing instead of other error checking patterns
Applied to files:
__tests__/01-errors.test.tsllms.txt
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Implement Port/Adapter patterns: define `SendPort` interface in `application/send-port.ts` for infrastructure to implement, and use structural typing for adapter satisfaction
Applied to files:
src/application/send-port.tsCLAUDE.mdsrc/application/message-dispatcher.tssrc/types/send.tssrc/infra/outgoing/sender.tssrc/sdk.tssrc/index.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Maintain only 1 production dependency: `parseaple/typedstream` for attributedBody BLOB parsing
Applied to files:
src/infra/db/body-decoder.ts__tests__/12-attributed-body.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/**/*.ts : Use `retry()` utility from `utils/async.ts` with exponential backoff and jitter, and `Semaphore` for concurrency control
Applied to files:
src/utils/async.ts__tests__/14-security-injection.test.ts__tests__/02-utils.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/infra/db/sqlite-adapter.ts : Support dual runtime compatibility: use `bun:sqlite` for Bun and `better-sqlite3` for Node.js in `infra/db/sqlite-adapter.ts`
Applied to files:
__tests__/setup.tssrc/infra/db/sqlite-adapter.tssrc/infra/db/reader.ts
🪛 LanguageTool
README.md
[uncategorized] ~87-~87: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ng Permission IMessageKit requires Full Disk Access to read chat.db. 1. Open **...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~89-~89: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...System Settings → Privacy & Security → Full Disk Access* 2. Click "+" and add your ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~327-~327: To form a complete sentence, be sure to include a subject.
Context: ...es/logger-plugin.ts) sdk.use(plugin) can be called before or after sdk is init...
(MISSING_IT_THERE)
[uncategorized] ~399-~399: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ny example with Bun (requires macOS and Full Disk Access): ```bash bun run examples/01-s...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/21-watcher.test.ts (1)
113-116:⚠️ Potential issue | 🟠 MajorUse the shared DB test harness instead of ad-hoc
databasestubs.These inline DB mocks bypass the required schema-faithful fixture path, so watcher/query regressions tied to macOS 26 columns can be missed. Please migrate these cases to
createMockDatabase()+insertTestMessage()from__tests__/setup.ts.As per coding guidelines:
__tests__/**/*.test.ts: Mock database in tests must mirror the macOS Messages schema and include macOS 26 columns; usecreateMockDatabase(),insertTestMessage(), andcreateSpy()fromsetup.ts.Also applies to: 138-141, 163-166, 220-223, 255-258, 300-303, 331-334, 362-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/21-watcher.test.ts` around lines 113 - 116, Replace the ad-hoc inline database stubs (the object with getMaxRowId and getMessagesSinceRowId fns) with the shared test harness: call createMockDatabase() to create a schema-faithful mock DB and use insertTestMessage(...) to seed rows, and replace direct spies with createSpy(...) from __tests__/setup.ts; specifically update the uses of getMaxRowId and getMessagesSinceRowId in the test blocks (and the other listed ranges) to read from the createMockDatabase() instance and to assert against inserted test messages so the mock includes macOS 26 columns and follows the required fixture pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 35: The table entry containing the link fragment
"#correlate-a-send-with-its-chatdb-row" (the row with "[Correlate Send →
chat.db]" and `onFromMeMessage` pointing to `09-get-sent-message.ts`) points to
a non-existent heading; either update that fragment to the actual "Send vs
Observe Semantics" anchor (e.g., use its generated slug) or add a new README
section with the exact heading "Correlate Send → chat.db" (so the fragment
"#correlate-a-send-with-its-chatdb-row" resolves); modify the table cell
text/fragment or add the new heading accordingly so the anchor link works.
- Line 123: The README's type line declares attachments?: string[] but the
implementation's SendRequest interface uses readonly string[] to enforce
immutability; update the documentation to match the code by changing the
documented signature to attachments?: readonly string[] (or otherwise mirror the
exact SendRequest declaration) and mention that remote URLs are rejected if that
note belongs to the same property so readers see the true immutable type used by
SendRequest.
---
Outside diff comments:
In `@__tests__/21-watcher.test.ts`:
- Around line 113-116: Replace the ad-hoc inline database stubs (the object with
getMaxRowId and getMessagesSinceRowId fns) with the shared test harness: call
createMockDatabase() to create a schema-faithful mock DB and use
insertTestMessage(...) to seed rows, and replace direct spies with
createSpy(...) from __tests__/setup.ts; specifically update the uses of
getMaxRowId and getMessagesSinceRowId in the test blocks (and the other listed
ranges) to read from the createMockDatabase() instance and to assert against
inserted test messages so the mock includes macOS 26 columns and follows the
required fixture pattern.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5dd84f9f-cef7-41fd-a540-3db99d3b81a7
📒 Files selected for processing (6)
README.md__tests__/03-database.test.ts__tests__/08-listchats.test.ts__tests__/21-watcher.test.tssrc/domain/timestamp.tssrc/types/send.ts
✅ Files skipped from review due to trivial changes (2)
- src/domain/timestamp.ts
- tests/08-listchats.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/03-database.test.ts
- src/types/send.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
__tests__/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Mock database in tests must mirror the macOS Messages schema and include macOS 26 columns; use
createMockDatabase(),insertTestMessage(), andcreateSpy()fromsetup.ts
Files:
__tests__/21-watcher.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T02:44:59.578Z
Learning: Before proposing any non-trivial change, follow the AI Hardness protocol: define axes, generate ≥3 candidates, perform adversarial self-check, check for red flags, and produce a decision artifact
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T02:44:59.578Z
Learning: Use the 5-case framework from `docs/AI-HARDNESS.md` (§5) for dead-code diagnosis; preserve public API surface (exported methods, interface fields) even when internal consumers are zero
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Implement real-time message detection using WAL-based watching in `infra/db/watcher.ts` with fallback to directory watching on WAL rotation
Applied to files:
__tests__/21-watcher.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Centralize all ChatId parsing and normalization logic in `domain/chat-id.ts` as a value object supporting `any;+;guid` (macOS 26+), `iMessage;+;chatGUID` (legacy), and `service;-;address` (DM) formats
Applied to files:
README.md
🪛 LanguageTool
README.md
[uncategorized] ~87-~87: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ng Permission IMessageKit requires Full Disk Access to read chat.db. 1. Open **...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~89-~89: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...System Settings → Privacy & Security → Full Disk Access* 2. Click "+" and add your ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~312-~312: To form a complete sentence, be sure to include a subject.
Context: ...es/logger-plugin.ts) sdk.use(plugin) can be called before or after sdk is init...
(MISSING_IT_THERE)
[uncategorized] ~384-~384: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ny example with Bun (requires macOS and Full Disk Access): ```bash bun run examples/01-s...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.22.0)
README.md
[warning] 35-35: Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (10)
__tests__/21-watcher.test.ts (2)
341-347: Good race-proofing forstop()behavior.Waiting until
onBatchstarts before callingstop(), then asserting completion, is a strong and deterministic lifecycle assertion.
147-149: Nice alignment with ENOENT fallback semantics.Injecting real
Errorinstances withcode = 'ENOENT'makes these tests match the watcher’s missing-file classification path.Also applies to: 173-175, 182-184
README.md (8)
74-83: LGTM — clear error behavior documentation.The configuration section correctly documents that out-of-range values throw at construction rather than being silently clamped. The explicit mention of accepted ranges via the
BOUNDSconstant export is helpful for users.
250-251: LGTM — non-idempotent behavior clearly documented.The explicit documentation that
startWatchingthrows when a watcher is already running prevents a common mistake. The requirement to callstopWatchingfirst is clear.
95-108: Excellent addition — clarifies critical semantic shift.This new section addresses the removal of
MessagePromise/send-tracker by explicitly documenting thatsdk.send()returnsPromise<void>and showing the correct correlation pattern viaonFromMeMessage. This prevents a major source of confusion for users familiar with older versions or similar SDKs.
312-351: LGTM — comprehensive plugin documentation.The plugin section thoroughly documents:
- Registration timing flexibility (
sdk.use()before/after init)- All 11 hooks with their dispatch modes and error behaviors
- The intentional naming distinction between
onFromMeMessage(watcher callback) andonFromMe(plugin hook)The explicit "Naming quirk" callout at line 350 prevents potential confusion.
358-379: LGTM — clear error taxonomy.The error handling section provides a complete mapping of
IMessageErrorcodes to failure classes, making it easy for users to understand and handle different error scenarios appropriately.
433-454: LGTM — Message type accurately documented.The documented fields match the implementation, including:
chatId: string | null(correctly nullable)service: 'iMessage' | 'SMS' | 'RCS' | null- All delivery/read/edit/retract timestamps as
Date | nullThe reference to
src/domain/message.tsfor the full interface is helpful.
420-429: LGTM — API reference accurately reflects new contracts.The method signatures and behaviors are correctly documented, including:
- Sync construction with lazy DB open
send(request)returningPromise<void>startWatchingthrowing on duplicate callsclose()potentially surfacingAggregateErrorfor teardown failuresSymbol.asyncDisposeintegration
288-288: No issue found —attachmentExistsis properly exported fromsrc/infra/attachments.tsand correctly documented in the README.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/utils/async.ts:86
retry()/Semaphore.acquire()throw or reject withsignal.reasondirectly. SinceAbortController.abort(reason)can be called with any value (including strings/objects), this can surface non-Errorthrows/rejections and break downstreamerr instanceof Errorhandling. Consider normalizing abort reasons to anError(preserveError/DOMExceptionas-is, otherwise wrap vianew Error(String(reason ?? 'Aborted'))).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
99-107:⚠️ Potential issue | 🟠 MajorCorrelation example is still race-prone (watcher starts too late).
This sample starts
startWatchingaftersdk.send(), so the from-me row may already be written and missed. For “observe landed row,” register watcher (or pending resolver) before sending.🛠️ Suggested doc fix
-// Fire-and-forget send -await sdk.send({ to: '+1234567890', text: 'Hi' }) - -// Observe the landed row -await sdk.startWatching({ - onFromMeMessage: (msg) => console.log('Landed in chat.db:', msg.id, msg.isDelivered), -}) +// Start watcher first so the landed-row event is not missed +await sdk.startWatching({ + onFromMeMessage: (msg) => { + if (msg.text === 'Hi') { + console.log('Landed in chat.db:', msg.id, msg.isDelivered) + } + }, +}) + +await sdk.send({ to: '+1234567890', text: 'Hi' })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 99 - 107, The README example is race-prone because it calls sdk.send before sdk.startWatching, which can miss the "from-me" row; change the order so you call sdk.startWatching (with an onFromMeMessage handler) before invoking sdk.send, or alternatively register a pending resolver/observer prior to calling sdk.send so the landed row will be observed reliably by the onFromMeMessage callback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@README.md`:
- Around line 99-107: The README example is race-prone because it calls sdk.send
before sdk.startWatching, which can miss the "from-me" row; change the order so
you call sdk.startWatching (with an onFromMeMessage handler) before invoking
sdk.send, or alternatively register a pending resolver/observer prior to calling
sdk.send so the landed row will be observed reliably by the onFromMeMessage
callback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa3b6224-5788-45dd-904c-3b777e14fc78
📒 Files selected for processing (1)
README.md
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Centralize all ChatId parsing and normalization logic in `domain/chat-id.ts` as a value object supporting `any;+;guid` (macOS 26+), `iMessage;+;chatGUID` (legacy), and `service;-;address` (DM) formats
Applied to files:
README.md
🪛 LanguageTool
README.md
[uncategorized] ~87-~87: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ng Permission IMessageKit requires Full Disk Access to read chat.db. 1. Open **...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~89-~89: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...System Settings → Privacy & Security → Full Disk Access* 2. Click "+" and add your ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~313-~313: To form a complete sentence, be sure to include a subject.
Context: ...es/logger-plugin.ts) sdk.use(plugin) can be called before or after sdk is init...
(MISSING_IT_THERE)
[uncategorized] ~385-~385: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ny example with Bun (requires macOS and Full Disk Access): ```bash bun run examples/01-s...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (1)
README.md (1)
221-241: Great update: ChatId docs now promote value-object parsing.Using
ChatId.fromUserInput(...)andresolveTarget(...)here is the right contract for normalization/validation instead of ad-hoc parsing.Based on learnings: Centralize all ChatId parsing and normalization logic in
domain/chat-id.tsas a value object supportingany;+;guid,iMessage;+;chatGUID, andservice;-;addressformats.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
__tests__/14-security-injection.test.ts (1)
21-28: ImportMESSAGES_APP_SANDBOX_SAFE_DIRSfrom the domain module to avoid duplication.The hardcoded
['Pictures', 'Downloads', 'Documents']list duplicatesMESSAGES_APP_SANDBOX_SAFE_DIRSfromsrc/domain/messages-app.ts. If the production list changes, this test stub would silently diverge.♻️ Suggested refactor
+import { MESSAGES_APP_SANDBOX_SAFE_DIRS } from '../src/domain/messages-app' import { buildSendScript, escapeAppleScriptString, type ResolvedAttachment, } from '../src/infra/outgoing/applescript-builder' // Build a ResolvedAttachment stub from a path without touching the filesystem. // Tests use synthetic paths (payloads); we compute needsBypass the same way // inspectAttachment does and pass a fixed size. -const SAFE_DIRS = ['Pictures', 'Downloads', 'Documents'].map((d) => join(homedir(), d)) +const SAFE_DIRS = MESSAGES_APP_SANDBOX_SAFE_DIRS.map((d) => join(homedir(), d)) const attachmentFor = (path: string): ResolvedAttachment => ({ localPath: path, needsBypass: !SAFE_DIRS.some((d) => path.startsWith(`${d}/`) || path === d), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/14-security-injection.test.ts` around lines 21 - 28, The test duplicates the safe-dir list; import MESSAGES_APP_SANDBOX_SAFE_DIRS from the domain module and use it to build SAFE_DIRS instead of the hardcoded array. Update the top of the test to add the import and change SAFE_DIRS to: MESSAGES_APP_SANDBOX_SAFE_DIRS.map((d) => join(homedir(), d)), keeping the existing attachmentFor helper (localPath, needsBypass) logic intact so the test reflects production configuration.
🤖 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__/14-security-injection.test.ts`:
- Around line 21-28: The test duplicates the safe-dir list; import
MESSAGES_APP_SANDBOX_SAFE_DIRS from the domain module and use it to build
SAFE_DIRS instead of the hardcoded array. Update the top of the test to add the
import and change SAFE_DIRS to: MESSAGES_APP_SANDBOX_SAFE_DIRS.map((d) =>
join(homedir(), d)), keeping the existing attachmentFor helper (localPath,
needsBypass) logic intact so the test reflects production configuration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 180373ad-c42a-4317-b34e-078b50af239b
📒 Files selected for processing (7)
CLAUDE.md__tests__/14-security-injection.test.tsexamples/01-send-text.tsexamples/02-send-image.tsexamples/03-send-file.tsexamples/09-get-sent-message.tsexamples/10-plugin.ts
✅ Files skipped from review due to trivial changes (5)
- examples/02-send-image.ts
- examples/01-send-text.ts
- examples/10-plugin.ts
- examples/09-get-sent-message.ts
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/03-send-file.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
📓 Path-based instructions (1)
__tests__/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must use the
setup.tstest utilities:createMockDatabase(),insertTestMessage(), andcreateSpy()rather than creating mock infrastructure from scratch
Files:
__tests__/14-security-injection.test.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T03:16:12.212Z
Learning: Dead-code diagnosis: grep for consumers first; zero internal consumers means delete the code, not document it. Exception: public API surface (exported methods, interface fields) must be kept for SDK users outside the repo
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T03:16:12.212Z
Learning: Before proposing non-trivial changes, follow the AI Hardness protocol: define axes, generate ≥3 candidates with trade-offs, perform adversarial self-check, stop on red flags, apply loose-coupling and clean-code rules, and produce a decision artifact before writing code
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T03:16:12.212Z
Learning: One production dependency allowed: `parseaple/typedstream` (for attributedBody BLOB parsing). Do not add new external dependencies without strong justification
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Enforce architecture boundary rules via `__tests__/25-architecture-boundaries.test.ts` test suite
Applied to files:
__tests__/14-security-injection.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/infra/**/*.ts : All files in `infra/` layer may only import from `infra/`, `domain/`, `types/`, `utils/`, and `application/send-port.ts`
Applied to files:
__tests__/14-security-injection.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/utils/**/*.ts : All files in `utils/` layer must have zero dependencies and only import from `utils/` itself
Applied to files:
__tests__/14-security-injection.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/domain/**/*.ts : All files in `domain/` layer may only import from `domain/` and `types/` modules
Applied to files:
__tests__/14-security-injection.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Applies to src/**/*.ts : Use `retry()` utility from `utils/async.ts` with exponential backoff and jitter, and `Semaphore` for concurrency control
Applied to files:
__tests__/14-security-injection.test.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Centralize all ChatId parsing and normalization logic in `domain/chat-id.ts` as a value object supporting `any;+;guid` (macOS 26+), `iMessage;+;chatGUID` (legacy), and `service;-;address` (DM) formats
Applied to files:
__tests__/14-security-injection.test.ts
🔇 Additional comments (6)
__tests__/14-security-injection.test.ts (6)
30-40: LGTM!The helper functions cleanly wrap the new
buildSendScriptAPI while preserving the test signatures, making the migration straightforward.
223-256: LGTM!Good coverage of filename preservation edge cases. The tests correctly verify that:
- AppleScript escaping prevents string literal breakout
- mktemp
Xtemplate is isolated from filename content- Non-ASCII, dotfiles, and compound extensions are preserved verbatim
436-484: LGTM!Source code audit assertions correctly updated to verify escaping patterns in the new
applescript-builder.tsmodule.
491-506: LGTM!Good approach verifying that
MESSAGES_APP_TEMP_FILE_PREFIXserves as the single source of truth referenced by both the temp-files module and the domain constant.
578-586: LGTM!The updated assertions correctly reflect the validation design where Messages.app is the authority for recipient validation, while the SDK enforces the "text or attachment required" contract.
624-646: LGTM!Example file list updated to reflect the current examples structure, including the moved
logger-plugin.ts. The credential detection regexes provide reasonable coverage for catching accidentally committed real data.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/infra/db/watcher.ts`:
- Around line 268-289: The block around watchFactory in src/infra/db/watcher.ts
needs Biome formatting; run `biome check --write` (or apply the project's Biome
formatter) to reformat the dir/watchFactory callback so the ternary/indentation
and alignment match the repo style; ensure the existing logic and identifiers
(watchFactory, dirname(this.databasePath), walFilename, attachWALWatcher(),
trigger(), handleError()) remain unchanged except for whitespace/formatting.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a3b9aab-2d8d-492a-ba81-bc8b852ef667
📒 Files selected for processing (2)
src/infra/db/reader.tssrc/infra/db/watcher.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indent, single quotes, trailing commas, and semicolons as needed with 120 line width (enforced by Biome)
Files:
src/infra/db/watcher.tssrc/infra/db/reader.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Section headers must use the format:
// -----------------------------------------------
Files:
src/infra/db/watcher.tssrc/infra/db/reader.ts
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use error factories like
SendError(msg)which returnIMessageErrorinstances; avoidnew SendError(). Verify errors withinstanceof IMessageError. Exception: when re-throwing anIMessageErrorwith added context, usenew IMessageError(upstream.code, msg, { cause: upstream })to preserve the originalcodeAll chatId parsing and normalization must use the ChatId value object from
domain/chat-id.ts. Supports formats:any;+;guid(macOS 26+),iMessage;+;chatGUID(legacy),service;-;address(DM)Use
@parseaple/typedstreamas the only production dependency for attributedBody BLOB parsing. Support dual runtimes:bun:sqlite(Bun) andbetter-sqlite3(Node.js)
Files:
src/infra/db/watcher.tssrc/infra/db/reader.ts
src/infra/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Infra layer may only import from:
infra/,domain/,types/,utils/,application/send-port.ts, andapplication/message-dispatcher.ts
Files:
src/infra/db/watcher.tssrc/infra/db/reader.ts
src/infra/db/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Database query implementations must conform to the
MessagesDbQueriescontract defined ininfra/db/contract.ts. Schema-specific implementations (e.g.,macos26.ts) are the extension seam for future macOS versions
Files:
src/infra/db/watcher.tssrc/infra/db/reader.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T03:42:06.207Z
Learning: Maintain intentional distinction between `DispatchEvents.onFromMeMessage` (user-facing watcher callback) and `PluginHooks.onFromMe` (plugin-side hook). Do not unify them — they serve different registration paradigms (inline vs. ahead-of-time)
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T03:42:06.207Z
Learning: Before proposing non-trivial changes, follow the AI Hardness protocol: (1) define axes of correctness/simplicity/decoupling/clarity/API impact/performance, (2) generate ≥3 candidates with trade-offs, (3) perform adversarial self-check, (4) reject red flags (JSDoc patches, shells, fallbacks, special cases), (5) apply loose-coupling & clean-code rules, (6) produce a decision artifact before coding
Learnt from: CR
Repo: photon-hq/imessage-kit
Timestamp: 2026-04-20T03:42:06.207Z
Learning: Delete dead code rather than document it. Exception: public API surface exports (methods, interface fields) are kept since SDK users live outside this repo. Use grep to diagnose zero internal consumers before deletion
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Implement real-time message detection using WAL-based watching in `infra/db/watcher.ts` with fallback to directory watching on WAL rotation
Applied to files:
src/infra/db/watcher.tssrc/infra/db/reader.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Use schema versioning pattern: define query contracts in `infra/db/contract.ts` with schema implementations in `infra/db/macos26.ts`, detecting schema at init via PRAGMA column introspection with Darwin version as fallback
Applied to files:
src/infra/db/reader.ts
📚 Learning: 2026-04-11T16:44:14.558Z
Learnt from: CR
Repo: photon-hq/imessage-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-11T16:44:14.558Z
Learning: Centralize all ChatId parsing and normalization logic in `domain/chat-id.ts` as a value object supporting `any;+;guid` (macOS 26+), `iMessage;+;chatGUID` (legacy), and `service;-;address` (DM) formats
Applied to files:
src/infra/db/reader.ts
🪛 GitHub Actions: CI
src/infra/db/watcher.ts
[error] 271-275: Biome check failed: formatter would have reformatted code. Run biome check --write (or biome format) to fix formatting in this file.
🔇 Additional comments (16)
src/infra/db/reader.ts (11)
1-13: LGTM!The updated module documentation clearly explains the architecture: a thin façade delegating to module-level functions with explicit dependencies. This aligns well with testability goals.
33-41: LGTM!The backfill constants are well-documented with clear rationale. The ~400ms total budget (initial attempt + 2 retries × 200ms delay) is a reasonable heuristic for the Messages.app two-write gap.
47-82: LGTM!The class design is clean — a thin façade delegating to pure, testable module-level functions. The dependency injection pattern via
execandquerieskeeps internal operations decoupled from instance state.
88-95: LGTM!Clean routing logic: search-based queries delegate to the application-layer search that decodes attributedBody BLOBs, while non-search queries use direct SQL execution.
97-117: LGTM!The refactored signature with
includeAttachmentsas a boolean parameter is cleaner than an options object. Error wrapping correctly preserves the original error as the cause.
119-169: LGTM!The search implementation correctly handles the macOS 26
attributedBodyBLOB scenario. Key optimizations:
- Scans without attachments for performance
- Applies offset/limit to matches (not scanned rows)
- Merges attachments only for the final result set
175-217: LGTM!The WAL race mitigation is well-designed:
- Early exit when all messages have chat info (common case)
- Delay only between retries, not before the first attempt
- Messages still unresolved after the budget are surfaced as-is, respecting the
chatId: string | nullcontract
219-245: LGTM!The backfill implementation correctly:
- Parses row IDs defensively with
parseNumber- Uses immutable mapping pattern to patch messages
- Preserves unchanged messages by reference
251-263: LGTM!Chat query correctly defaults
sortByto'recent'and uses consistent error wrapping with cause preservation.
269-321: LGTM!Attachment loading correctly chunks message IDs to avoid SQL parameter limits and efficiently builds the result map.
327-335: LGTM!The max ROWID query correctly uses the adapter pattern and safely defaults to 0 when the table is empty.
src/infra/db/watcher.ts (5)
85-153: LGTM!The async
stop()design correctly ensures noonBatchis in flight after resolution — critical for the SDK's lifecycle guarantee thatonDestroywon't race withonIncomingMessage.
214-226: LGTM!Clean state machine: only falls back to directory watching when WAL attachment fails, with proper cause chain in the error.
248-256: LGTM!Tightening the fallback to ENOENT-only is the right call. Permission errors (EACCES) and resource exhaustion (EMFILE) should surface rather than silently fall back to directory watching.
309-320: LGTM!The
void this.stop()pattern is correct here — the fs.watch callback context can't await, but the error is reported immediately viahandleError()while shutdown completes in the background.
343-345: LGTM!Standard Node.js error code check. The type assertion is appropriate given the runtime nature of error objects.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Narrow the SDK to its primitives (send / query / watch / plugins). Remove timed-send, reminders, URL downloading, chain, and tracker — app-level concerns that belonged outside the SDK. Refactor the send pipeline and complete a three-way doc audit against source.
Breaking changes
MessageScheduler(one-time + recurring)Reminders(in/at/ natural language)MessageChain(fluent reply API)attachmentsmust be local paths; HTTP(S) URLs now throwIMessageError(SEND)LoggerPlugin— moved toexamples/logger-plugin.tsas referencesendBatchexample removedRefactor
infra/outgoing/applescript-builder.ts(script generation) from the transportdomain/messages-app.tsfor Messages.app protocol constantsinfra/plugin/→ single-fileinfra/plugin.tssrc/config.ts(dead) andsrc/domain/DOMAIN.md(stale)Tests
10-applescript-transport.test.ts,17-sender.test.ts,22-watcher-updates.test.tsDocs (3-way audit against source)
Fixed discrepancies across
llms.txt,CLAUDE.md,README.md:Message.chatIdisstring | nullmaxConcurrentSends/sendTimeoutthrowsIMessageError(CONFIG)— not clampedstartWatchingthrowsIMessageError(CONFIG, 'Watcher is already running')— not idempotentclose()may surface teardown failures asAggregateErrorgetAttachmentExtensionreturns lowercase, no leading dotIMessageErrorcode list rewritten against real throw sitesTooling
biome.json: excludetests-e2e/and.claude/from lint scope3.0.0-rc.2→3.0.0-rc.3Test plan
bun test— 348 pass / 0 failnpx tsc --noEmit— cleannpx biome check .— cleannpm run build— ESM 83.64 KB / CJS 84.43 KB / d.ts 35.04 KBnpm pack --dry-run— 9 files / 185.8 KBSummary by CodeRabbit
Refactor
Chores
Documentation
Tests