[SebTardif]

Rich-link requests with caller-owned GUIDs, unsupported send modifiers, or a live non-iMessage service are refused before message construction. Return not_started so callers can retry these refusals safely.

Validated with the compiled synthetic helper fixture: all three cases fail on main with a missing disposition and pass with this change. No Messages injection or sends were performed. Independent Codex review through P2: scoped-clean. Full exact-head CI is required before merge.

Thanks @SebTardif for the fix and fixture.

[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.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

Mark three rich-link refusals before dispatch as not_started and add native regression coverage.

Example: A send-rich-link request includes effectId=com.apple.messages.effect.Echo.

• Before: The helper refuses it without a delivery disposition, so the client reports may_have_completed and retry_safe=false.
• After: The refusal includes delivery_disposition=not_started, so retry_safe=true.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A useful, focused correction supported by production IPC output and regression coverage, with no actionable defects.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The updated macOS terminal trace exercises the production v2 filesystem inbox, rich-link modifier refusal, response envelope, and production client classification types, yielding not_started and retry_safe=true with the claim removed; native tests separately cover all three branches. This closes the prior production-boundary proof gap without requiring a live send for a refusal-only change.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Callers are warned that retrying is unsafe even when an unsupported rich-link request was rejected before dispatch.
Reason: This restores the existing transport-owned delivery contract without changing supported requests, defaults, or public configuration.

Merge readiness

✅ Ready for maintainer review

This PR fixes a current delivery-classification bug with a focused patch and no actionable findings.

Priority: P2
Reviewed head: da1ba8b0ea6658a7ee592851abbca6f002abb741

Before merge

None.

Findings

None.

Agent review details

How this fits together

The injected helper consumes bridge requests, rejects unsupported rich-link combinations, and returns delivery facts that the Swift client uses to determine retry safety.

flowchart LR
A[Bridge request] --> B[V2 filesystem inbox]
B --> C[Action router]
C --> D[Rich-link validation]
D --> E[Response envelope]
E --> F[Client delivery classification]

Loading

Technical review

Best possible solution:

Use the existing disposition helper for these three proven pre-dispatch refusals while preserving all validation and uncertain-delivery handling.

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

Current-main source deterministically omits the disposition on these refusals, and the client consequently classifies them as may_have_completed; supplied macOS output corroborates the path.

Is this the best way to solve the issue?

The patch uses the existing authoritative response helper at the three refusal sites and leaves later failures unchanged, making it the smallest complete fix for the stated scope.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 1bc0679663bf.

Provenance checked

• Rich-link modifier and service refusals keeps the original intent (feat: add bridge rich links #165: Strict validation keeps URL previews limited to supported modifiers and iMessage services; the later module split preserved behavior.)
• Delivery disposition and retry safety keeps the original intent (93a644b: Only the transport can authorize retries after proving dispatch did not occur.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Introduced patch and current-main defect: The pinned diff changes only three early error returns in production. Current main uses errorResponse on all three paths, before message construction begins. (Sources/IMsgHelper/SendMessage.inc:59, da1ba8b0ea66)
• Client consequence: IMsgBridgeClient classifies mutation failures without a disposition as mayHaveCompleted; DeliveryFailure permits retries only for notStarted. (Sources/IMsgCore/IMsgBridgeClient.swift:375, 1bc0679663bf)
• Production response boundary: The filesystem inbox claims requests, dispatches through processV2Envelope, writes the outbox, and removes the claim; the router preserves delivery_disposition in failed responses. (Sources/IMsgHelper/Inbox.inc:46, da1ba8b0ea66)
• Updated behavior proof: The complete PR body reports a patched macOS helper processing an effect-bearing request through the real v2 filesystem inbox without substitute classes, producing not_started in the outbox; production BridgeResponse and DeliveryFailure types report retry_safe=true. The author explicitly distinguishes this from Messages injection and live sending. (da1ba8b0ea66)
• Regression coverage: The new native test exercises the actual handler for caller-owned GUID, send-modifier, and live-SMS service refusals. The existing Makefile discovers *Tests.m automatically; the body supplies failing-base and passing-head output. (Tests/IMsgHelperTests/RichLinkRefusalTests.m:89, da1ba8b0ea66)
• Original delivery contract: fix: prevent retries after uncertain delivery #216 established transport-owned retry dispositions to prevent duplicate or downgraded sends after uncertain delivery; this PR preserves that conservative contract and labels only proven refusals. (Sources/IMsgCore/DeliveryFailure.swift:44, 93a644b5e893)

Likely related people:

• Peter Steinberger: Raw commit 93a644b adds Sources/IMsgCore/DeliveryFailure.swift:44 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 93a644b5e893; files: Sources/IMsgCore/DeliveryFailure.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 proof: sufficient: Contributor real behavior proof is sufficient.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
• remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
• remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

• P2: Incorrect retry classification affects three rejected rich-link request paths without evidence of dispatched or lost messages.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
• proof: sufficient: Contributor real behavior proof is sufficient.

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.

History

Review history (1 earlier review cycle)

• reviewed 2026-11-05T23:11:57.850Z sha da1ba8b :: needs real behavior proof before merge. :: none

Reviewed November 6, 2026, 11:54 AM ET / 16:54 UTC (Revision 2).

[SebTardif]

@clawsweeper re-review

A v2 inbox scan of the helper source refused a send-rich-link effect before building a message. The production client types report not_started and retry_safe true.

[clawsweeper]

🦞👀
Exact review queued.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/42256918828
• Updated: 2026-11-06T16:56:11.937Z

[steipete]

Reviewed code and fixture proof; exact-head CI is green.