Merge pull request #10 from docwho76/v5.2.0

add test suite, deal with commas

CHANGELOG.md

@@ -2,6 +2,49 @@
 
 All notable changes to `sendmsg` are documented here.
 
+## 5.2.0
+
+### Added
+- **`--list-groups`:** Lists the Signal groups the account belongs to, with their names, IDs, member counts, and blocked status (via `GET /v1/groups/{number}`). This is the sanctioned way to discover the group IDs that `--recipients` and CSV rows expect. Errors out clearly when no real account is configured.
+- **Test suite and CI:** A pytest suite (`tests/`) covering phone/group detection, path expansion, attachment validation, Signal payload construction, CSV dispatch (BOM, blank rows, skips, dry run, `--json`), and CLI argument validation, plus a GitHub Actions workflow running it on Python 3.10–3.13.
+- **Commas in CSV messages:** Quoted message fields (`"Hi, there"`) are parsed per standard CSV rules as before, and messages with *unquoted* commas are now repaired automatically — the extra fragments are merged back into the `message` column (columns before and after it are preserved) with a warning recommending quoting. Previously an unquoted comma silently shifted every subsequent column, leaking message text into `account`, `service`, etc. Rows with fewer fields than the header are also tolerated (missing trailing columns are treated as empty).
+- **Dry-run validation:** `--dry-run` now flags rows with missing recipients and verifies that `file`/`voice` paths exist (reported as `[NOT FOUND]` and listed in the summary notes), so problems surface before a real run.
+
+### Fixed
+- **Voice messages: removed the non-existent `voice` API flag.** signal-cli-rest-api's `/v2/send` has no voice-note field; the flag sent since 5.0.0 was silently ignored and the audio was always delivered as a regular audio attachment. The payload and documentation now reflect reality. (`--voice` still works — the audio is sent as its own message — but whether it renders as a playable voice note depends on the receiving client.)
+- **UTF-8 BOM no longer misroutes CSV rows.** CSVs are read with `utf-8-sig`. Previously a BOM (written by Excel and many editors) corrupted the first header to `\ufeffmethod`, making every row's method appear empty — which defaulted to `signal` and silently sent explicit `sms` rows via Signal.
+- **Formatted phone numbers are no longer misdetected as group IDs.** Spaces, dashes, dots, and parentheses are stripped before phone-vs-group detection and before sending, so `+1 (800) 555-1212` and `555-867-5309` are routed as direct messages in E.164 form instead of down the group path.
+- **`--delay abc` no longer crashes.** The global `--delay` is parsed by argparse as a float, producing a clean usage error instead of a `ValueError` traceback.
+- **`--json` output is machine-parseable.** Per-row progress lines now go to stderr when `--json` is set, so stdout contains only the JSON summary and can be piped to `jq` etc. The summary also gains a `blank` count.
+- **`--sms` sends to every `--to` recipient.** Previously only the first was sent and the rest were silently dropped; the SMS path now fans out like the Signal path and recipients are `+`-normalized.
+- **Rows without a recipient are skipped and reported** instead of being POSTed with an empty recipient and failing with a confusing API error.
+- **CSV summary accounting is exact.** Blank rows are counted and reported (`blank` in `--json`, a `Blank` line in the text summary), and the blank/actionable test is shared between the pre-flight count and the row loop, so a row containing only a `file` value can no longer be counted as actionable yet silently skipped. `Total = blank + success + skipped + failed` always holds.
+- **SMS retries only on timeout.** Non-zero `imsg` exits (bad recipient, Messages not signed in, ...) are permanent and now fail fast instead of being retried three times with backoff.
+- **`--link-signal` timeout exits non-zero**, so scripts can detect a failed link.
+- **`--list-signal` and other GET calls now retry** transient failures (429/5xx, network errors) with backoff, matching the send path.
+
+## 5.1.1
+
+### Added
+- **Updated README.md:** Exposes docker compose sample config for signal-rest-api setup along with notes on Traefik and Pullio usage and integration
+
+## 5.1.0
+
+### Added
+- **Multiple recipients per Signal send:** `--recipients` now accepts more than one value, and `--to` accepts multiple numbers, so a single invocation can fan out to several groups and/or direct recipients. Each target is sent independently and reported individually; the command exits non-zero if any target fails.
+- **`--json` CSV summary:** Emits a machine-readable JSON summary (totals, per-bucket counts, and notes) for unattended runs, as an alternative to the formatted text summary.
+- **SMS timeout and retry:** `imsg` invocations now run with a 60-second timeout and are retried up to 3 times with backoff on timeout or transient failure, matching the resilience of the Signal REST path. Previously a hung Messages app could stall an entire batch and SMS sends had no retry.
+- **Startup configuration warnings:** A `signal_rest_url` that is not a well-formed `http(s)://` URL now warns at startup, and attempting a Signal send from the built-in placeholder account (`+1234567890`) warns that no real account is configured.
+
+### Changed
+- **Skip vs. fail accounting corrected.** Intentional skips — unknown `method`, missing/invalid SMS `service`, `voice` on an SMS row, and empty messages — are now counted under **Skipped** rather than **Failed**. The process exit code is `1` only when a genuine send fails, so deliberate skips no longer cause unattended runs to report failure.
+- **Dry runs no longer report sends as successful.** A `--dry-run` now reports a separate "would send" count instead of incrementing the success total for rows that were never sent.
+- Empty-message skip notes now distinguish SMS rows (which always require text) from Signal rows (which may be attachment- or voice-only).
+
+### Fixed
+- **`--link-signal` false success:** The account snapshot used for new-link detection now tolerates all response shapes returned by the REST API (bare list, `{"accounts": [...]}`, and `{"number": ...}`). Previously a dict-shaped response produced an empty baseline, so any pre-existing account could be misreported as a newly linked device. Account listing and link detection now share one shape-tolerant helper.
+- The `--help` epilog and module docstring CSV column lists now include the `voice` column, matching the documented CSV format and the code.
+
 ## 5.0.0
 
 ### Added

README.md

@@ -55,7 +55,7 @@ It supports individual direct messages, Signal group broadcasts, file attachment
 └─────────────────────────────────────────────────────────┘
 ```
 
-**Signal path:** `sendmsg` → JSON HTTP POST → `signal-cli-rest-api` → Docker container → Signal network. Attachments are base64-encoded inline in the JSON request body (the API does not accept multipart uploads). Voice messages are sent the same way with the API's `voice` flag set so they render as playable voice notes.
+**Signal path:** `sendmsg` → JSON HTTP POST → `signal-cli-rest-api` → Docker container → Signal network. Attachments are base64-encoded inline in the JSON request body (the API does not accept multipart uploads). Voice messages are sent the same way, as an audio attachment in a message of its own — the REST API has no dedicated voice-note flag, so how the audio renders (inline player vs. file) is up to the receiving client.
 
 **SMS/iMessage path:** `sendmsg` → subprocess call → `imsg` CLI → macOS Messages framework → carrier/Apple.
 
@@ -107,15 +107,52 @@ chmod +x /usr/local/bin/sendmsg
 
    **For Signal:**
 
+   a. **Get signal-cli-rest running in docker compose:**
+   Put the following config into your docker-compose.yml file (this assumes you are using [traefik](https://github.com/traefik/traefik) to reverse proxy your docker compose services and [pullio](https://hotio.dev/scripts/pullio/) to automate updates)
+
+```
+services:
+  signal-api:
+    image: bbernhard/signal-cli-rest-api:latest
+    restart: always
+    expose:
+      - "8080"
+    labels:
+      - "traefik.http.routers.$NAME.rule=Host(`$SIGNAL_INTERNAL_URL`)"
+      - "traefik.http.routers.$NAME.entrypoints=websecure"
+      - "traefik.http.routers.$NAME.tls.certresolver=route53"
+      - "org.hotio.pullio.notify=true"
+      - "org.hotio.pullio.update=true"
+      - "org.hotio.pullio.generic.webhook=$WEBHOOK_URL"
+      - "org.hotio.pullio.author.avatar=$GRAVATAR_URL"
+    volumes:
+      - $DOCKER_STORAGE_DIR:/home/.local/share/signal-cli
+    environment:
+      - MODE=native
+      - AUTO_RECEIVE_SCHEDULE=0 22 * * *
+      - PUID=$USER_ID
+      - PGID=$GROUP_ID
+      - TZ=$TIMEZONE_NAME
+```
+
+- _$SIGNAL_INTERNAL_URL_ - The internal URL traefik will server the container from https://example.internal.mydomain.com
+- _$WEBHOOK_URL_ - The URL to your IFTTT webhook for Pullio, i.e https://maker.ifttt.com/trigger/Pullio/with/key/test-key
+- _$GRAVATAR_URL_ - The URL for your [Gravatar](https://gravatar.com/) icon i.e https://s.gravatar.com/avatar/1234567890
+- _$NAME_ - The name you give to this service in traefik
+- _$DOCKER_STORAGE_DIR_ - The directory on your docker compose host where the signal containers filesystem lives i.e /opt/docker-data/signal
+- _$TIMEZONE_NAME_ - The [Linux Timezone](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones) name i.e "America/Los_Angeles"
+
+<sub>**Note:** See https://builder.aws.com/content/2tmBo5uCgpwQegRBsdSDmupj5rH/lets-encrypt-dns01-challenge-with-traefik-and-aws-route-53 for examples of using AWS Route53 with traefik to do Lets Encrypt ACME DNS-01 requests for automatic SSL certificate creation with Traefik. Installation of traefik is outside the scope of this document.</sub>
+
+  b. **Verify signal-cli-rest-api is running**
 ```
