Merge pull request #6 from CCExtractor/fix/wait-error-vs-outcome

Separate a failed run from a wait that could not finish

Author: cfsmp3

AGENTS.md

@@ -69,7 +69,8 @@ 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
+                                      # 1 a run failed, 9 timed out,
+                                      # 10 the wait itself broke
 sp run ls --pr <n>                    # every run for one pull request
 sp run ls --created-after 2026-08-09T00:00:00Z   # recent activity
 ```
@@ -133,9 +134,13 @@ 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, `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.
+`6` auth, `7` rate limited, `8` conflict, `9` `run wait` timed out, `10` `run
+wait` broke before it could finish. Under `run wait`, `1` means a run failed or
+was canceled and nothing else does, so `1` is a verdict about the code while
+`9`/`10` say only that the platform misbehaved — block on the first, retry the
+others. **`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

@@ -277,12 +277,26 @@ Scripts and agents can branch on the exit status:
 | 7 | rate limited |
 | 8 | conflict (e.g. deleting a test that has results) |
 | 9 | `run wait` gave up before every run finished |
+| 10 | `run wait` could not finish waiting (API error with no code of its own) |
 
-`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.
+Under `run wait`, **1 means a run you waited for failed or was canceled** — a
+verdict about the code, and the only exit code that is one. Everything that
+went wrong with the wait itself lands on 3–8 or 10. The distinction is what
+lets a merge gate block on a bad branch and retry on a bad afternoon:
+
+```bash
+sp run wait $ID
+case $? in
+  0)     merge ;;
+  1)     block ;;      # the branch is bad
+  9|10)  retry ;;      # the platform is; nothing was learned about the branch
+esac
+```
+
+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

@@ -12,10 +12,11 @@
 from sp_cli.constants import (ARTIFACT_TYPES, CANCEL_REASON_MIN_LENGTH,
                               COMMIT_SHA_LENGTH, ERROR_GROUP_BY,
                               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,
+                              EXIT_WAIT_ABORTED, 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,
@@ -778,7 +779,9 @@ def run_wait(ctx: click.Context, run_ids: Tuple[int, ...], interval: int,
 
     Exits 0 only if every run finished successfully; a failed or canceled run
     exits 1 and a timeout exits 9, which lets a script gate on the result
-    without parsing the output.
+    without parsing the output. If the wait itself cannot be completed the exit
+    code describes that instead: 3 through 8 for an API error that maps to one,
+    10 for anything else. Nothing but a run's own verdict exits 1.
 
     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
@@ -797,7 +800,12 @@ def run_wait(ctx: click.Context, run_ids: Tuple[int, ...], interval: int,
                 record = client.get(f'/runs/{run_id}')
             except ApiError as error:
                 render_error(error, output)
-                raise SystemExit(error.exit_code)
+                # An error with no code of its own exits 1 everywhere else, but
+                # this command has already given 1 a meaning: a run you waited
+                # for failed. Collapsing both into 1 tells a merge gate to block
+                # the branch when the platform merely returned a 500.
+                code = error.exit_code
+                raise SystemExit(EXIT_WAIT_ABORTED if code == 1 else code)
             status = record.get('status')
             if status not in RUN_PENDING_STATUSES:
                 finished[run_id] = record

sp_cli/constants.py

@@ -1,4 +1,3 @@
-
 """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
@@ -38,6 +37,14 @@
 #: and Click, so the first free code is 9.
 EXIT_TIMEOUT = 9
 
+#: ``sp run wait`` exits with this when the wait itself could not be completed:
+#: an API failure that has no more specific code of its own (a 500, a malformed
+#: response). Distinct from 1, which this command spends on "a run you waited
+#: for failed" -- a merge gate has to tell a bad branch from a platform hiccup,
+#: because the first should block and the second should be retried. Errors that
+#: do map to a code of their own (3 through 8) keep it.
+EXIT_WAIT_ABORTED = 10
+
 #: ``sp run wait`` polling bounds, in seconds.
 WAIT_INTERVAL_DEFAULT = 30
 WAIT_INTERVAL_MIN = 5

tests/test_cli.py

@@ -7,7 +7,7 @@
 from click.testing import CliRunner
 
 from sp_cli.client import ApiError
-from sp_cli.constants import EXIT_TIMEOUT
+from sp_cli.constants import EXIT_TIMEOUT, EXIT_WAIT_ABORTED
 from sp_cli.main import cli
 from tests import SESSION_SANDBOX  # noqa: F401
 
@@ -1675,3 +1675,31 @@ def test_wait_surfaces_api_errors(self, mock_get):
         result = self.runner.invoke(cli, ['run', 'wait', '9476'])
 
         self.assertNotEqual(result.exit_code, 0)
+
+    @mock.patch('sp_cli.client.ApiClient.get')
+    def test_a_broken_wait_is_distinguishable_from_a_failed_run(self, mock_get):
+        """1 means a run failed, never that the wait itself broke.
+
+        A merge gate blocks on 1. If an API failure with no code of its own also
+        exited 1, a 500 from the platform would read as "this branch is bad" and
+        block a branch nothing is wrong with -- and retrying, the right response
+        to a 500, is the one thing the script would not do.
+        """
+        mock_get.side_effect = ApiError('server_error', 'boom', status=500)
+        broke = self.runner.invoke(cli, ['run', 'wait', '9476'])
+
+        mock_get.side_effect = None
+        mock_get.return_value = {'run_id': 9476, 'status': 'fail'}
+        failed = self.runner.invoke(cli, ['run', 'wait', '9476'])
+
+        self.assertEqual(broke.exit_code, EXIT_WAIT_ABORTED)
+        self.assertEqual(failed.exit_code, 1)
+        self.assertNotEqual(broke.exit_code, failed.exit_code)
+
+    @mock.patch('sp_cli.client.ApiClient.get')
+    def test_api_errors_that_map_to_a_code_keep_it(self, mock_get):
+        """Only the codeless errors are remapped; 3-8 still mean what they mean."""
+        for status, expected in ((404, 4), (401, 6), (429, 7), (409, 8)):
+            mock_get.side_effect = ApiError('err', 'nope', status=status)
+            result = self.runner.invoke(cli, ['run', 'wait', '9476'])
+            self.assertEqual(result.exit_code, expected, f'HTTP {status}')