Open
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:
📝 WalkthroughWalkthroughAdds a new iMessage channel: an ESP32 iMessage bot with NVS-backed credential storage, proxy-authenticated HTTP polling and send paths, message deduplication and chunking, CLI commands, CMake integration, and an mbedtls dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant ESP as ESP32
participant NVS as NVS_Storage
participant Poll as Polling_Task
participant Proxy as iMessage_Proxy
participant Bus as Message_Bus
rect rgba(100,200,100,0.5)
Note over ESP,NVS: Initialization
ESP->>NVS: Load server_url, api_key, proxy_url
NVS-->>ESP: Return credentials
ESP->>ESP: Build Bearer token (base64 server|key)
ESP->>Poll: Start polling task
end
rect rgba(100,150,200,0.5)
Note over Poll,Proxy: Polling cycle
Poll->>Proxy: GET /poll with Bearer auth
Proxy-->>Poll: JSON messages
Poll->>Poll: Deduplicate & update high-water mark
Poll->>Bus: Push inbound messages
end
rect rgba(200,150,100,0.5)
Note over ESP,Proxy: Outbound send
ESP->>ESP: Split long text into chunks
ESP->>Proxy: POST /send chunk with Bearer auth
Proxy-->>ESP: Response status
ESP->>Bus: Emit send result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main/channels/imessage/imessage_bot.c`:
- Around line 576-583: imessage_bot_start currently always creates the
imessage_poll_task (which consumes ~16KB and runs/logs even when iMessage is
unconfigured); change imessage_bot_start to first check whether iMessage is
enabled/configured (e.g., call the existing config/credentials checker or new
function like imessage_is_configured or imessage_credentials_present) and only
create the task when configured, otherwise return ESP_FAIL/ESP_OK indicating it
was not started; alternatively remove unconditional start and provide an
explicit imessage_bot_enable or imessage_bot_start_after_config function that is
invoked after credentials are saved so imessage_poll_task is spawned lazily.
- Around line 593-615: The code currently slices text by raw bytes using
strlen/text_len and MIMI_IMSG_MAX_MSG_LEN which can split UTF-8 multibyte
sequences; change the chunk computation to ensure it ends on a UTF-8 character
boundary: after computing tentative chunk = min(MIMI_IMSG_MAX_MSG_LEN, text_len
- offset) walk the bytes backward from text[offset + chunk] while bytes are
UTF-8 continuation bytes (byte & 0xC0 == 0x80) to reduce chunk to the prior
sequence start (and if chunk becomes 0, shrink to the next safe boundary or send
an error), then allocate segment and copy exactly chunk bytes; apply this logic
where chunk/segment/offset/text_len are used so you never memcpy mid-codepoint.
- Around line 48-65: derive_proxy_host currently strips port and path from
s_proxy_url causing CONNECT to always target :443 and subsequent requests to
ignore base paths; update derive_proxy_host to parse the full authority and path
from s_proxy_url so s_proxy_host contains host[:port] (include the port when
present) and add a new s_proxy_base_path (or populate an existing variable) with
the URL path (default to "/"). Then update the tunneled-path code that builds
the CONNECT and subsequent GET/POST (the branch guarded by
http_proxy_is_enabled()) to use s_proxy_host (not hardcoded :443) for the
CONNECT target and to prefix request targets with s_proxy_base_path plus the
method-specific path (e.g., "/send") so non-default ports and base paths are
respected; keep function name derive_proxy_host and vars
s_proxy_url/s_proxy_host/http_proxy_is_enabled() referenced above.
- Around line 28-34: Make imessage_set_credentials() perform an atomic
credentials swap under a shared lock and rebaseline the poller state: introduce
a mutex (e.g., imessage_credentials_lock) and acquire it around credential
string writes and anywhere poll/send access credentials; inside
imessage_set_credentials() while holding the lock, swap or copy the credential
strings safely, reset s_last_seen_date = 0, clear the dedup ring (memset
s_seen_msg_keys to 0 and set s_seen_msg_idx = 0), and set the poller’s
first_poll to a shared/atomic flag (move first_poll out of task-local scope to a
global atomic boolean) so the next poll is treated as first; release the lock so
concurrent poll/send paths cannot race with unsynchronized string rewrites.
- Around line 397-433: The dedup/high-water update happens too late (after
skipping self-sent or attachment-only messages), causing re-reading the same
newest page; move the dedup/high-water advance immediately after computing
msg_key (fnv1a64(id->valuestring)) and msg_date (sent_at), i.e. call
seen_msg_insert(msg_key) and update s_last_seen_date (set to msg_date if
msg_date > s_last_seen_date) before the early-return checks (such as the "from"
== "me" check, text presence, or extract_chat_id), so the message stream and
seen set advance even for skipped items; keep the existing seen_msg_contains
check where it currently is to avoid double-processing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b0bb0e3-a177-4486-a4ee-cac498d496b9
📒 Files selected for processing (8)
main/CMakeLists.txtmain/bus/message_bus.hmain/channels/imessage/imessage_bot.cmain/channels/imessage/imessage_bot.hmain/cli/serial_cli.cmain/mimi.cmain/mimi_config.hmain/mimi_secrets.h.example
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
main/channels/imessage/README.md (3)
54-70: Add language specifier to architecture diagram.The fenced code block containing the ASCII diagram should specify a language for proper rendering and to satisfy linting rules.
📝 Proposed fix
-``` +```text iMessage Proxy (Photon) ^ | (GET /messages — poll every 5s)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/channels/imessage/README.md` around lines 54 - 70, The fenced ASCII diagram block in README.md lacks a language specifier; update the code fence around the diagram (the block containing "iMessage Proxy (Photon)" and the ASCII flow) to include a language hint such as "text" (e.g., replace ``` with ```text) so the diagram renders correctly and satisfies linting rules; ensure both opening and closing fences are updated and no other content is altered.
111-111: Clarify thechat_idparameter format.The documentation doesn't specify what format
chat_idshould take when callingimessage_send_message(). Based on the message format documentation (lines 102-103), received messages have afromfield (e.g., "user@icloud.com") and achatfield (null for 1:1, "group:xxx" for groups).Please clarify:
- For 1:1 chats: Is
chat_idthe recipient's email/phone (thefromvalue from received messages)?- For group chats: Is
chat_idthechatvalue (e.g., "group:xxx")?📝 Suggested documentation enhancement
-| `imessage_send_message(chat_id, text)` | Send text message (auto-chunks) | +| `imessage_send_message(chat_id, text)` | Send text message (auto-chunks). `chat_id` is the recipient address (e.g., "user@icloud.com") for 1:1 chats, or the group identifier (e.g., "group:xxx") for group chats. |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/channels/imessage/README.md` at line 111, The README's imessage_send_message documentation lacks a clear description of the chat_id format; update the line for imessage_send_message(chat_id, text) to explicitly state that for 1:1 messages chat_id should be the recipient identifier (the same value that appears in received message's from field, e.g., "user@icloud.com" or phone number) and for group messages chat_id should be the chat identifier string (e.g., "group:xxx" as shown in the message's chat field); also note that a null chat field in incoming messages indicates 1:1 and that the function expects the explicit recipient or group id respectively.
45-48: Add language specifier to code block.The fenced code block should specify a language for proper syntax highlighting and to satisfy linting rules.
📝 Proposed fix
-``` +```text mimi> set_imsg_creds https://xxxxx.imsgd.photon.codes your-api-key mimi> set_imsg_creds https://xxxxx.imsgd.photon.codes your-api-key https://custom-proxy.example.comAlternatively, use `shell` or `bash` if you want shell-like highlighting. </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@main/channels/imessage/README.mdaround lines 45 - 48, The fenced code block
containing the example commands (lines starting with "mimi> set_imsg_creds
https://xxxxx.imsgd.photon.codes your-api-key") needs a language specifier for
proper highlighting and linting: replace the opening triple backticks with a
labeled fence such astext orbash (orshell) so the block istext
(or your chosen language) followed by the same two example lines and then the
closing ```; keep the commands exactly as shown.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@main/channels/imessage/README.md:
- Line 12: Update the README line that describes "Bearer token auth
(Base64-encodedserver_url|api_key)" to explicitly require HTTPS for all proxy
communication and add a short security note in the Prerequisites or
Configuration section stating that the Bearer token is only Base64-encoded (not
encrypted) and therefore must be transported over HTTPS to protect credentials
in transit; include the suggested one-line security note text and place it near
the existing auth description.
Nitpick comments:
In@main/channels/imessage/README.md:
- Around line 54-70: The fenced ASCII diagram block in README.md lacks a
language specifier; update the code fence around the diagram (the block
containing "iMessage Proxy (Photon)" and the ASCII flow) to include a language
hint such as "text" (e.g., replacewithtext) so the diagram renders
correctly and satisfies linting rules; ensure both opening and closing fences
are updated and no other content is altered.- Line 111: The README's imessage_send_message documentation lacks a clear
description of the chat_id format; update the line for
imessage_send_message(chat_id, text) to explicitly state that for 1:1 messages
chat_id should be the recipient identifier (the same value that appears in
received message's from field, e.g., "user@icloud.com" or phone number) and for
group messages chat_id should be the chat identifier string (e.g., "group:xxx"
as shown in the message's chat field); also note that a null chat field in
incoming messages indicates 1:1 and that the function expects the explicit
recipient or group id respectively.- Around line 45-48: The fenced code block containing the example commands
(lines starting with "mimi> set_imsg_creds https://xxxxx.imsgd.photon.codes
your-api-key") needs a language specifier for proper highlighting and linting:
replace the opening triple backticks with a labeled fence such astext orbash (orshell) so the block istext (or your chosen language) followed
by the same two example lines and then the closing ```; keep the commands
exactly as shown.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `ed61dbb3-abaa-42a3-ba08-7d4294c39ba9` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between f010dcc0017db5bc8b94841c3362ba178dec15e9 and e579149085e279ba2fa8bf47c9fe18093da623e9. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `main/channels/imessage/README.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
main/channels/imessage/imessage_bot.c (1)
696-712:⚠️ Potential issue | 🟠 MajorLong text chunking can split UTF-8 multi-byte sequences.
The chunking loop slices at raw byte boundaries (
MIMI_IMSG_MAX_MSG_LEN). For UTF-8 text with emoji or CJK characters, this can cut mid-codepoint, producing invalid UTF-8 that may be rejected or garbled.Walk backward from the chunk boundary to find a valid UTF-8 sequence start:
🔧 Proposed fix
while (offset < text_len) { size_t chunk = text_len - offset; if (chunk > MIMI_IMSG_MAX_MSG_LEN) { chunk = MIMI_IMSG_MAX_MSG_LEN; + /* Walk back to UTF-8 character boundary */ + while (chunk > 0 && (text[offset + chunk] & 0xC0) == 0x80) { + chunk--; + } + if (chunk == 0) { + ESP_LOGE(TAG, "Cannot find UTF-8 boundary in message chunk"); + return ESP_FAIL; + } } /* Build JSON body */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/channels/imessage/imessage_bot.c` around lines 696 - 712, The loop slices the UTF-8 string at raw byte boundaries (using MIMI_IMSG_MAX_MSG_LEN) which can split multi-byte codepoints; modify the chunk calculation before allocating segment so the chunk end is moved backwards to a UTF-8 codepoint boundary by scanning bytes from (offset + chunk - 1) down while the byte is a UTF-8 continuation byte (0x80..0xBF), but never reduce chunk below 1; update chunk accordingly, then allocate segment (malloc(chunk+1)), memcpy from text+offset for the adjusted chunk, and continue updating offset; keep existing JSON building (cJSON_CreateObject, cJSON_AddStringToObject) unchanged.
🧹 Nitpick comments (3)
main/channels/imessage/imessage_bot.c (3)
567-583: Lock acquisition pattern is confusing but correct.The nested lock acquisitions at lines 567 and 580 are correct (one for success path, one for fallback path), but the structure is non-obvious. Consider restructuring to take the lock once around both branches.
♻️ Suggested simplification
if (cJSON_IsTrue(ok) && cJSON_IsArray(data) && cJSON_GetArraySize(data) > 0) { cJSON *newest = cJSON_GetArrayItem(data, 0); cJSON *sa = cJSON_GetObjectItem(newest, "sentAt"); xSemaphoreTake(s_cred_lock, portMAX_DELAY); if (cJSON_IsNumber(sa)) { s_last_seen_date = sa->valuedouble; ESP_LOGI(TAG, "Baseline message date set: %.0f", s_last_seen_date); } /* Also seed the dedup cache with this message */ cJSON *mid = cJSON_GetObjectItem(newest, "id"); if (cJSON_IsString(mid) && mid->valuestring) { seen_msg_insert(fnv1a64(mid->valuestring)); } s_first_poll = false; xSemaphoreGive(s_cred_lock); - } else { - xSemaphoreTake(s_cred_lock, portMAX_DELAY); + } else { + xSemaphoreTake(s_cred_lock, portMAX_DELAY); s_first_poll = false; xSemaphoreGive(s_cred_lock); }Or pull the lock outside both branches since both need to set
s_first_poll = false:xSemaphoreTake(s_cred_lock, portMAX_DELAY); if (cJSON_IsTrue(ok) && cJSON_IsArray(data) && cJSON_GetArraySize(data) > 0) { // ... update s_last_seen_date, seed dedup ... } s_first_poll = false; xSemaphoreGive(s_cred_lock);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/channels/imessage/imessage_bot.c` around lines 567 - 583, The lock acquisition is duplicated in both branches; simplify by taking s_cred_lock once before the if/else that checks the response and releasing it after, so both the success path (where you may read sa, set s_last_seen_date, seed the dedup cache using newest -> id with seen_msg_insert(fnv1a64(...))) and the fallback path always execute under the same lock and you set s_first_poll = false once; locate the block around s_cred_lock, s_first_poll, s_last_seen_date, newest, seen_msg_insert and refactor to a single xSemaphoreTake(...) before the conditional and a single xSemaphoreGive(...) after.
767-774: Consider softer error handling for NVS operations.
ESP_ERROR_CHECKwill abort the device on NVS failure (e.g., partition full, flash errors). For a runtime credential update triggered via CLI, this may be unexpectedly harsh. Consider returning an error code instead and letting the caller decide.♻️ Suggested graceful handling
nvs_handle_t nvs; - ESP_ERROR_CHECK(nvs_open(MIMI_NVS_IMSG, NVS_READWRITE, &nvs)); - ESP_ERROR_CHECK(nvs_set_str(nvs, MIMI_NVS_KEY_IMSG_URL, server_url)); - ESP_ERROR_CHECK(nvs_set_str(nvs, MIMI_NVS_KEY_IMSG_API_KEY, api_key)); + esp_err_t err = nvs_open(MIMI_NVS_IMSG, NVS_READWRITE, &nvs); + if (err != ESP_OK) { + ESP_LOGE(TAG, "Failed to open NVS: %s", esp_err_to_name(err)); + return err; + } + err = nvs_set_str(nvs, MIMI_NVS_KEY_IMSG_URL, server_url); + if (err != ESP_OK) { + ESP_LOGE(TAG, "Failed to save server URL: %s", esp_err_to_name(err)); + nvs_close(nvs); + return err; + } + // ... similar for other nvs_set_str calls ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/channels/imessage/imessage_bot.c` around lines 767 - 774, Replace the hard aborts from ESP_ERROR_CHECK in the credential store path with graceful error handling: check return values from nvs_open, nvs_set_str (for MIMI_NVS_KEY_IMSG_URL, MIMI_NVS_KEY_IMSG_API_KEY, and optional MIMI_NVS_KEY_IMSG_PROXY_URL), and nvs_commit, log or return the esp_err_t to the caller instead of calling ESP_ERROR_CHECK, ensure nvs_close(nvs) is always called when nvs_open succeeded (even on errors), and propagate a meaningful error code/string so the CLI caller can decide how to handle failures rather than aborting the device.
321-337: Host header should include port for non-standard ports.Per HTTP/1.1 (RFC 7230), the
Hostheader should include the port when it differs from the scheme's default. Currently,Host: %suses onlylocal_proxy_hostwithout the port, which may cause issues with proxies listening on non-standard ports.♻️ Suggested fix
+ /* Build Host header value (include port if non-default) */ + char host_header_val[160]; + bool is_default_port = (local_port == 443); /* Assuming HTTPS */ + if (is_default_port) { + snprintf(host_header_val, sizeof(host_header_val), "%s", host); + } else { + snprintf(host_header_val, sizeof(host_header_val), "%s:%d", host, local_port); + } + if (post_data) { header_len = snprintf(header, sizeof(header), "%s %s HTTP/1.1\r\n" - "Host: %s\r\n" + "Host: %s\r\n" "Authorization: %s\r\n" "Content-Type: application/json\r\n" "Content-Length: %d\r\n" "Connection: close\r\n\r\n", - method_str, req_path, host, auth_header, (int)strlen(post_data)); + method_str, req_path, host_header_val, auth_header, (int)strlen(post_data));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/channels/imessage/imessage_bot.c` around lines 321 - 337, The Host header currently uses just the host string (variable host) and must include the port when it differs from the scheme default; update the request header construction to compute a host_with_port (e.g., build a string that appends ":<port>" when port != 80 for http or != 443 for https / when using your proxy port variable) and use that host_with_port in the snprintf calls instead of host so both the post_data and non-post branches include the correct Host: value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main/channels/imessage/imessage_bot.c`:
- Around line 175-194: The OOM path inside http_event_handler currently returns
ESP_ERR_NO_MEM which neither logs nor prevents esp_http_client_perform() from
continuing; modify http_event_handler (and the http_resp_t struct) to set a
persistent error flag (e.g., add a bool or int field like resp->oom or
resp->error_code) and log the allocation failure before returning, and update
the caller that invokes esp_http_client_perform() to check resp->oom (or
resp->error_code) after perform returns and abort/cleanup appropriately; ensure
all branches that detect realloc failure set the flag so subsequent ON_DATA
calls don't assume resp->buf is valid and avoid further memcpy into NULL.
- Around line 115-124: The FNV-1a implementation in fnv1a64 uses the wrong
64-bit offset basis constant; update the initial hash value in fnv1a64 from
1469598103934665603ULL to the correct FNV-1a 64-bit offset basis
14695981039346656037ULL so the function conforms to the standard algorithm (keep
the rest of the loop and multiplication with 1099511628211ULL unchanged).
---
Duplicate comments:
In `@main/channels/imessage/imessage_bot.c`:
- Around line 696-712: The loop slices the UTF-8 string at raw byte boundaries
(using MIMI_IMSG_MAX_MSG_LEN) which can split multi-byte codepoints; modify the
chunk calculation before allocating segment so the chunk end is moved backwards
to a UTF-8 codepoint boundary by scanning bytes from (offset + chunk - 1) down
while the byte is a UTF-8 continuation byte (0x80..0xBF), but never reduce chunk
below 1; update chunk accordingly, then allocate segment (malloc(chunk+1)),
memcpy from text+offset for the adjusted chunk, and continue updating offset;
keep existing JSON building (cJSON_CreateObject, cJSON_AddStringToObject)
unchanged.
---
Nitpick comments:
In `@main/channels/imessage/imessage_bot.c`:
- Around line 567-583: The lock acquisition is duplicated in both branches;
simplify by taking s_cred_lock once before the if/else that checks the response
and releasing it after, so both the success path (where you may read sa, set
s_last_seen_date, seed the dedup cache using newest -> id with
seen_msg_insert(fnv1a64(...))) and the fallback path always execute under the
same lock and you set s_first_poll = false once; locate the block around
s_cred_lock, s_first_poll, s_last_seen_date, newest, seen_msg_insert and
refactor to a single xSemaphoreTake(...) before the conditional and a single
xSemaphoreGive(...) after.
- Around line 767-774: Replace the hard aborts from ESP_ERROR_CHECK in the
credential store path with graceful error handling: check return values from
nvs_open, nvs_set_str (for MIMI_NVS_KEY_IMSG_URL, MIMI_NVS_KEY_IMSG_API_KEY, and
optional MIMI_NVS_KEY_IMSG_PROXY_URL), and nvs_commit, log or return the
esp_err_t to the caller instead of calling ESP_ERROR_CHECK, ensure
nvs_close(nvs) is always called when nvs_open succeeded (even on errors), and
propagate a meaningful error code/string so the CLI caller can decide how to
handle failures rather than aborting the device.
- Around line 321-337: The Host header currently uses just the host string
(variable host) and must include the port when it differs from the scheme
default; update the request header construction to compute a host_with_port
(e.g., build a string that appends ":<port>" when port != 80 for http or != 443
for https / when using your proxy port variable) and use that host_with_port in
the snprintf calls instead of host so both the post_data and non-post branches
include the correct Host: value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b62541bf-b141-4d09-bd52-294d724063c7
📒 Files selected for processing (1)
main/channels/imessage/imessage_bot.c
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
20-20: Tighten the opening sentence.
evolves over time with local memoryreads redundant here.learns with local memoryorevolves with local memoryis cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 20, The opening sentence in README.md contains a redundant phrase "evolves over time with local memory"; replace that fragment in the sentence "MimiClaw turns a tiny ESP32-S3 board into a personal AI assistant. Plug it into USB power, connect to WiFi, and talk to it through Telegram or iMessage — it handles any task you throw at it and evolves over time with local memory — all on a chip the size of a thumb." with a tighter option such as "learns with local memory" or "evolves with local memory" to make the sentence more concise and clear.
🤖 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`:
- Around line 44-45: The README and the mimi_secrets.h snippet are missing the
build-time iMessage proxy variable; update the docs to document
MIMI_SECRET_IMSG_PROXY_URL (the build-time iMessage proxy override) so
self-hosted users can discover it—reference the existing declarations in
mimi_config.h and uses in main/channels/imessage/imessage_bot.c when adding the
line to the README and the mimi_secrets.h example and also add the same note to
the other README locations mentioned (lines ~137-138 and ~181-182).
---
Nitpick comments:
In `@README.md`:
- Line 20: The opening sentence in README.md contains a redundant phrase
"evolves over time with local memory"; replace that fragment in the sentence
"MimiClaw turns a tiny ESP32-S3 board into a personal AI assistant. Plug it into
USB power, connect to WiFi, and talk to it through Telegram or iMessage — it
handles any task you throw at it and evolves over time with local memory — all
on a chip the size of a thumb." with a tighter option such as "learns with local
memory" or "evolves with local memory" to make the sentence more concise and
clear.
Member
|
Thanks! I will test it! |
IRONICBo
added
enhancement
Member
|
Hi, I have met some issues.
|
Member
BTW, Could you move your docs to |
Author
Hi! Thanks for your review. I've checked the server log - this is caused by Apple's spam detection mechanism, if it's our server who initiated the conversation then the message would be filtered as spam. It must be the user who sends the first message. Do you think we should add a check for imsg_send regarding this limitation? If no prior message has been received from this user, the bot cannot send them an outbound message? |
Member
Yes, I think it is better to understand. Just to clarify the operational flow: for a cold start, which address/ID should the first message be sent to? |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
README.md (1)
44-45:⚠️ Potential issue | 🟡 MinorDocument
MIMI_SECRET_IMSG_PROXY_URLhere too.The runtime CLI example already exposes a custom proxy, but the prerequisites text and
mimi_secrets.hsnippet still omit the build-timeMIMI_SECRET_IMSG_PROXY_URLoverride defined inmain/mimi_secrets.h.exampleandmain/mimi_config.h. That leaves self-hosted proxy users without a documented non-CLI setup path.Also applies to: 137-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 44 - 45, Add documentation for the build-time override MIMI_SECRET_IMSG_PROXY_URL: update the prerequisites section and the mimi_secrets.h snippet to mention the MIMI_SECRET_IMSG_PROXY_URL macro (used as a build-time override for the iMessage proxy), show the equivalent entry that appears in main/mimi_secrets.h.example, and explain how to set it in main/mimi_config.h or via build system defines as an alternative to the runtime CLI proxy flag so self-hosted proxy users have a non-CLI setup path.
🤖 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`:
- Around line 181-182: Update the CLI README section to include an example usage
for the new imsg_send command showing accepted recipient formats; add one-line
examples demonstrating sending to a phone number with a leading plus (imsg_send
+1234567890), to an email address (imsg_send user@example.com), and to an
iMessage group using the group:id form (imsg_send group:abcdef123456), and
mention that credentials are set via set_imsg_creds so readers can test the flow
end-to-end.
- Line 20: Update the README to clearly document the iMessage cold-start
limitation: state that server-initiated outbound iMessage is blocked until the
recipient has previously messaged the bot, describe the exact first-contact flow
a new user must follow (recipient sends any iMessage to the device/bot first,
then subsequent messages from imsg_send will be delivered), and note that
imsg_send may report success even when the initial delivery is filtered; apply
this clarification to the existing lines around the product blurb and the other
referenced README sections (the sentences at lines 20, 34 and 284) so all
mentions of iMessage delivery include the prerequisite and expected behavior on
first contact.
---
Duplicate comments:
In `@README.md`:
- Around line 44-45: Add documentation for the build-time override
MIMI_SECRET_IMSG_PROXY_URL: update the prerequisites section and the
mimi_secrets.h snippet to mention the MIMI_SECRET_IMSG_PROXY_URL macro (used as
a build-time override for the iMessage proxy), show the equivalent entry that
appears in main/mimi_secrets.h.example, and explain how to set it in
main/mimi_config.h or via build system defines as an alternative to the runtime
CLI proxy flag so self-hosted proxy users have a non-CLI setup path.
IRONICBo
approved these changes
Mar 8, 2026
Member
IRONICBo
left a comment
IRONICBo
left a comment
There was a problem hiding this comment.
LGTM, could you squash your commit?
Author
Done! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds iMessage channel support through Photon's advanced-imessage-kit. Since the server requires a long-running websocket connection which would be too expensive for an ESP32, requests are polled through our rest api proxy.
Summary by CodeRabbit
New Features
CLI
Documentation