-# Verify signal-cli-rest-api is running
 curl -s http://localhost:8080/v1/accounts | python3 -m json.tool
 ```
 
    **For SMS/iMessage:**
 
+  Verify imsg is installed and available
 ```
-# Verify imsg is installed and available
 imsg send --help
 ```
 
@@ -140,6 +177,7 @@ Messaging methods (choose one):
 
 Management commands (choose one):
   --list-signal         List linked Signal accounts
+  --list-groups         List Signal groups and their IDs (use --account to pick the account)
   --link-signal         Link a new device to your Signal account
   --show-config         Print resolved configuration and where each value came from
 ```
@@ -173,8 +211,10 @@ sendmsg --signal --to +18885551212 --attach ~/report.pdf
 
 ### Voice Messages
 
-Send an audio file as a Signal **voice note** (a playable voice message,
-not a file attachment) with `--voice`:
+Send an audio file as a Signal **voice message** with `--voice`. The audio is
+delivered as an audio attachment in its own message (the Signal REST API has
+no dedicated voice-note field, so whether it renders as an inline playable
+voice note or as an audio file depends on the recipient's client):
 
 ```
 # Send a voice message
@@ -230,6 +270,10 @@ sendmsg --csv messages.csv --yes        # Skip the large-batch confirmation
 # List linked Signal accounts
 sendmsg --list-signal
 
+# List Signal groups and their IDs (for --recipients and CSV rows)
+sendmsg --list-groups
+sendmsg --list-groups --account +18885551212
+
 # Link a new device
 sendmsg --link-signal
 sendmsg --link-signal --name "my-laptop"
@@ -254,11 +298,12 @@ Create a CSV file with the following columns:
 | `account`   | No       | Signal account phone number (defaults to `$SIGNAL_ACCOUNT` / config value)           |
 | `service`   | Varies   | SMS service: `imessage` or `sms`. **Required** on `sms` rows; ignored for `signal`.  |
 | `file`      | No       | Path to an attachment file. `~` and environment variables are expanded.              |
-| `voice`     | No       | Path to an audio file to send as a Signal **voice note**. Signal only.               |
+| `voice`     | No       | Path to an audio file to send as a Signal **voice message**. Signal only.                       |
 | `delay`     | No       | Seconds to wait after this row is sent (overrides the global `--delay`).             |
 
 **Notes**
 
+- **Commas in the message are fine.** The standard way is to quote the field: `signal,+1888,Alice,"Hi, how are you?",,,,,`. If a message contains *unquoted* commas, `sendmsg` detects that the row has more fields than the header, merges the extra fragments back into the `message` column (keeping the columns before and after it intact), and prints a warning suggesting quoting. Fields that themselves contain quotes follow normal CSV rules (`"She said ""hi"", then left"`).
 - `signal` rows may be **attachment-only** or **voice-only**: if a `file` or `voice` is present, the `message` may be left blank. `sms` rows always require a `message`.
 - The `voice` column is **Signal only**. An `sms` row with a `voice` value is skipped and reported, since SMS/iMessage has no voice-note concept.
 - A voice note is sent as its own message; if a row has both `voice` and `file`, they are delivered as separate messages.
@@ -329,7 +374,7 @@ $ sendmsg --show-config
 ==================================================
 ⚙️  sendmsg configuration
 ==================================================
-  Version:        5.0.0
+  Version:        5.1.0
 
   Config file:    /Users/you/.sendmsg.conf (found)
 
@@ -442,8 +487,25 @@ After processing all rows, a summary is printed:
 ==================================================
 ```
 
+> Intentional skips — unknown method, missing/invalid SMS service, voice on
+> an SMS row, and empty messages — are counted under **Skipped**, not
+> **Failed**. Only genuine send failures count as **Failed**, and the exit
+> code is `1` only when something actually failed to send. Add `--json` for a
+> machine-readable summary in unattended runs.
+
 ---
 
+## Testing
+
+The repository ships with a pytest suite (`tests/test_sendmsg.py`) that runs entirely offline — no Signal container, no macOS, and no real config needed:
+
+```bash
+pip install pytest
+python3 -m pytest tests/
+```
+
+See [TESTING.md](TESTING.md) for details on selecting subsets of tests, how the suite stubs out sends, and how CI runs it.
+
 ## Changelog
 
 See [CHANGELOG.md](CHANGELOG.md) for the full version history.

TESTING.md

@@ -0,0 +1,98 @@
+# Testing sendmsg
+
+The test suite lives in `tests/test_sendmsg.py` and covers phone/group
+detection, path expansion, attachment validation, Signal payload
+construction, CSV parsing and dispatch (BOM handling, comma repair, blank
+rows, skips, dry run, `--json` output), and CLI argument validation.
+
+## Requirements
+
+- Python 3.10 or newer
+- `pytest` (the only test dependency — sendmsg itself is stdlib-only)
+
+```bash
+pip install pytest
+# or on a system where pip refuses to touch system packages (Homebrew, Debian):
+pip install pytest --break-system-packages
+# or keep it isolated:
+python3 -m venv .venv && source .venv/bin/activate && pip install pytest
+```
+
+## Running the tests
+
+Run from the repository root (the tests locate the `sendmsg` script
+relative to their own path, so the working directory doesn't actually
+matter — but the root is the natural place):
+
+```bash
+python3 -m pytest tests/
+```
+
+Expected output looks like:
+
+```
+.....................................................................    [100%]
+69 passed in 0.14s
+```
+
+### Useful variations
+
+```bash
+python3 -m pytest tests/ -v                      # one line per test, with names
+python3 -m pytest tests/ -q                      # terse summary only
+python3 -m pytest tests/ -k csv                  # only tests matching "csv"
+python3 -m pytest tests/ -k "bom or comma"       # match multiple keywords
+python3 -m pytest tests/test_sendmsg.py::TestPhoneNumbers   # one test class
+python3 -m pytest tests/test_sendmsg.py::TestCsv::test_quoted_commas_in_message  # one test
+python3 -m pytest tests/ -x                      # stop at the first failure
+python3 -m pytest tests/ --lf                    # re-run only what failed last time
+```
+
+## What the tests do NOT need
+
+- **No running signal-cli-rest-api container.** All network calls
+  (`signal_rest_post`, `signal_rest_get`) are replaced with fakes; the
+  suite asserts on the payloads sendmsg *would* send.
+- **No macOS / no `imsg`.** SMS sends are stubbed the same way, so the
+  suite runs identically on Linux and macOS.
+- **No real config.** The suite pins `SIGNAL_REST_URL` and
+  `SIGNAL_ACCOUNT` environment variables *before* importing the script,
+  so your real `~/.sendmsg.conf` and shell environment can't leak into
+  (or be touched by) test runs. Nothing is ever actually sent.
+
+## How the script gets imported
+
+`sendmsg` has no `.py` extension, so the suite loads it with
+`importlib`'s `SourceFileLoader` (see the top of `tests/test_sendmsg.py`).
+The loaded module is a normal Python module — tests call its functions
+(`normalize_account`, `send_one_signal`, `cmd_csv` via `main()`, ...)
+directly and monkeypatch its globals.
+
+Two consequences worth knowing:
+
+1. Module-level code (config loading) runs at import time — that's why
+   the env vars are pinned first.
+2. If you rename or move the `sendmsg` script, update `SCRIPT_PATH` at
+   the top of the test file.
+
+## Continuous integration
+
+`.github/workflows/ci.yml` runs on every push and pull request:
+a compile check (`python -m py_compile sendmsg`) followed by the full
+suite on Python 3.10, 3.11, 3.12, and 3.13. A green run on your branch
+means the same command that CI uses passed:
+
+```bash
+python -m py_compile sendmsg && python3 -m pytest tests/ -v
+```
+
+## Adding tests
+
+- Pure helpers (parsing, normalization, validation) get direct
+  parametrized tests — see `TestPhoneNumbers` for the pattern.
+- Anything that would send goes through the stubbing helpers:
+  `run_csv()` for CSV behavior (returns exit code, stdout, stderr, and
+  the captured Signal/SMS calls) and the `captured_posts` fixture for
+  payload-level assertions.
+- When fixing a bug, add a test that fails on the old behavior first —
+  most tests in the suite carry a comment naming the bug they pin down.

sendmsg

@@ -9,19 +9,20 @@ Usage:
     sendmsg --sms --to +18005551212 --text "Hey" --service imessage --file ~/photo.png
     sendmsg --signal --to +18005551212 --text "Multi" --text "more text"
     sendmsg --signal --to +18005551212 --attach ~/doc.pdf ~/pic.jpg
-    sendmsg --signal --to +18005551212 --voice ~/note.m4a   # Send a voice message
+    sendmsg --signal --to +18005551212 --voice ~/note.m4a   # Send an audio/voice message
     sendmsg --csv messages.csv             # Bulk send from CSV file
     sendmsg --csv messages.csv --delay 2   # Wait N seconds between sends
     sendmsg --csv messages.csv --dry-run   # Preview without sending
     sendmsg --list-signal                  # Show linked Signal accounts
+    sendmsg --list-groups                  # Show Signal groups and their IDs
     sendmsg --link-signal                  # Link a new device to your existing Signal account
 
 CSV format (messages.csv):
-    method,recipient,name,message,account,service,file,delay
-    signal,+1888222333,Alice,Hello via Signal,+1888222333,,,
-    signal,group.ZzBHd3NZO...,Group alert,Hi team,,,,
-    sms,+1234567890,Bob,SMS test,,imessage,~/pic.jpg,
-    signal,+1888222333,Carol,With delay,,,,3
+    method,recipient,name,message,account,service,file,voice,delay
+    signal,+1888222333,Alice,Hello via Signal,+1888222333,,,,
+    signal,group.ZzBHd3NZO...,Group alert,Hi team,,,,,
+    sms,+1234567890,Bob,SMS test,,imessage,~/pic.jpg,,
+    signal,+1888222333,Carol,With delay,,,,,3
 """
 
 import argparse
@@ -39,7 +40,7 @@ import urllib.error
 import urllib.parse
 import urllib.request
 
-VERSION = "5.0.0"
+VERSION = "5.2.0"
 
 CONFIG_PATHS = [
     os.path.expanduser("~/.sendmsg.conf"),
@@ -108,6 +109,18 @@ CFG = load_config()
 SIGNAL_REST_URL = CFG["signal_rest_url"]
 SIGNAL_DEFAULT_ACCOUNT = CFG["signal_default_account"]
 
+# The built-in default account is a placeholder; sending from it will fail at
+# the Signal server. Track whether the resolved account is still the default
+# so we can warn before attempting a real send.
+PLACEHOLDER_ACCOUNT = "+1234567890"
+
+if not (SIGNAL_REST_URL.startswith("http://") or SIGNAL_REST_URL.startswith("https://")):
+    print(
+        f"⚠️ Warning: signal_rest_url '{SIGNAL_REST_URL}' does not look like a "
+        f"valid URL (expected http:// or https://).",
+        file=sys.stderr,
+    )
+
 
 # ---------------------------------------------------------------------------
 # Helpers — reusable without argparse objects (needed for CSV dispatch)
@@ -125,9 +138,23 @@ def expand_path(path):
     return os.path.expanduser(os.path.expandvars(path.strip()))
 
 
+# Characters commonly used to format phone numbers. These must be stripped
+# before phone-vs-group detection: '+1 (800) 555-1212' is a phone number, but
+# without stripping it fails the all-digits test and is mis-routed as a
+# Signal group ID (the same class of bug as issue #3).
+_PHONE_FORMAT_CHARS = str.maketrans("", "", " \t-.()")
+
+
+def strip_phone_separators(value):
+    """Remove spaces, dashes, dots, and parentheses from a phone number."""
+    return value.translate(_PHONE_FORMAT_CHARS)
+
+
 def normalize_account(account):
     """Ensure an account/phone number carries a leading '+'.
 
+    Formatting characters (spaces, dashes, dots, parentheses) are stripped
+    so the number is in the E.164-ish form the Signal REST API expects.
     Group IDs (starting with 'group.') are returned unchanged.
     """
     if not account:
@@ -135,6 +162,7 @@ def normalize_account(account):
     account = account.strip()
     if account.startswith("group."):
         return account
+    account = strip_phone_separators(account)
     if not account.startswith("+"):
         account = "+" + account
     return account
@@ -143,17 +171,20 @@ def normalize_account(account):
 def looks_like_phone_number(value):
     """Return True if value looks like a phone number.
 
-    Accepts numbers with an explicit '+', and also bare all-digit strings
-    (which a user may type without the leading '+'). This is the inverse
-    test used by is_group_id, and the two must agree on what a bare number
-    is so a phone number is never mis-routed as a group.
+    Accepts numbers with an explicit '+', bare all-digit strings, and
+    numbers containing common formatting characters ('+1 (800) 555-1212',
+    '555-867-5309'). This is the inverse test used by is_group_id, and the
+    two must agree on what a phone number is so a number is never
+    mis-routed as a group.
     """
     if not value:
         return False
-    value = value.strip()
+    value = strip_phone_separators(value.strip())
+    if not value:
+        return False
     if value.startswith("+"):
         return value[1:].isdigit() and len(value) > 1
-    # A bare run of digits (optionally long) is a phone number, not a group.
+    # A bare run of digits is a phone number, not a group.
     return value.isdigit()
 
 
@@ -237,6 +268,14 @@ def send_one_signal(account, recipient, message, group_id=None,
     """
     account = normalize_account(account)
 
+    if account == PLACEHOLDER_ACCOUNT:
+        print(
+            f"⚠️ Warning: sending from the placeholder account {PLACEHOLDER_ACCOUNT}. "
+            f"Set SIGNAL_ACCOUNT or signal_default_account in ~/.sendmsg.conf, "
+            f"or pass --account.",
+            file=sys.stderr,
+        )
+
     target = group_id if group_id else normalize_recipient(recipient)
     payload = {"number": account, "recipients": [target]}
 
@@ -265,12 +304,16 @@ def send_one_signal(account, recipient, message, group_id=None,
         )
         return False
 
-    # A voice note is sent as its own message (a voice note cannot be mixed
-    # with arbitrary file attachments in a single send).
+    # A voice message is sent as its own message, separate from any other
+    # file attachments. NOTE: signal-cli-rest-api's /v2/send has no
+    # voice-note field (a previous release sent "voice": true, which the
+    # API silently ignored), so the audio is delivered as a regular audio
+    # attachment. How it renders (inline player vs. file) is up to the
+    # receiving client.
     overall_ok = True
 
     if has_voice:
-        voice_payload = {"number": account, "recipients": [target], "voice": True}
+        voice_payload = {"number": account, "recipients": [target]}
         if message and not has_attachment:
             # Attach the text to the voice message if there are no other files.
             voice_payload["message"] = message
@@ -314,8 +357,25 @@ def send_one_sms(to, message, service, file=None):
         print("❌ SMS send failed: 'imsg' CLI not found on PATH.", file=sys.stderr)
         return False
 
+    # imsg can hang if the Messages app is unresponsive; bound each attempt
+    # with a timeout and retry a couple of times on transient failures.
+    max_attempts = 3
+    backoff = 2.0
+    for attempt in range(1, max_attempts + 1):
         try:
-        result = subprocess.run(cmd, capture_output=True, text=True)
+            result = subprocess.run(cmd, capture_output=True, text=True, timeout=60)
+        except subprocess.TimeoutExpired:
+            if attempt < max_attempts:
+                wait = backoff * attempt
+                print(
+                    f"⚠️ SMS send to {to} timed out; retrying in {wait:.0f}s "
+                    f"(attempt {attempt}/{max_attempts})...",
+                    file=sys.stderr,
+                )
+                time.sleep(wait)
+                continue
+            print(f"❌ SMS send failed: 'imsg' timed out after {max_attempts} attempts.", file=sys.stderr)
+            return False
         except OSError as e:
             print(f"❌ SMS send failed: could not run 'imsg': {e}", file=sys.stderr)
             return False
@@ -323,10 +383,15 @@ def send_one_sms(to, message, service, file=None):
         if result.returncode == 0:
             print(f"✅ SMS/iMessage sent to {to} (service: {service.upper()})")
             return True
-    else:
+
+        # A non-zero exit is almost always permanent (bad recipient, Messages
+        # not signed in, etc.); retrying just burns time. Only timeouts —
+        # a hung Messages app — are treated as transient and retried above.
         print(f"❌ SMS send failed (exit {result.returncode}): {result.stderr.strip()}", file=sys.stderr)
         return False
 
+    return False
+
 
 # ---------------------------------------------------------------------------
 # REST API
@@ -418,8 +483,15 @@ def signal_rest_post(endpoint, payload=None, files=None):
 
 
 def signal_rest_get(endpoint):
-    """GET a JSON response from the signal-cli-rest-api."""
+    """GET a JSON response from the signal-cli-rest-api.
+
+    Transient failures (429/5xx, network errors) are retried with backoff,
+    matching the resilience of the POST helper.
+    """
     url = f"{SIGNAL_REST_URL.rstrip('/')}{endpoint}"
+    max_attempts = 3
+    backoff = 2.0
+    for attempt in range(1, max_attempts + 1):
         req = urllib.request.Request(url, method="GET")
         try:
             with urllib.request.urlopen(req, timeout=15) as resp:
@@ -429,11 +501,30 @@ def signal_rest_get(endpoint):
                 return None
         except urllib.error.HTTPError as e:
             body = e.read().decode("utf-8", errors="replace")
+            if e.code in (429, 500, 502, 503, 504) and attempt < max_attempts:
+                wait = backoff * attempt
+                print(
+                    f"⚠️ Signal REST HTTP {e.code}; retrying in {wait:.0f}s "
+                    f"(attempt {attempt}/{max_attempts})...",
+                    file=sys.stderr,
+                )
+                time.sleep(wait)
+                continue
             print(f"❌ Signal REST error: HTTP {e.code} — {body[:500]}", file=sys.stderr)
             return None
         except urllib.error.URLError as e:
+            if attempt < max_attempts:
+                wait = backoff * attempt
+                print(
+                    f"⚠️ Cannot reach Signal REST API ({e.reason}); retrying in "
+                    f"{wait:.0f}s (attempt {attempt}/{max_attempts})...",
+                    file=sys.stderr,
+                )
+                time.sleep(wait)
+                continue
             print(f"❌ Cannot reach Signal REST API at {SIGNAL_REST_URL}: {e.reason}", file=sys.stderr)
             return None
+    return None
 
 
 # ---------------------------------------------------------------------------
@@ -456,16 +547,24 @@ def cmd_signal(args):
         sys.exit(1)
 
     account = normalize_account(args.account or SIGNAL_DEFAULT_ACCOUNT)
-
-    if args.recipients:
-        recipient = None
-        group_id = args.recipients
-    else:
-        recipient = args.to[0]
-        group_id = None
-
     message = "\n".join(args.text) if args.text else None
 
+    # Build the list of targets. --recipients values are treated as group IDs
+    # or raw recipients; --to values are direct phone numbers. Either may
+    # contain more than one entry, so a single invocation can fan out.
+    targets = []  # list of (recipient, group_id, label)
+    if args.recipients:
+        for r in args.recipients:
+            if is_group_id(r):
+                targets.append((None, r, f"group {r}"))
+            else:
+                targets.append((r, None, normalize_recipient(r)))
+    if args.to:
+        for t in args.to:
+            targets.append((t, None, normalize_recipient(t)))
+
+    all_ok = True
+    for recipient, group_id, label in targets:
         ok = send_one_signal(
             account=account,
             recipient=recipient,
@@ -474,16 +573,13 @@ def cmd_signal(args):
             attach=attach_files,
             voice=voice_file,
         )
-
         if ok:
-        if args.recipients:
-            print(f"✅ Signal message sent to group {args.recipients}")
-        elif args.to:
-            print(f"✅ Signal message sent to {normalize_recipient(args.to[0])}")
+            print(f"✅ Signal message sent to {label}")
         else:
-            print("✅ Signal message sent")
-    else:
-        print("❌ Signal message send failed.", file=sys.stderr)
+            print(f"❌ Signal message send failed: {label}", file=sys.stderr)
+            all_ok = False
+
+    if not all_ok:
         sys.exit(1)
 
 
@@ -515,13 +611,18 @@ def cmd_sms(args):
             else:
                 print(f"⚠️ Warning: SMS supports one attachment; ignoring extra file: {ef}", file=sys.stderr)
 
+    # Fan out to every --to recipient, matching the Signal path. Previously
+    # only args.to[0] was sent and extra recipients were silently dropped.
+    all_ok = True
+    for to in args.to:
         ok = send_one_sms(
-        to=args.to[0],
+            to=normalize_account(to),
             message=msg_text,
             service=args.service,
             file=chosen_file,
         )
-    if not ok:
+        all_ok = all_ok and ok
+    if not all_ok:
         sys.exit(1)
 
 
@@ -538,28 +639,83 @@ def cmd_csv(args):
 
     # Global delay from --delay applies between every send unless a row
     # specifies its own delay (which takes precedence).
-    global_delay = float(args.delay) if args.delay else 0.0
+    global_delay = args.delay if args.delay else 0.0
 
-    with open(filepath, newline="", encoding="utf-8") as csvfile:
-        reader = csv.DictReader(csvfile)
+    # With --json the summary on stdout must be the ONLY stdout output so it
+    # can be piped to a parser; per-row progress then goes to stderr.
+    progress = sys.stderr if getattr(args, "json_output", False) else sys.stdout
+
+    # 'utf-8-sig' transparently strips a UTF-8 BOM. Excel and many editors
+    # write one; under plain 'utf-8' it corrupts the first header to
+    # '\ufeffmethod', making every row's method look empty — which then
+    # defaulted to 'signal' and silently misrouted explicit 'sms' rows.
+    # restkey collects overflow fields (see repair below); restval="" fills
+    # missing trailing columns so short rows never produce None values.
+    with open(filepath, newline="", encoding="utf-8-sig") as csvfile:
+        reader = csv.DictReader(csvfile, restkey="_overflow", restval="")
+        fieldnames = reader.fieldnames or []
         rows = list(reader)
 
     if not rows:
         print("❌ Error: CSV file is empty (no data rows).", file=sys.stderr)
         sys.exit(1)
 
-    # Count the rows that will actually be attempted (non-blank).
-    actionable = [
-        r for r in rows
-        if (r.get("method", "").strip()
-            or r.get("message", "").strip()
-            or r.get("recipient", "").strip()
-            or r.get("file", "").strip()
-            or r.get("voice", "").strip())
-    ]
+    def repair_overflow(row):
+        """Merge unquoted-comma overflow back into the message field.
+
+        Standard CSV quoting ('"Hi, there"') is fully supported and needs no
+        repair. But a message containing UNQUOTED commas splits into extra
+        fields, shifting every column after 'message' (so part of the text
+        lands in 'account', the delay in 'voice', etc.) and corrupting the
+        row silently. When a row has more fields than the header, the only
+        sane interpretation is that the extras belong to the free-text
+        message column: keep the columns before 'message' from the front,
+        the columns after it from the back, and rejoin everything in the
+        middle with commas.
+
+        Returns (row, n_extra_fields_merged).
+        """
+        overflow = row.pop("_overflow", None)
+        if not overflow or "message" not in fieldnames:
+            return row, 0
+        idx = fieldnames.index("message")
+        n_after = len(fieldnames) - idx - 1
+        raw = [str(row.get(h) or "") for h in fieldnames] + [str(x) for x in overflow]
+        merged = ",".join(raw[idx:len(raw) - n_after])
+        repaired = dict(zip(fieldnames, raw[:idx] + [merged] + raw[len(raw) - n_after:]))
+        return repaired, len(overflow)
+
+    repair_notes = []
+    for idx0, row in enumerate(rows):
+        repaired, n_extra = repair_overflow(row)
+        if n_extra:
+            rows[idx0] = repaired
+            note = (
+                f"Row {idx0 + 1}: message contains unquoted commas; merged "
+                f"{n_extra} extra field(s) back into the message. Quote the "
+                f'field ("...") to silence this.'
+            )
+            repair_notes.append(note)
+            print(f"⚠️ {note}", file=sys.stderr)
+
+    def row_is_blank(r):
+        """True if a row has no content in any actionable column.
+
+        Used both for the pre-flight count and the per-row skip so the two
+        can never disagree (previously a row with only a 'file' value was
+        counted as actionable but then silently skipped as blank).
+        """
+        return not any(
+            (r.get(col) or "").strip()
+            for col in ("method", "recipient", "message", "file", "voice")
+        )
+
+    actionable = [r for r in rows if not row_is_blank(r)]
+    blank = len(rows) - len(actionable)
 
     if args.dry_run:
-        print(f"🔎 Dry run: {len(actionable)} row(s) would be processed from {filepath}\n")
+        print(f"🔎 Dry run: {len(actionable)} row(s) would be processed from {filepath}\n",
+              file=progress)
     elif len(actionable) > CSV_CONFIRM_THRESHOLD and not args.yes:
         try:
             resp = input(
@@ -575,17 +731,18 @@ def cmd_csv(args):
     success = 0
     failed = 0
     skipped = 0
-    errors = []
+    previewed = 0
+    errors = list(repair_notes)
 
     for i, row in enumerate(rows, 1):
-        method = row.get("method", "").strip().lower()
-        message = row.get("message", "").strip()
-        to = row.get("recipient", "").strip()
-        name = row.get("name", "").strip()
-        voice_path = row.get("voice", "").strip() or None
+        method = (row.get("method") or "").strip().lower()
+        message = (row.get("message") or "").strip()
+        to = (row.get("recipient") or "").strip()
+        name = (row.get("name") or "").strip()
+        voice_path = (row.get("voice") or "").strip() or None
 
-        # Skip blank rows
-        if not method and not message and not to and not voice_path:
+        # Skip blank rows (same test as the pre-flight count above).
+        if row_is_blank(row):
             continue
 
         if not method:
@@ -593,7 +750,7 @@ def cmd_csv(args):
 
         if method not in ("signal", "sms"):
             print(f"⚠️ Row {i}: unknown method '{method}' — skipping", file=sys.stderr)
-            failed += 1
+            skipped += 1
             errors.append(f"Row {i}: unknown method '{method}'")
             continue
 
@@ -608,14 +765,28 @@ def cmd_csv(args):
             print(f"⚠️ Row {i}: invalid delay '{delay_str}' — ignoring", file=sys.stderr)
             row_delay = None
 
+        # A recipient is always required. Previously a recipient-less signal
+        # row was passed through and POSTed with an empty recipients entry,
+        # producing a confusing API error (and a dry run reported it as
+        # sendable).
+        if not to:
+            print(f"⚠️ Row {i}: missing recipient — skipping", file=sys.stderr)
+            skipped += 1
+            errors.append(f"Row {i}: missing recipient")
+            continue
+
         # A message is required UNLESS there is an attachment or a voice note
         # (Signal can send attachment-only / voice-only messages). SMS still
         # requires text.
         has_signal_media = method == "signal" and (file_path or voice_path)
         if not message and not has_signal_media:
-            print(f"⚠️ Row {i}: empty message — skipping", file=sys.stderr)
+            if method == "sms":
+                reason = "empty message (SMS requires text)"
+            else:
+                reason = "empty message (no text, attachment, or voice)"
+            print(f"⚠️ Row {i}: {reason} — skipping", file=sys.stderr)
             skipped += 1
-            errors.append(f"Row {i}: empty message")
+            errors.append(f"Row {i}: {reason}")
             continue
 
         if method == "sms":
@@ -625,7 +796,7 @@ def cmd_csv(args):
                     f"(method 'signal') — skipping",
                     file=sys.stderr,
                 )
-                failed += 1
+                skipped += 1
                 errors.append(f"Row {i}: voice not supported for SMS")
                 continue
             if service not in ("sms", "imessage"):
@@ -634,28 +805,39 @@ def cmd_csv(args):
                     f"(got '{service or ''}') — skipping",
                     file=sys.stderr,
                 )
-                failed += 1
+                skipped += 1
                 errors.append(f"Row {i}: invalid/missing SMS service")
                 continue
 
         label = f"{name} ({to})" if name else to or "(group)"
         prefix = "[DRY] " if args.dry_run else ""
-        print(f"{prefix}[{i}/{total}] {method.upper()} → {label}")
+        print(f"{prefix}[{i}/{total}] {method.upper()} → {label}", file=progress)
 
         if args.dry_run:
-            # Report what would happen without sending.
+            # Report what would happen without sending, and validate the
+            # attachment paths so problems surface BEFORE a real run.
             extras = []
             if file_path:
+                expanded = expand_path(file_path)
+                if os.path.isfile(expanded):
                     extras.append(f"file={file_path}")
+                else:
+                    extras.append(f"file={file_path} [NOT FOUND]")
+                    errors.append(f"Row {i}: attachment not found: {expanded}")
             if voice_path:
+                expanded = expand_path(voice_path)
+                if os.path.isfile(expanded):
                     extras.append(f"voice={voice_path}")
+                else:
+                    extras.append(f"voice={voice_path} [NOT FOUND]")
+                    errors.append(f"Row {i}: voice file not found: {expanded}")
             if method == "signal" and is_group_id(to):
                 extras.append("group")
             if method == "sms":
                 extras.append(f"service={service}")
             if extras:
-                print(f"        ({', '.join(extras)})")
-            success += 1
+                print(f"        ({', '.join(extras)})", file=progress)
+            previewed += 1
             continue
 
         if method == "signal":
@@ -691,11 +873,30 @@ def cmd_csv(args):
             time.sleep(effective_delay)
 
     # Summary
+    if getattr(args, "json_output", False):
+        summary = {
+            "total": total,
+            "blank": blank,
+            "success": success,
+            "skipped": skipped,
+            "failed": failed,
+            "dry_run": bool(args.dry_run),
+            "notes": errors,
+        }
+        if args.dry_run:
+            summary["would_send"] = previewed
+        print(json.dumps(summary, indent=2))
+    else:
         print()
         print("=" * 50)
         print("📊 CSV Send Summary" + (" (DRY RUN)" if args.dry_run else ""))
         print("=" * 50)
         print(f"  Total rows:   {total}")
+        if blank:
+            print(f"  ⬜ Blank:      {blank}")
+        if args.dry_run:
+            print(f"  🔎 Would send: {previewed}")
+        else:
             print(f"  ✅ Success:    {success}")
         print(f"  ⏭️  Skipped:    {skipped}")
         print(f"  ❌ Failed:     {failed}")
@@ -750,6 +951,24 @@ def cmd_show_config(args):
         print("=" * 50)
 
 
+def _extract_accounts(result):
+    """Normalize the various shapes /v1/accounts may return into a list of str.
+
+    The REST API has returned a bare list, a {'accounts': [...]} dict, and a
+    single {'number': ...} dict across versions. Returns [] for None/unknown.
+    """
+    if result is None:
+        return []
+    if isinstance(result, list):
+        return [str(a) for a in result]
+    if isinstance(result, dict):
+        if "accounts" in result and isinstance(result["accounts"], list):
+            return [str(a) for a in result["accounts"]]
+        if "number" in result:
+            return [str(result["number"])]
+    return []
+
+
 def cmd_list_signal(args):
     """List linked Signal accounts via the REST API."""
     result = signal_rest_get("/v1/accounts")
@@ -757,14 +976,7 @@ def cmd_list_signal(args):
         print(f"❌ Failed to list accounts (cannot reach {SIGNAL_REST_URL})", file=sys.stderr)
         sys.exit(1)
 
-    if isinstance(result, list):
-        accounts = [str(a) for a in result]
-    elif isinstance(result, dict) and "accounts" in result:
-        accounts = result["accounts"]
-    elif isinstance(result, dict) and "number" in result:
-        accounts = [result["number"]]
-    else:
-        accounts = [str(result)]
+    accounts = _extract_accounts(result)
 
     if not accounts:
         print("No Signal accounts linked. Use --link-signal to set one up.")
@@ -785,9 +997,7 @@ def cmd_link_signal(args):
     # Snapshot existing accounts so we can detect a genuinely NEW link rather
     # than reporting success just because a primary account already exists.
     before = signal_rest_get("/v1/accounts")
-    before_set = set()
-    if isinstance(before, list):
-        before_set = {str(a) for a in before}
+    before_set = set(_extract_accounts(before))
 
     payload = {"name": name}
     result = signal_rest_post("/v1/link", payload=payload)
@@ -811,15 +1021,58 @@ def cmd_link_signal(args):
     start = time.time()
     while time.time() - start < 120:
         time.sleep(3)
-        current = signal_rest_get("/v1/accounts")
-        if isinstance(current, list):
-            current_set = {str(a) for a in current}
+        current_set = set(_extract_accounts(signal_rest_get("/v1/accounts")))
         new_accounts = current_set - before_set
         if new_accounts:
             print(f"✅ Device linked! New account(s): {', '.join(sorted(new_accounts))}")
             return
 
     print("⚠️ Linking timed out (no new account detected). Please try again.", file=sys.stderr)
+    sys.exit(1)
+
+
+def cmd_list_groups(args):
+    """List the Signal groups the account belongs to, with their IDs.
+
+    Uses GET /v1/groups/{number}. This is the sanctioned way to discover the
+    group IDs that CSV rows and --recipients expect.
+    """
+    account = normalize_account(args.account or SIGNAL_DEFAULT_ACCOUNT)
+    if account == PLACEHOLDER_ACCOUNT:
+        print(
+            f"❌ Error: no Signal account configured (still the placeholder "
+            f"{PLACEHOLDER_ACCOUNT}). Set SIGNAL_ACCOUNT, set "
+            f"signal_default_account in ~/.sendmsg.conf, or pass --account.",
+            file=sys.stderr,
+        )
+        sys.exit(1)
+
+    endpoint = f"/v1/groups/{urllib.parse.quote(account, safe='')}"
+    result = signal_rest_get(endpoint)
+    if result is None:
+        print(f"❌ Failed to list groups for {account}.", file=sys.stderr)
+        sys.exit(1)
+
+    # Shape-tolerant: expect a list of group dicts, but don't crash on
+    # variations. Known keys across API versions: name, id ('group.<b64>'),
+    # internal_id, members, blocked.
+    groups = result if isinstance(result, list) else result.get("groups", []) if isinstance(result, dict) else []
+    if not groups:
+        print(f"No Signal groups found for {account}.")
+        return
+
+    print(f"Signal groups for {account}:")
+    for g in groups:
+        if not isinstance(g, dict):
+            print(f"  • {g}")
+            continue
+        gname = g.get("name") or "(unnamed)"
+        gid = g.get("id") or g.get("internal_id") or "(no id)"
+        members = g.get("members")
+        member_note = f", {len(members)} member(s)" if isinstance(members, list) else ""
+        blocked_note = " [blocked]" if g.get("blocked") else ""
+        print(f"  • {gname}{blocked_note}{member_note}")
+        print(f"      id: {gid}")
 
 
 # ---------------------------------------------------------------------------
@@ -841,9 +1094,10 @@ def main():
             "  %(prog)s --csv messages.csv --delay 2\n"
             "  %(prog)s --csv messages.csv --dry-run\n"
             "  %(prog)s --list-signal\n"
+            "  %(prog)s --list-groups\n"
             "  %(prog)s --link-signal --name 'my-automation'\n"
             "\n"
-            "CSV columns: method, recipient, name, message, account, service, file, delay\n"
+            "CSV columns: method, recipient, name, message, account, service, file, voice, delay\n"
             "The 'name' field is printed in the status output during CSV sends.\n"
             "For SMS rows the 'service' column must be 'sms' or 'imessage'.\n"
             f"Signal REST API: {SIGNAL_REST_URL}\n"
@@ -873,13 +1127,16 @@ def main():
     # Signal-specific
     signal_grp = parser.add_argument_group("Signal options")
     signal_grp.add_argument("--account", metavar="PHONE", help="Signal account phone number to use")
-    signal_grp.add_argument("--recipients", metavar="RECIPIENTS", help="Send to Signal group ID (no --to needed)")
+    signal_grp.add_argument("--recipients", nargs="+", metavar="RECIPIENT",
+                            help="Send to one or more Signal group IDs / recipients (no --to needed)")
     signal_grp.add_argument("--voice", metavar="AUDIO", help="Send an audio file as a Signal voice message")
     signal_grp.add_argument("--name", metavar="NAME", help="Device name for Signal linking (default: 'automation')")
 
     # Management commands
     mgmt = parser.add_argument_group("Management commands")
     mgmt.add_argument("--list-signal", action="store_true", help="List linked Signal accounts")
+    mgmt.add_argument("--list-groups", action="store_true",
+                      help="List Signal groups (and their IDs) for the account")
     mgmt.add_argument("--link-signal", action="store_true", help="Link a new device to your Signal account")
     mgmt.add_argument("--show-config", action="store_true",
                       help="Print resolved configuration and where each value came from")
@@ -891,10 +1148,13 @@ def main():
 
     # CSV-specific
     csv_grp = parser.add_argument_group("CSV options")
-    csv_grp.add_argument("--delay", metavar="SECONDS", help="Wait N seconds between CSV row sends (default: 0)")
+    csv_grp.add_argument("--delay", metavar="SECONDS", type=float,
+                         help="Wait N seconds between CSV row sends (default: 0)")
     csv_grp.add_argument("--dry-run", action="store_true", help="Preview a CSV batch without sending")
     csv_grp.add_argument("--yes", "-y", action="store_true",
                          help="Skip the large-batch confirmation prompt")
+    csv_grp.add_argument("--json", action="store_true", dest="json_output",
+                         help="Emit a machine-readable JSON summary (for unattended runs)")
 
     args = parser.parse_args()
 
@@ -907,6 +1167,7 @@ def main():
         "--sms": args.sms,
         "--csv": bool(args.csv),
         "--list-signal": args.list_signal,
+        "--list-groups": args.list_groups,
         "--link-signal": args.link_signal,
         "--show-config": args.show_config,
     }
@@ -914,7 +1175,7 @@ def main():
 
     if not chosen:
         parser.print_help()
-        print("\n❌ Error: Specify --signal, --sms, --csv, --list-signal, --link-signal, or --show-config", file=sys.stderr)
+        print("\n❌ Error: Specify --signal, --sms, --csv, --list-signal, --list-groups, --link-signal, or --show-config", file=sys.stderr)
         sys.exit(1)
 
     if len(chosen) > 1:
@@ -939,6 +1200,8 @@ def main():
         cmd_csv(args)
     elif args.list_signal:
         cmd_list_signal(args)
+    elif args.list_groups:
+        cmd_list_groups(args)
     elif args.link_signal:
         cmd_link_signal(args)
     elif args.show_config:

tests/test_sendmsg.py

