fix(wait): stop sharing exit code 2 with Click's usage errors

AGENTS.md

@@ -68,6 +68,8 @@ sp run infra-errors <run>             # VM / checkout / build / storage problems
 sp run artifacts <run>                # binary, coredump, stdout, outputs
 sp queue                              # what is running right now
 sp run progress <run>                 # live status of one run
+sp run wait <run> [<run> ...]         # block until they finish; 0 all passed,
+                                      # 1 something failed, 9 timed out
 sp run ls --pr <n>                    # every run for one pull request
 sp run ls --created-after 2026-08-09T00:00:00Z   # recent activity
 ```
@@ -131,7 +133,9 @@ sp run ls --limit 5   | jq -r '.data[]     | "\(.run_id) \(.platform) \(.status)
 ```
 
 Exit codes to branch on: `0` ok, `3` unreachable, `4` not found, `5` validation,
-`6` auth, `7` rate limited, `8` conflict.
+`6` auth, `7` rate limited, `8` conflict, `9` `run wait` timed out. **`2` means
+you got the command wrong** — an unknown flag, a missing argument, a value out
+of range — so treat it as a bug in your invocation, never as an outcome.
 
 Progress spinners, retry notices, and colour go to **stderr** and are suppressed
 when stdout is not a terminal, so JSON on stdout is always parseable.

README.md

@@ -134,6 +134,7 @@ sp health                            # API + dependency health
 sp queue                             # queue depth and running jobs
 sp run ls                            # list CI runs
 sp run ls --pr 2309                  # ... just one pull request's runs
+sp run wait 9412 9413                # block until they finish; exit code is the verdict
 sp run create --commit <sha> --platform linux --repository owner/repo
 sp sample ls / show / details <id>   # media samples
 sp regression ls / show <id>         # regression-test definitions
@@ -275,6 +276,13 @@ Scripts and agents can branch on the exit status:
 | 6 | authentication / authorization failure |
 | 7 | rate limited |
 | 8 | conflict (e.g. deleting a test that has results) |
+| 9 | `run wait` gave up before every run finished |
+
+`run wait` additionally exits 1 when a run it waited for failed or was
+canceled. Note what is *not* in this table: **2**, which Click returns for any
+usage error — an unknown flag, a missing argument, an out-of-range value. Never
+give 2 a meaning of your own, or a mistyped flag becomes indistinguishable from
+a real outcome.
 
 ## Development
 

sp_cli/commands/run.py

@@ -11,11 +11,11 @@
 from sp_cli.compare import compare_runs, coverage_warnings
 from sp_cli.constants import (ARTIFACT_TYPES, CANCEL_REASON_MIN_LENGTH,
                               COMMIT_SHA_LENGTH, ERROR_GROUP_BY,
-                              ERROR_SEVERITIES, ERROR_TYPES, INFRA_ERROR_TYPES,
-                              LOG_CONTAINS_MAX_LENGTH, LOG_LEVELS, LOG_SOURCES,
-                              MAX_OFFSET, MAX_PAGE_LIMIT,
-                              MAX_REGRESSION_TEST_IDS, PLATFORMS,
-                              PR_SCAN_DEFAULT, PR_SCAN_MAX,
+                              ERROR_SEVERITIES, ERROR_TYPES, EXIT_TIMEOUT,
+                              INFRA_ERROR_TYPES, LOG_CONTAINS_MAX_LENGTH,
+                              LOG_LEVELS, LOG_SOURCES, MAX_OFFSET,
+                              MAX_PAGE_LIMIT, MAX_REGRESSION_TEST_IDS,
+                              PLATFORMS, PR_SCAN_DEFAULT, PR_SCAN_MAX,
                               RUN_PENDING_STATUSES, RUN_STATUSES,
                               RUN_UNSUCCESSFUL_STATUSES, SAMPLE_STATUSES,
                               WAIT_INTERVAL_DEFAULT, WAIT_INTERVAL_MAX,
@@ -777,8 +777,13 @@ def run_wait(ctx: click.Context, run_ids: Tuple[int, ...], interval: int,
     stays pipeable.
 
     Exits 0 only if every run finished successfully; a failed or canceled run
-    exits 1 and a timeout exits 2, which lets a script gate on the result
+    exits 1 and a timeout exits 9, which lets a script gate on the result
     without parsing the output.
+
+    A run whose status is `pass` exits 0 even if it recorded results for only a
+    fraction of its samples, because the API derives that status from the rows
+    that exist rather than from the ones expected. Check `sp run summary`'s
+    skipped_count before treating a green wait as full coverage.
     """
     client = ctx.obj['client']
     output = ctx.obj['output']
@@ -806,7 +811,7 @@ def run_wait(ctx: click.Context, run_ids: Tuple[int, ...], interval: int,
                 click.echo(f'timed out after {timeout}s; still pending: '
                            f'{", ".join(str(r) for r in pending)}', err=True)
             _render_wait(ctx, run_ids, finished, output)
-            raise SystemExit(2)
+            raise SystemExit(EXIT_TIMEOUT)
         if not quiet:
             click.echo(f'waiting on {", ".join(str(r) for r in pending)} '
                        f'({interval}s)', err=True)
@@ -834,5 +839,7 @@ def _render_wait(ctx: click.Context, run_ids: Tuple[int, ...],
     :type output: str
     """
     rows = [finished[run_id] for run_id in dict.fromkeys(run_ids) if run_id in finished]
-    payload: Any = rows[0] if len(rows) == 1 else {'data': rows}
-    render(payload, output, ctx.obj.get('color', False))
+    # Always a collection, even for one run. The shape must not depend on how
+    # many ids the caller passed, or `sp run wait $IDS | jq '.data[]'` works
+    # until the list happens to contain a single run.
+    render({'data': rows, 'total': len(rows)}, output, ctx.obj.get('color', False))

sp_cli/constants.py

@@ -1,3 +1,4 @@
+
 """Enumerations accepted by the platform API, mirrored so the CLI can reject bad input locally.
 
 Each tuple matches a validator in the merged ``mod_api`` blueprint. Keeping them
@@ -29,6 +30,14 @@
 #: exits non-zero on these so a script can gate on it.
 RUN_UNSUCCESSFUL_STATUSES = ('fail', 'canceled', 'error')
 
+#: ``sp run wait`` exits with this when the deadline passes before every run
+#: finishes. Deliberately **not** 2: Click exits 2 on any usage error -- an
+#: unknown flag, a missing argument, an out-of-range ``--interval`` -- so a
+#: script branching on 2 would read a typo as "still running" and retry a
+#: command that never executed. 1 through 8 are taken by ``ApiError.exit_code``
+#: and Click, so the first free code is 9.
+EXIT_TIMEOUT = 9
+
 #: ``sp run wait`` polling bounds, in seconds.
 WAIT_INTERVAL_DEFAULT = 30
 WAIT_INTERVAL_MIN = 5

tests/test_cli.py

@@ -7,6 +7,7 @@
 from click.testing import CliRunner
 
 from sp_cli.client import ApiError
+from sp_cli.constants import EXIT_TIMEOUT
 from sp_cli.main import cli
 from tests import SESSION_SANDBOX  # noqa: F401
 
@@ -1621,15 +1622,52 @@ def test_wait_treats_unknown_status_as_terminal(self, mock_get):
     @mock.patch('sp_cli.commands.run.time.monotonic')
     @mock.patch('sp_cli.client.ApiClient.get')
     def test_wait_times_out(self, mock_get, mock_clock):
-        """Passing the deadline exits 2 and reports what was still pending."""
+        """Passing the deadline exits 9 and reports what was still pending."""
         mock_get.return_value = {'run_id': 9476, 'status': 'queued'}
         # Start, then a reading past the deadline on the first check.
         mock_clock.side_effect = [0, 10_000, 10_000]
         result = self.runner.invoke(cli, ['run', 'wait', '9476', '--timeout', '60'])
 
-        self.assertEqual(result.exit_code, 2)
+        self.assertEqual(result.exit_code, EXIT_TIMEOUT)
         self.assertIn('timed out', result.stderr)
 
+    @mock.patch('sp_cli.client.ApiClient.get')
+    def test_a_timeout_is_distinguishable_from_a_usage_error(self, mock_get):
+        """The reason the timeout code is not 2.
+
+        Click exits 2 on any usage error. Sharing that code would make a
+        mistyped flag indistinguishable from "the runs are still going", so a
+        script branching on it would sleep and retry a command that never ran.
+        """
+        for argv in (['run', 'wait'],                          # no run ids
+                     ['run', 'wait', '--nosuchflag'],          # unknown option
+                     ['run', 'wait', '9476', '--interval', '1']):   # below the minimum
+            result = self.runner.invoke(cli, argv)
+            self.assertEqual(result.exit_code, 2, argv)
+            self.assertNotEqual(result.exit_code, EXIT_TIMEOUT, argv)
+        mock_get.assert_not_called()
+
+    @mock.patch('sp_cli.client.ApiClient.get')
+    def test_one_run_is_rendered_as_a_collection_like_every_other_list(self, mock_get):
+        """The payload shape must not depend on how many ids were passed.
+
+        Returning a bare record for a single run breaks
+        `sp run wait $IDS | jq '.data[]'` exactly when the list happens to hold
+        one run -- the same command, working or not by coincidence.
+        """
+        mock_get.return_value = {'run_id': 9476, 'status': 'pass'}
+        single = json.loads(self.runner.invoke(cli, ['run', 'wait', '9476']).stdout)
+
+        mock_get.side_effect = [{'run_id': 9476, 'status': 'pass'},
+                                {'run_id': 9477, 'status': 'pass'}]
+        several = json.loads(
+            self.runner.invoke(cli, ['run', 'wait', '9476', '9477']).stdout)
+
+        self.assertEqual(single['total'], 1)
+        self.assertEqual(several['total'], 2)
+        for payload in (single, several):
+            self.assertIsInstance(payload['data'], list)
+
     @mock.patch('sp_cli.client.ApiClient.get')
     def test_wait_surfaces_api_errors(self, mock_get):
         """An API failure stops the wait rather than retrying forever."""