[niXta1]

AppleScript number-to-text conversion can use U+2212 for a negative sign under locales such as sv_SE. Format error numbers with an explicit ASCII minus while preserving delivery dispositions and retry safety.

Added a fixture that executes the production formatter for negative, zero, and positive numbers before entering the Messages block. The formatter is absent on main; all four cases pass with this change. No messages, account changes, or locale changes were made. Independent Codex review through P2: scoped-clean. Exact-head CI is required before merge.

Thanks @niXta1 for the fix and locale-specific diagnosis.

[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: ✅ Completed

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-14T06:24:34.171982Z	c200bf2	New commits

ℹ️ 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: blocked before merge.

What this changes

This PR formats negative AppleScript send-error numbers with an ASCII minus.

Example: On an sv_SE Mac, send to nonexistent chat GUID iMessage;+;chat000000000000000000000.

• Before: The CLI reports AppleScript error −1728.
• After: The CLI reports AppleScript error -1728, retaining not_started disposition and exit code 1.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	Focused, correct repair with relevant shipped-CLI proof and existing regression coverage; automated validation remains approval-gated.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The contributor's terminal output exercises MessageSender through the shipped send CLI on an sv_SE Mac using a nonexistent chat GUID, showing the reviewed head emit ASCII -1728 with unchanged pre-dispatch disposition and exit status; this satisfies the previous review's requested scenario.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Negative AppleScript send-error numbers use a Unicode minus on Swedish-region Macs, producing inconsistent diagnostics and failing existing exact-result tests.
Reason: The small change restores the existing ASCII result expectation without adding options or altering delivery behavior.

Merge readiness

⛔ Blocked before merge - 1 item remains

This PR fixes a concrete locale-dependent diagnostic defect still present on main, with no introduced correctness finding; CI and CodeQL await workflow approval.

Priority: P2
Reviewed head: e72333bf97d0ae32e2f3e517eece40c0cd22544b

Before merge

• Await maintainer action - Approve the waiting CI and CodeQL workflow runs for the reviewed head.

Findings

None.

Agent review details

How this fits together

The send CLI passes destination and message arguments to MessageSender's AppleScript; AppleScriptSendTransport converts its structured result into a delivery failure displayed to callers.

flowchart LR
  A[Send CLI] --> B[MessageSender]
  B --> C[AppleScript and Messages]
  C --> D[Structured delivery result]
  D --> E[AppleScriptSendTransport]
  E --> F[CLI error and retry guidance]

Loading

Technical review

Best possible solution:

Land the focused formatter fix after the existing CI and CodeQL workflows complete successfully.

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

The main-source coercion path and contributor's pinned v0.21.0 sv_SE CLI transcript establish the defect; no target code was executed during this read-only review.

Is this the best way to solve the issue?

The shared formatter fixes both failure branches at the producer while preserving the existing result contract and transport-owned delivery decisions.

AGENTS.md: found and applied where relevant.

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

Provenance checked

• AppleScript structured send failures keeps the original intent (93a644b: Merged fix: prevent retries after uncertain delivery #216 made dispatch disposition authoritative to prevent unsafe retries and fallback after uncertain delivery.)
• Unreleased changelog keeps the original intent (1bc0679: Merged chore: prepare imsg 0.21.0 release #386 retained the post-release Unreleased section; this PR adds its focused fix entry there.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Verified introduced change: Both failure branches use the same small formatter; delivery phases, dispatch operations, and fallback eligibility remain unchanged. (Sources/IMsgCore/MessageSender.swift:281, e72333bf97d0)
• Current-main defect: Main still concatenates errorNumber directly into both failure results, retaining locale-dependent number-to-text coercion. (Sources/IMsgCore/MessageSender.swift:281, 1bc0679663bf)
• Error consumer preserves disposition: The transport reads disposition from field 3 and interpolates the number field into diagnostic text; retry safety does not depend on parsing the error number. (Sources/IMsgCore/AppleScriptSendTransport.swift:97, 1bc0679663bf)
• Contributor CLI proof resolves prior request: The complete PR body supplies before/after shipped-CLI output on macOS 26.6.2 with sv_SE, pinned to v0.21.0 and the reviewed head, showing U+2212 replaced by byte 2d while disposition and exit status remain unchanged; niXta1 identifies the added proof in fix(send): report AppleScript error numbers with an ASCII minus #395 (comment). (e72333bf97d0)
• Existing regression coverage: Existing production-script tests expect ASCII negative numbers for lookup and dispatch failures, covering both dispositions; the contributor reports these failing before and passing after on sv_SE. (Tests/IMsgCoreTests/MessageSenderDirectParticipantTests.swift:114, 1bc0679663bf)
• Feature provenance and owner: History traces the structured result to merged fix: prevent retries after uncertain delivery #216 and direct-participant recovery to fix: recover stale direct chat targets before dispatch #246; both preserve authoritative dispatch facts rather than infer retry safety from diagnostics. The follow-history search reached an unavailable creation-era object; the relevant later commits and base blobs were readable. (Sources/IMsgCore/MessageSender.swift:279, 93a644b5e893)

Likely related people:

• Peter Steinberger: Raw commit 93a644b adds Sources/IMsgCore/MessageSender.swift:231 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/MessageSender.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 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• add status: ⏳ waiting on maintainer: Only explicit maintainer approvals or decisions remain; merge is still blocked.
• remove status: 📣 needs proof: Current PR status label is status: ⏳ waiting on maintainer.
• remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

• P2: Locale-dependent error formatting breaks deterministic diagnostics and existing tests without changing message dispatch or retry safety.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: ⏳ waiting on maintainer: Only explicit maintainer approvals or decisions remain; merge is still blocked.
• 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-08T13:43:13.825Z sha e72333b :: needs real behavior proof before merge. :: none

Reviewed November 11, 2026, 6:12 PM ET / 23:12 UTC (Revision 2).

[niXta1]

Added real behavior proof to the PR body: the shipped CLI on an sv_SE Mac before (v0.21.0, U+2212) and after (e72333b, ASCII -) for a pre-dispatch send failure, plus full make lint (SwiftLint 0.65.1) and make test output. CI and CodeQL for this PR are waiting for workflow approval.

@clawsweeper re-review

[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/43118245495
• Updated: 2026-11-11T23:14:36.904Z

[steipete]

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