@@ -0,0 +1,444 @@
+"""Unit tests for the sendmsg CLI.
+
+The script has no .py extension, so it's loaded via SourceFileLoader.
+Environment variables are pinned before import so a developer's real
+~/.sendmsg.conf or env can't leak into test results.
+"""
+
+import importlib.util
+import io
+import json
+import os
+import sys
+from importlib.machinery import SourceFileLoader
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+SCRIPT_PATH = REPO_ROOT / "sendmsg"
+
+# Pin config to known values BEFORE the module-level load_config() runs.
+os.environ["SIGNAL_REST_URL"] = "http://localhost:8080"
+os.environ["SIGNAL_ACCOUNT"] = "+15550001111"
+
+loader = SourceFileLoader("sendmsg_module", str(SCRIPT_PATH))
+spec = importlib.util.spec_from_loader("sendmsg_module", loader)
+sendmsg = importlib.util.module_from_spec(spec)
+loader.exec_module(sendmsg)
+
+
+# ---------------------------------------------------------------------------
+# Phone-number handling
+# ---------------------------------------------------------------------------
+
+class TestPhoneNumbers:
+    @pytest.mark.parametrize("raw,expected", [
+        ("+18005551212", "+18005551212"),
+        ("18005551212", "+18005551212"),
+        ("+1 (800) 555-1212", "+18005551212"),   # formatted (bug fix)
+        ("555-867-5309", "+5558675309"),
+        ("+1.800.555.1212", "+18005551212"),
+        ("  +18005551212  ", "+18005551212"),
+        ("group.ZzBHd3NZ", "group.ZzBHd3NZ"),    # groups pass through
+        ("", ""),
+        (None, None),
+    ])
+    def test_normalize_account(self, raw, expected):
+        assert sendmsg.normalize_account(raw) == expected
+
+    @pytest.mark.parametrize("value,expected", [
+        ("+18005551212", True),
+        ("18005551212", True),
+        ("+1 (800) 555-1212", True),   # bug fix: formatted numbers
+        ("555-867-5309", True),
+        ("+1.800.555.1212", True),
+        ("group.ZzBHd3NZ", False),
+        ("ZzBHd3NZOWlrY2xrpB==", False),  # raw base64 group key
+        ("+", False),
+        ("", False),
+        (None, False),
+        ("()- .", False),               # separators only is not a number
+    ])
+    def test_looks_like_phone_number(self, value, expected):
+        assert sendmsg.looks_like_phone_number(value) is expected
+
+    @pytest.mark.parametrize("value,expected", [
+        ("group.ZzBHd3NZ", True),
+        ("ZzBHd3NZOWlrY2xrpB==", True),   # raw base64 key is a group
+        ("+18005551212", False),
+        ("18005551212", False),
+        ("+1 (800) 555-1212", False),     # bug fix: NOT a group
+        ("555-867-5309", False),          # bug fix: NOT a group
+        ("", False),
+        (None, False),
+    ])
+    def test_is_group_id(self, value, expected):
+        assert sendmsg.is_group_id(value) is expected
+
+    def test_detectors_agree(self):
+        """A value must never be both a phone number and a group ID."""
+        for v in ["+18005551212", "555-867-5309", "group.abc", "Zz09==", "+1 (800) 555-1212"]:
+            assert not (sendmsg.looks_like_phone_number(v) and sendmsg.is_group_id(v))
+
+    def test_normalize_recipient(self):
+        assert sendmsg.normalize_recipient("800-555-1212") == "+8005551212"
+        assert sendmsg.normalize_recipient("group.abc") == "group.abc"
+
+
+# ---------------------------------------------------------------------------
+# Path expansion / attachment validation
+# ---------------------------------------------------------------------------
+
+class TestPaths:
+    def test_expand_path_tilde(self, monkeypatch):
+        monkeypatch.setenv("HOME", "/home/tester")
+        assert sendmsg.expand_path("~/pic.jpg") == "/home/tester/pic.jpg"
+
+    def test_expand_path_env_var(self, monkeypatch):
+        monkeypatch.setenv("PICDIR", "/data/pics")
+        assert sendmsg.expand_path("$PICDIR/a.png") == "/data/pics/a.png"
+
+    def test_expand_path_empty(self):
+        assert sendmsg.expand_path("") == ""
+        assert sendmsg.expand_path(None) is None
+
+    def test_validate_attachment_missing(self, capsys):
+        assert sendmsg.validate_attachment("/nope/missing.bin") is None
+        assert "not found" in capsys.readouterr().err
+
+    def test_validate_attachment_ok(self, tmp_path):
+        f = tmp_path / "a.txt"
+        f.write_text("hi")
+        assert sendmsg.validate_attachment(str(f)) == str(f)
+
+    def test_validate_attachment_too_large(self, tmp_path, monkeypatch, capsys):
+        f = tmp_path / "big.bin"
+        f.write_bytes(b"x" * 10)
+        monkeypatch.setattr(sendmsg, "MAX_ATTACHMENT_BYTES", 5)
+        assert sendmsg.validate_attachment(str(f)) is None
+        assert "too large" in capsys.readouterr().err
+
+    def test_validate_voice_format_warning(self, tmp_path, capsys):
+        f = tmp_path / "clip.txt"
+        f.write_text("not audio")
+        # Unrecognized format warns but still returns the path.
+        assert sendmsg.validate_attachment(str(f), as_voice=True) == str(f)
+        assert "not a recognized voice/audio format" in capsys.readouterr().err
+
+
+# ---------------------------------------------------------------------------
+# Accounts response shape tolerance
+# ---------------------------------------------------------------------------
+
+class TestExtractAccounts:
+    def test_none(self):
+        assert sendmsg._extract_accounts(None) == []
+
+    def test_bare_list(self):
+        assert sendmsg._extract_accounts(["+1", "+2"]) == ["+1", "+2"]
+
+    def test_accounts_dict(self):
+        assert sendmsg._extract_accounts({"accounts": ["+1"]}) == ["+1"]
+
+    def test_number_dict(self):
+        assert sendmsg._extract_accounts({"number": "+1"}) == ["+1"]
+
+    def test_unknown_shape(self):
+        assert sendmsg._extract_accounts({"weird": 1}) == []
+        assert sendmsg._extract_accounts("string") == []
+
+
+# ---------------------------------------------------------------------------
+# Signal payload construction (voice fix)
+# ---------------------------------------------------------------------------
+
+class TestSignalPayloads:
+    @pytest.fixture
+    def captured_posts(self, monkeypatch):
+        calls = []
+
+        def fake_post(endpoint, payload=None, files=None):
+            calls.append({"endpoint": endpoint, "payload": dict(payload or {}), "files": files})
+            return {}
+
+        monkeypatch.setattr(sendmsg, "signal_rest_post", fake_post)
+        return calls
+
+    def test_voice_flag_not_sent(self, captured_posts, tmp_path):
+        """/v2/send has no 'voice' field upstream; we must not send one."""
+        clip = tmp_path / "note.m4a"
+        clip.write_bytes(b"\x00\x01")
+        ok = sendmsg.send_one_signal("+15550001111", "+15550002222",
+                                     message=None, voice=str(clip))
+        assert ok is True
+        assert len(captured_posts) == 1
+        call = captured_posts[0]
+        assert call["endpoint"] == "/v2/send"
+        assert "voice" not in call["payload"]
+        assert call["files"] == str(clip) or call["files"] == [str(clip)]
+
+    def test_voice_carries_text_when_no_attachments(self, captured_posts, tmp_path):
+        clip = tmp_path / "note.m4a"
+        clip.write_bytes(b"\x00")
+        sendmsg.send_one_signal("+15550001111", "+15550002222",
+                                message="hi", voice=str(clip))
+        assert len(captured_posts) == 1
+        assert captured_posts[0]["payload"].get("message") == "hi"
+
+    def test_voice_plus_attachment_two_sends_text_once(self, captured_posts, tmp_path):
+        clip = tmp_path / "note.m4a"
+        clip.write_bytes(b"\x00")
+        doc = tmp_path / "doc.pdf"
+        doc.write_bytes(b"%PDF")
+        sendmsg.send_one_signal("+15550001111", "+15550002222",
+                                message="hi", voice=str(clip), attach=[str(doc)])
+        assert len(captured_posts) == 2
+        voice_call, attach_call = captured_posts
+        assert "message" not in voice_call["payload"]
+        assert attach_call["payload"].get("message") == "hi"
+
+    def test_formatted_number_routed_direct_not_group(self, captured_posts):
+        sendmsg.send_one_signal("+15550001111", "+1 (555) 000-2222", message="hi")
+        assert captured_posts[0]["payload"]["recipients"] == ["+15550002222"]
+
+    def test_empty_send_skipped(self, captured_posts, capsys):
+        ok = sendmsg.send_one_signal("+15550001111", "+15550002222", message=None)
+        assert ok is False
+        assert captured_posts == []
+        assert "skipped" in capsys.readouterr().err
+
+
+# ---------------------------------------------------------------------------
+# CSV behavior (run through cmd_csv with sends stubbed out)
+# ---------------------------------------------------------------------------
+
+def run_csv(monkeypatch, tmp_path, csv_bytes, argv_extra=(), signal_result=True, sms_result=True):
+    """Write csv_bytes to a file, run cmd_csv with stubbed senders.
+
+    Returns (exit_code, stdout, stderr, signal_calls, sms_calls).
+    """
+    csv_file = tmp_path / "batch.csv"
+    csv_file.write_bytes(csv_bytes)
+
+    signal_calls, sms_calls = [], []
+    monkeypatch.setattr(sendmsg, "send_one_signal",
+                        lambda **kw: signal_calls.append(kw) or signal_result)
+    monkeypatch.setattr(sendmsg, "send_one_sms",
+                        lambda **kw: sms_calls.append(kw) or sms_result)
+    monkeypatch.setattr(sendmsg.time, "sleep", lambda s: None)
+
+    argv = ["sendmsg", "--csv", str(csv_file), *argv_extra]
+    monkeypatch.setattr(sys, "argv", argv)
+
+    out, err = io.StringIO(), io.StringIO()
+    monkeypatch.setattr(sys, "stdout", out)
+    monkeypatch.setattr(sys, "stderr", err)
+
+    code = 0
+    try:
+        sendmsg.main()
+    except SystemExit as e:
+        code = e.code or 0
+    return code, out.getvalue(), err.getvalue(), signal_calls, sms_calls
+
+
+HEADER = b"method,recipient,name,message,account,service,file,voice,delay\n"
+
+
+class TestCsv:
+    def test_bom_does_not_misroute_sms_rows(self, monkeypatch, tmp_path):
+        """A UTF-8 BOM previously blanked the method column, silently
+        sending explicit 'sms' rows via Signal."""
+        data = b"\xef\xbb\xbf" + HEADER + b"sms,+15550003333,Bob,Hi,,imessage,,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert code == 0
+        assert sig == []
+        assert len(sms) == 1
+        assert sms[0]["service"] == "imessage"
+
+    def test_missing_recipient_skipped(self, monkeypatch, tmp_path):
+        data = HEADER + b"signal,,NoOne,Hello,,,,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert sig == [] and sms == []
+        assert "missing recipient" in err
+        assert code == 0  # a skip is not a failure
+
+    def test_json_stdout_is_pure_json(self, monkeypatch, tmp_path):
+        data = HEADER + (b"signal,+15550003333,A,Hello,,,,,\n"
+                         b"signal,+15550004444,B,World,,,,,\n")
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data,
+                                           argv_extra=["--json"])
+        summary = json.loads(out)  # would raise if progress lines leaked in
+        assert summary["success"] == 2
+        assert summary["failed"] == 0
+        assert "[1/2]" in err  # progress went to stderr
+
+    def test_dry_run_counts_and_flags_missing_files(self, monkeypatch, tmp_path):
+        data = HEADER + b"signal,+15550003333,A,Hello,,,/definitely/missing.jpg,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data,
+                                           argv_extra=["--dry-run", "--json"])
+        assert sig == [] and sms == []
+        summary = json.loads(out)
+        assert summary["would_send"] == 1
+        assert any("attachment not found" in n for n in summary["notes"])
+
+    def test_blank_rows_accounted(self, monkeypatch, tmp_path):
+        data = HEADER + (b"signal,+15550003333,A,Hello,,,,,\n"
+                         b",,,,,,,,\n")
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data,
+                                           argv_extra=["--json"])
+        summary = json.loads(out)
+        assert summary["total"] == 2
+        assert summary["blank"] == 1
+        assert summary["blank"] + summary["success"] + summary["skipped"] + summary["failed"] == summary["total"]
+
+    def test_failed_send_sets_exit_code(self, monkeypatch, tmp_path):
+        data = HEADER + b"signal,+15550003333,A,Hello,,,,,\n"
+        code, *_ = run_csv(monkeypatch, tmp_path, data, signal_result=False)
+        assert code == 1
+
+    def test_sms_row_requires_service(self, monkeypatch, tmp_path):
+        data = HEADER + b"sms,+15550003333,A,Hello,,,,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert sms == []
+        assert "requires service" in err
+
+    def test_voice_on_sms_row_skipped(self, monkeypatch, tmp_path):
+        data = HEADER + b"sms,+15550003333,A,Hello,,imessage,,~/note.m4a,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert sms == []
+        assert "voice" in err
+
+    def test_group_recipient_routed_as_group(self, monkeypatch, tmp_path):
+        data = HEADER + b"signal,group.ZzBHd3NZ,Team,Hi all,,,,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert sig[0]["group_id"] == "group.ZzBHd3NZ"
+
+    def test_formatted_phone_row_not_group(self, monkeypatch, tmp_path):
+        data = HEADER + b'signal,555-867-5309,Jenny,Hi,,,,,\n'
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert sig[0]["group_id"] is None
+
+    def test_bad_delay_rejected_cleanly(self, monkeypatch, tmp_path):
+        """--delay abc previously crashed with a ValueError traceback."""
+        data = HEADER + b"signal,+15550003333,A,Hello,,,,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data,
+                                           argv_extra=["--delay", "abc"])
+        assert code == 2  # argparse usage error
+        assert "invalid float value" in err
+        assert sig == []
+
+    # -- commas in the message field ------------------------------------
+
+    def test_quoted_commas_in_message(self, monkeypatch, tmp_path):
+        """Standard CSV quoting must pass commas through untouched."""
+        data = HEADER + b'signal,+15550003333,Al,"Hi, there, friend",,,,,2\n'
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert code == 0
+        assert sig[0]["message"] == "Hi, there, friend"
+        assert "unquoted commas" not in err  # no repair needed
+
+    def test_unquoted_commas_merged_into_message(self, monkeypatch, tmp_path):
+        """Unquoted commas previously shifted every column after 'message'
+        (text leaked into 'account', etc.); they are now merged back."""
+        data = HEADER + b"signal,+15550003333,Al,Hey, how are you, friend,,,,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert code == 0
+        assert sig[0]["message"] == "Hey, how are you, friend"
+        assert sig[0]["account"] == "+15550001111"  # default, not leaked text
+        assert "unquoted commas" in err
+
+    def test_unquoted_commas_preserve_trailing_columns(self, monkeypatch, tmp_path):
+        """Columns after the message (service, voice, delay...) must still
+        land in the right place after the merge."""
+        data = HEADER + b"sms,+15550003333,Al,Hi, there,,imessage,,,2\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert code == 0
+        assert sms[0]["message"] == "Hi, there"
+        assert sms[0]["service"] == "imessage"
+
+    def test_unquoted_comma_repair_noted_in_json(self, monkeypatch, tmp_path):
+        data = HEADER + b"signal,+15550003333,Al,One, two,,,,,\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data,
+                                           argv_extra=["--json"])
+        summary = json.loads(out)
+        assert summary["success"] == 1
+        assert any("unquoted commas" in n for n in summary["notes"])
+
+    def test_short_rows_tolerated(self, monkeypatch, tmp_path):
+        """Rows with fewer fields than the header must not crash."""
+        data = HEADER + b"signal,+15550003333,Al,Hello\n"
+        code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data)
+        assert code == 0
+        assert sig[0]["message"] == "Hello"
+
+
+# ---------------------------------------------------------------------------
+# CLI argument validation
+# ---------------------------------------------------------------------------
+
+class TestCli:
+    def run_main(self, monkeypatch, argv):
+        monkeypatch.setattr(sys, "argv", ["sendmsg", *argv])
+        out, err = io.StringIO(), io.StringIO()
+        monkeypatch.setattr(sys, "stdout", out)
+        monkeypatch.setattr(sys, "stderr", err)
+        code = 0
+        try:
+            sendmsg.main()
+        except SystemExit as e:
+            code = e.code or 0
+        return code, out.getvalue(), err.getvalue()
+
+    def test_no_action_errors(self, monkeypatch):
+        code, out, err = self.run_main(monkeypatch, [])
+        assert code == 1
+        assert "--list-groups" in err  # new command advertised
+
+    def test_multiple_actions_rejected(self, monkeypatch):
+        code, out, err = self.run_main(monkeypatch, ["--list-signal", "--link-signal"])
+        assert code == 1
+        assert "only one action" in err
+
+    def test_sms_fans_out_to_all_recipients(self, monkeypatch):
+        calls = []
+        monkeypatch.setattr(sendmsg, "send_one_sms",
+                            lambda **kw: calls.append(kw) or True)
+        code, out, err = self.run_main(
+            monkeypatch,
+            ["--sms", "--to", "+15550001111", "+15550002222",
+             "--text", "hi", "--service", "sms"],
+        )
+        assert code == 0
+        assert [c["to"] for c in calls] == ["+15550001111", "+15550002222"]
+
+    def test_list_groups_refuses_placeholder_account(self, monkeypatch):
+        monkeypatch.setattr(sendmsg, "SIGNAL_DEFAULT_ACCOUNT", "+1234567890")
+        code, out, err = self.run_main(monkeypatch, ["--list-groups"])
+        assert code == 1
+        assert "placeholder" in err
+
+    def test_list_groups_output(self, monkeypatch):
+        monkeypatch.setattr(sendmsg, "signal_rest_get", lambda ep: [
+            {"name": "Ops", "id": "group.QWJj", "members": ["+1", "+2"], "blocked": False},
+            {"name": None, "internal_id": "Zz09"},
+        ])
+        code, out, err = self.run_main(
+            monkeypatch, ["--list-groups", "--account", "+15550001111"])
+        assert code == 0
+        assert "Ops" in out
+        assert "group.QWJj" in out
+        assert "2 member(s)" in out
+        assert "(unnamed)" in out
+
+    def test_list_groups_url_encodes_account(self, monkeypatch):
+        seen = {}
+
+        def fake_get(endpoint):
+            seen["endpoint"] = endpoint
+            return []
+
+        monkeypatch.setattr(sendmsg, "signal_rest_get", fake_get)
+        self.run_main(monkeypatch, ["--list-groups", "--account", "+15550001111"])
+        assert seen["endpoint"] == "/v1/groups/%2B15550001111"