Skip to content

cmux-imessage: review follow-ups (date-ordered previews, NULL rows, stderr drain, terminal error code) - #33219

Merged
teamleaderleo merged 5 commits into
feat-cmux-nextfrom
fix/imessage-crate-followups
Nov 3, 2026
Merged

teamleaderleo merged 5 commits into
feat-cmux-nextfrom
fix/imessage-crate-followups

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes the review follow-ups of the Messages mirror reader and sender: #32896 (from the review of #32891) and #32923 (from the review of #32919). The changes stay inside cmux-tui/crates/cmux-imessage, and its public API is unchanged.

What changes

  • Thread order and preview: a thread's preview is now its newest visible message by date, with rowid breaking ties, and threads sort by that date. iCloud backfill gives old messages high rowids, so the old highest-rowid pick could show a stale preview.
  • NULL columns: rows with NULL item_type or associated_message_type (older databases) count as plain messages instead of disappearing.
  • since(): a message joined to more than one chat (an iMessage/SMS pair) comes back once.
  • Send, stderr: SystemRunner drains stderr on a thread while the child runs. A failure that writes more than the pipe buffer no longer stalls until the two-minute timeout. Stop condition: the thread ends when the pipe closes, after the child exits or is killed and reaped.
  • Send, error mapping: only the terminal AppleScript code (... (-1743)) decides it, so a code quoted inside a chat id can't turn invalid_chat into not_permitted.

Left open in #32896, as design changes rather than fixes: paging messages by date instead of rowid, showing stickers and app messages, and live updates for edits and unsends.

Tests

Two commits: the failing tests first (the reader's 3 new tests red on Linux CI; the send tests are red by construction against the old code), then the fixes. Green: 13 reader and 7 send tests, clippy -D warnings clean, rustfmt clean. The stderr test runs a real sh child that writes 300 KB to stderr.

Agent session: agent session

🤖 Generated with Claude Code


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes review follow-ups from #32896 and #32923 in the Messages mirror reader and sender. The public API is unchanged.

Reader

  • Thread previews and ordering use the newest visible message by date (rowid breaks ties), since iCloud backfill gives old messages high rowids.
  • NULL item_type and associated_message_type columns now count as plain messages instead of disappearing.
  • since() returns a message joined to more than one chat once.

Sender

  • SystemRunner drains stderr on a thread while the child runs, so a failure writing past the pipe buffer no longer stalls until the two-minute timeout.
  • Error mapping reads only the terminal AppleScript code, so a code quoted inside a chat id can't turn invalid_chat into not_permitted.
  • The error-code parse avoids string slicing, keeping the crate panic-free for the new panic ratchet.

Written for commit 8a80e42. Summary will update on new commits.

View guided diff Turn on auto-fix

teamleaderleo and others added 2 commits November 2, 2026 22:55
…32923

- a thread's preview is its newest message by date, not by rowid (iCloud backfill)
- rows with NULL item_type / associated_message_type count as visible
- since() returns a message joined to two chats once
- the terminal AppleScript error code decides, not one quoted in the message
- SystemRunner reads stderr past the pipe buffer without stalling

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-f62148d989879feda740
Reader: a thread's preview and order come from its newest visible message
by date (rowid breaks ties), NULL item_type / associated_message_type count
as a plain message, and since() returns a message joined to several chats
once. Send: SystemRunner drains stderr on a thread while the child runs, so a
noisy failure no longer stalls into the two-minute timeout, and the error
mapping reads only the terminal AppleScript code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-f62148d989879feda740
@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 8a80e42: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr33219 -f ref=fix/imessage-crate-followups -f platform=macos -f backend_mode=local

Shipped in:

@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@blacksmith-sh

blacksmith-sh Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
nightly preparation must run on Blacksmith Linux, keep its fallback, and reuse a succes
sful artifact/
nightly preparation must run on Blacksmith Linux, keep its fallback, and reuse a succes
sful artifact
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex, router-review): SHIP at bc62ef1

Reviewer: codex gpt-6.1-sol xhigh, fast tier.

Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward.

SHIP at bc62ef1
Visual verdict: no images; Leo tries the build

  1. [P2] [follow-up] send.rs:144: Bound stderr collection on timeout. A descendant can inherit stderr and keep the pipe open after the direct child is killed and reaped, so collect(reader) blocks beyond SEND_TIMEOUT. Make draining cancellable or bound the join, and test descendant-held stderr.

SQLite probes passed; a process probe confirmed the inherited-pipe behavior. Rust tests were unavailable because this box has no Rust toolchain.

Follow-ups:

  • Bound stderr collection when descendants keep the pipe open.

cmux-bot[bot]
cmux-bot Bot previously approved these changes Nov 3, 2026

@cmux-bot cmux-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmux-bot approves bc62ef1: an independent review passed this exact head (router-review receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@teamleaderleo teamleaderleo added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 3, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 07:54 Inactive
@cmux-bot
cmux-bot Bot dismissed their stale review November 3, 2026 08:01

A newer verdict blocks 6c726d8 (no independent SHIP at the head).

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Red triage: the clippy panic ratchet red is this PR's own: cmux-imessage: clippy::string_slice 2 > new crate (must be 0) at crates/cmux-imessage/src/send.rs:208 and :213, the two string slices this PR adds.

@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 3, 2026 08:49 Inactive
The panic ratchet starts a new crate panic-free; terminal_error_code sliced
&str with [..] (string_slice). It now uses str::get, which cannot panic.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-9bf3963ac29d9d619047
@teamleaderleo
teamleaderleo merged commit 0d430df into feat-cmux-next Nov 3, 2026
154 of 157 checks passed
@teamleaderleo
teamleaderleo deleted the fix/imessage-crate-followups branch November 3, 2026 09:38
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex, router-review): SHIP at 8a80e42

Reviewer: codex gpt-6.1-sol xhigh, fast tier.

Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward.

SHIP at 8a80e42
Visual verdict: no images; Leo tries the build

  1. [P2] [follow-up] send.rs:144: Bound stderr collection on timeout. With sh -c 'sleep 600 & wait', killing and reaping the shell at 120 seconds leaves its descendant holding stderr open, so collect(reader) waits until 600 seconds. The base timeout path returned after reaping. Make the drain cancellable and add an inherited-pipe timeout test.

Follow-ups:

  • Bound stderr collection when descendants retain the pipe.

This branch was previously deployed

3 inactive deployments
artifacts — 8a80e42b Deployed Nov 3, 2026 by teamleaderleo via publish tree-addressed cmux-tui #105177
sentry-dsyms — 8a80e42b Deployed Nov 3, 2026 by teamleaderleo via upload cmux-tui dSYMs to Sentry #105177
ci-routing-agent — 8a80e42b Deployed Nov 3, 2026 by teamleaderleo via cmux-tui tree preflight #105177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant