diff --git a/AGENTS.md b/AGENTS.md index 655b38b..72b91e5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,7 +69,8 @@ sp run artifacts # binary, coredump, stdout, outputs sp queue # what is running right now sp run progress # live status of one run sp run wait [ ...] # 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 # 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. diff --git a/README.md b/README.md index 5ebeac6..c6950b8 100644 --- a/README.md +++ b/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 diff --git a/sp_cli/commands/run.py b/sp_cli/commands/run.py index 7c55654..a2edccf 100644 --- a/sp_cli/commands/run.py +++ b/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 diff --git a/sp_cli/constants.py b/sp_cli/constants.py index 80eb94c..6d540e4 100644 --- a/sp_cli/constants.py +++ b/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 diff --git a/tests/test_cli.py b/tests/test_cli.py index a3c9de2..03e3156 100644 --- a/tests/test_cli.py +++ b/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}')