[steipete]

Fixes #381.

Prefer the registry operation that moves a chat’s messages into Recently Deleted, using the requested chat, a deletion date, and synchronous completion. Keep older selectors only when this operation is unavailable; exceptions never trigger permanent-removal fallback. Advertise the capability for RPC dispatch.

Validation: synthetic native regression fails on main and passes after the fix, covering method selection, target, synchronous arguments, exceptions, and legacy paths. RPC tests cover capability advertisement and dispatch for each supported selector. Independent P0–P2 review is clean. No live Messages sends or deletions were used.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T06:48:49.285198Z	c61e848	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

Prefer Recently Deleted for chat deletion, retain legacy selectors when unavailable, and advertise the new capability to RPC callers.

Example: Delete a disposable conversation on macOS 26.5.1 using chat-delete.

• Before: v0.20.0 selects _chat_remove:, erasing messages without recovery rows.
• After: The helper selects the Recently Deleted operation when available and never retries permanent removal after an exception.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Focused, source-consistent repair with useful regression coverage and supporting live reporter evidence, but no shipped-entry-point demonstration of the exact patch; no schema or serialized-format migration is introduced.
Proof confidence	🦐 gold shrimp (3/6)	Ordinary contributor proof gating is waived for this collaborator-authored PR; native and RPC harness coverage supports dispatch correctness, while the linked reporter's real Messages comparison supports the selector's recovery behavior without proving this exact head.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Deleting a conversation through imsg can erase its messages without the recovery available when deleting in Messages.
Reason: An affected-release comparison establishes concrete data loss, and collaborator authorship approves the scoped policy of preferring recovery while preserving older selectors.

Merge readiness

✅ Ready for maintainer review

Keep open: this maintainer-authored PR addresses demonstrated loss of recoverability that remains on main, and inspection found no introduced correctness defect.

Priority: P0
Reviewed head: c61e848fd6c69d0bd47efee995070d24d8df75af

Before merge

None.

Findings

None.

Tests

• Missing end-to-end proof: No supplied run of this exact PR through the shipped CLI or RPC demonstrates deletion and restoration in real Messages; the linked reporter comparison exercises the proposed selector on another implementation.

Agent review details

How this fits together

CLI and JSON-RPC chat deletion requests converge on the injected Messages helper, which resolves the requested chat and invokes IMChatRegistry.

flowchart LR
  A[CLI chat-delete] --> C[Bridge request]
  B[RPC chats.delete] --> C
  C --> D[Shared deletion handler]
  D --> E[Recently Deleted operation]
  D --> F[Legacy operation when unavailable]
  E --> G[Messages conversation storage]
  F --> G

Loading

Technical review

Best possible solution:

Land the focused shared-handler repair with matching RPC capability reporting and the explicitly accepted legacy compatibility behavior.

Do we have a high-confidence way to reproduce the issue?

Main demonstrably routes deletion through the reported legacy selector, and thurstonsand supplies a concrete v0.20.0 comparison on macOS 26.5.1; this review executed no deletion.

Is this the best way to solve the issue?

The patch changes the existing shared owner and its capability registry without adding a parallel deletion path, while preserving the compatibility policy chosen by its maintainer author.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 5dc47b8ee7b6.

Provenance checked

• Chat deletion selector selection changes intended behavior with a stated reason (fix(bridge): restore chat deletion on macOS 26 #149: Restore chat deletion on macOS 26 after deleteChat: disappeared; the later feature-module split preserved this behavior.)
• RPC deletion capability gating keeps the original intent (b0165e4: Make the descriptor registry authoritative for discovery, capability reporting, request lanes, and dispatch.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Current main still uses legacy deletion: The base handler selects deleteChat: or _chat_remove: and has no Recently Deleted operation; both public entry points dispatch to this shared handler. (Sources/IMsgHelper/ChatHandlers.inc:246, 5dc47b8ee7b6)
• Affected-release comparison: thurstonsand reports stock v0.20.0 permanently erasing disposable-chat messages on macOS 26.5.1, while the proposed selector creates recovery rows and reaches iPhone Recently Deleted: chats.delete erases chats outright instead of moving them to Recently Deleted #381 (comment).
• Narrow shared-owner implementation: The introduced diff prefers the recoverable selector, supplies only the resolved chat with a date and synchronous execution, and returns directly on exceptions; helper status and RPC capability gating agree. (Sources/IMsgHelper/ChatHandlers.inc:253, c61e848fd6c6)
• Legacy compatibility is an accepted direction: The collaborator-authored PR explicitly retains legacy selectors when recoverable deletion is unavailable, and documents their possible permanent removal; this settles that choice for this PR despite the reporter's later preference for recoverable-only refusal. (docs/bridge.md:180, c61e848fd6c6)
• Original fallback rationale: fix(bridge): restore chat deletion on macOS 26 #149 introduced _chat_remove: to restore unavailable deletion on macOS 26; its proof established selector availability rather than recoverability. Local historical blob searches encountered unavailable promisor objects, so REST patch evidence supplied the original hunk. (Sources/IMsgHelper/IMsgInjected.m, f884aef31877)
• Regression coverage and check state: Native tests exercise the real handler against a synthetic registry, including exceptions without permanent fallback; RPC tests exercise each capability. The macOS and Linux checks passed, while Swift analysis remained in progress. (Tests/IMsgHelperTests/ChatDeletionTests.m:54, c61e848fd6c6)

Likely related people:

• Peter Steinberger: Raw commit b0165e4 adds Sources/imsg/RPCMethodDescriptors.swift:125 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: b0165e455f66; files: Sources/imsg/RPCMethodDescriptors.swift)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Labels

Label changes:

• add P0: The linked affected-release comparison demonstrates irreversible message loss where normal Messages deletion preserves recovery.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P0: The linked affected-release comparison demonstrates irreversible message loss where normal Messages deletion preserves recovery.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Strengthen confidence with a redacted disposable-chat CLI or RPC trace showing Recently Deleted placement and successful restoration on the affected macOS release.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed November 5, 2026, 2:00 AM ET / 07:00 UTC.