From 3b211a5450db6e8a3baf17d26cb547a83110134b Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 31 Aug 2026 17:14:23 +0200 Subject: [PATCH] =?UTF-8?q?feat(config):=20T-1279=20=E2=80=94=20a=20detach?= =?UTF-8?q?ed=20failure=20reaches=20its=20caller?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The non-negotiable from D-263, pointed at its worst hiding place: a foreground command that swallows a failure at least does it in front of someone, while a background runner that reports "started" and loses the failure does it where nothing is watching. Testing the two timing cases the ticket names — fails before the parent exits, fails long after — needs a command slow enough to tell them apart, and every verb in reach finishes in milliseconds. So `reach dev selftest` exists: emits progress for N seconds, then optionally fails with a chosen code. A genuine diagnostic rather than a test hook, in the dev domain the map already planned, and the only way to answer "does streaming work here, can I tail it, does a failure survive detach" by observation instead of argument. The slow case is the one that proves the design. --detach returned in 75ms while the child ran six seconds, so the parent was demonstrably gone long before the child failed — and wait still relayed exit 7. That is the half of the recording path only this case reaches, and why T-1277 moved completion recording into the child. Also pinned: --detach exits 0 for starting and SAYS "not succeeded" in words, which the test asserts on rather than trusting the code to be read correctly; a failed job nobody waited on shows as failed in jobs list; and every event a detached job emits carries its job id. Closed T-1278's open gap in passing — jobs log --follow had never run against a genuinely long job because none existed. It now has: attached mid-flight, streamed the remaining steps live, and caught the final verdict after the job ended. Proven to fail by making effective_exit_code always return 0 — the trap itself. Both timing cases failed by name. Co-Authored-By: Claude Opus 5 (1M context) --- .pql/changelog/ticket_history/2026-08.sql | 22 +++ .pql/changelog/tickets/2026-08.sql | 44 ++++++ Makefile | 4 + tooling/domains/dev/__init__.py | 9 ++ tooling/domains/dev/router.py | 48 +++++++ tooling/domains/dev/service.py | 20 +++ tooling/main.py | 4 + tooling/test_job_exit_codes.py | 166 ++++++++++++++++++++++ 8 files changed, 317 insertions(+) create mode 100644 tooling/domains/dev/__init__.py create mode 100644 tooling/domains/dev/router.py create mode 100644 tooling/domains/dev/service.py create mode 100644 tooling/test_job_exit_codes.py diff --git a/.pql/changelog/ticket_history/2026-08.sql b/.pql/changelog/ticket_history/2026-08.sql index d04f4bd96..5a21fd23b 100644 --- a/.pql/changelog/ticket_history/2026-08.sql +++ b/.pql/changelog/ticket_history/2026-08.sql @@ -1961,3 +1961,25 @@ SERVICE-LEVEL TESTS — tooling/test_jobs.py, and these close the gap T-1257 nam read_events was split into read_events(job_id) and read_events_from(path) so a test can drive it against a temp file — better shape regardless. NOT YET EXERCISED, and honest about it: `jobs log --follow` against a genuinely LONG-running job. Nothing in reach currently runs long enough to tail. The offset mechanics underneath it are tested directly, and the live loop gets its first real workout when a slow domain lands (planet or db, T-1250).', NULL, '2026-08-31 15:02:20', '2026-08-31 15:02:20.709', '2026-08-31 15:02:20.709', NULL, 'becbc04f4ea86ae9f73e0c9541992300', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5VJ89C7Q4EPR6Q73FSS74', 'status', 'in_progress', 'done', NULL, '2026-08-31 15:02:40', '2026-08-31 15:02:40.198', '2026-08-31 15:02:40.198', NULL, '005d8c2b4c8e970ae5d4b33ab9bb15b5', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5WNGDMENP7AY9VMB5G3XM', 'status', 'backlog', 'in_progress', NULL, '2026-08-31 15:06:54', '2026-08-31 15:06:54.261', '2026-08-31 15:06:54.261', NULL, '85a6301b9102a7cd71655fa52a02963f', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5WNGDMENP7AY9VMB5G3XM', 'status', 'in_progress', 'in_progress', NULL, '2026-08-31 15:07:19', '2026-08-31 15:07:19.025', '2026-08-31 15:07:19.025', NULL, '8aa6e2e845e5dc18acf8c627dd14c52d', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5WNGDMENP7AY9VMB5G3XM', 'description', 'A runner that reports started and then loses the failure is the exit-0 trap from the top of D-263 relocated somewhere nothing is watching, which is strictly worse than the original: at least a foreground exit 0 is in front of someone. Requirements: reach jobs wait EXITS WITH THE JOB EXIT CODE, so a Makefile or hook can gate on a detached run exactly as it would on a foreground one; a failed job that nobody waited on is visible as failed in reach jobs list rather than merely absent; and reach --detach itself exits 0 for SUCCESSFULLY STARTING, which is a different claim from the job succeeding, so the output must say so in words rather than leaving a reader to infer it. TEST THE FAILING PATH FIRST and give it a deliberately failing command, since a job runner that has only ever run successful jobs has never been tested — this is the same discipline that caught the gate tests. Also test the case where the job fails BEFORE the parent has exited, and where it fails long AFTER, since those exercise different halves of the recording path.', 'A runner that reports started and then loses the failure is the exit-0 trap from the top of D-263 relocated somewhere nothing is watching, which is strictly worse than the original: at least a foreground exit 0 is in front of someone. Requirements: reach jobs wait EXITS WITH THE JOB EXIT CODE, so a Makefile or hook can gate on a detached run exactly as it would on a foreground one; a failed job that nobody waited on is visible as failed in reach jobs list rather than merely absent; and reach --detach itself exits 0 for SUCCESSFULLY STARTING, which is a different claim from the job succeeding, so the output must say so in words rather than leaving a reader to infer it. TEST THE FAILING PATH FIRST and give it a deliberately failing command, since a job runner that has only ever run successful jobs has never been tested — this is the same discipline that caught the gate tests. Also test the case where the job fails BEFORE the parent has exited, and where it fails long AFTER, since those exercise different halves of the recording path. + +DONE 2026-08-31. All four properties verified and pinned in tooling/test_job_exit_codes.py, wired into make test-tooling. + +BUILT A REAL VERB TO TEST WITH, rather than a test hook. The two timing cases this ticket names — fails BEFORE the parent exits, fails long AFTER — cannot be distinguished when every command in reach finishes in milliseconds. So `reach dev selftest --seconds N --fail --exit-code C` now exists: it emits progress events over N seconds then optionally fails. It is a genuine diagnostic ("does streaming work end to end on this machine, can I tail it, does a failure survive detach?"), it lives in the `dev` domain the map already planned, and it is the only command slow enough to answer those by observation instead of argument. A test-only hook would have been the same code with less value. + +VERIFIED, and the slow case is the one that proves the design: +- FAST failure (0s): wait relays exit 3. +- SLOW failure (6s real / 2s in the test): --detach returned in 75 ms while the child ran 6 seconds, so the parent was demonstrably gone long before the child failed — and wait still relayed exit 7. That is the half of the recording path only this case reaches, and it is why T-1277 moved completion recording to the child. +- SUCCESS: wait relays 0. +- A failed job nobody waited on shows as `failed` in jobs list with its full command. +- --detach exits 0 AND says "this exit status means STARTED, not succeeded" in words. The test asserts on that wording, not just the code, because a bare 0 invites exactly the wrong reading. +- Every event in a detached job''s log carries the job id. + +CLOSED T-1278''s OPEN GAP as a side effect. `jobs log --follow` had never been exercised against a genuinely long job because none existed. With selftest it now has been: attached to a running 6-second job mid-flight, streamed the remaining steps live as they happened, and caught the final failure verdict with its remedy after the job ended. The follow loop and its final post-completion read both work. + +PROVEN TO FAIL: made effective_exit_code always return 0 — the exit-0 trap itself — and both the fast and slow cases failed by name, reporting the expected code against the 0 they got. Restored, green. + +NOTE ON WHAT --follow RETURNS: `jobs log --follow` exits 0 even when the job it tailed failed. That is correct and deliberate — log succeeded at logging. `wait` is the verb that relays an outcome, and conflating the two would mean you could not read a failed job''s log without your shell treating the read as a failure.', NULL, '2026-08-31 15:14:09', '2026-08-31 15:14:09.188', '2026-08-31 15:14:09.188', NULL, 'a48443a46dc1cc27d6089e81695a0b5c', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-08.sql b/.pql/changelog/tickets/2026-08.sql index bdb4e81d2..0f4daf198 100644 --- a/.pql/changelog/tickets/2026-08.sql +++ b/.pql/changelog/tickets/2026-08.sql @@ -2287,3 +2287,47 @@ SERVICE-LEVEL TESTS — tooling/test_jobs.py, and these close the gap T-1257 nam read_events was split into read_events(job_id) and read_events_from(path) so a test can drive it against a temp file — better shape regardless. NOT YET EXERCISED, and honest about it: `jobs log --follow` against a genuinely LONG-running job. Nothing in reach currently runs long enough to tail. The offset mechanics underneath it are tested directly, and the live loop gets its first real workout when a slow domain lands (planet or db, T-1250).', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 13:52:06.722', '2026-08-31 15:02:20.709', NULL, '1ce1eb66a71783ab5218d27ae143b1aa', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5VJ89C7Q4EPR6Q73FSS74', 'task', '06G1S7NVJR0GT9KWS9QVYNNFMM', 'The jobs domain — list, status, log --follow, wait', 'The user-facing verbs, and a DOMAIN rather than core/ because they carry logic and state of their own — the first real test of the D-263 core bound, which it passes. Deliverables: reach jobs list (recent jobs with status, command and duration), status , log with --follow to tail, and wait . REATTACH IS A BYTE OFFSET into an append-only file, which is the entire reason no daemon is needed: a caller can attach, drop off, and come back without losing anything, and there is no lifecycle to get wrong, nothing to orphan, and no stale state to reconcile. log --follow is therefore a poll on file length, not a subscription. Render the JSONL through the same path a live terminal uses, so a tailed log and a live run are the same artefact in two presentations rather than two renderers that drift. Note for the port: this domain is the first one written from scratch under the full contract rather than ported, so it doubles as the worked example the reach skill (T-1254) should show. + +FROM T-1277 (2026-08-31) — the reconciliation requirement is yours and it is not optional. A child killed outright (SIGKILL, OOM, an interpreter crash) never gets to record its own completion, so its metadata file stays status=running with the process long gone. jobs list and jobs status must therefore RECONCILE against process.is_alive(pid) rather than trusting the file: a job whose pid is dead and whose status still reads running is not running, it died. Report it as such — ''died without recording an exit'' is honest and actionable, whereas showing it as running is the exit-0 trap in a place nobody is watching, and a caller polling for completion would wait forever on something that failed in milliseconds. Note the ordinary failure paths are already covered: T-1277 moved completion recording to the process''s exit (tooling/main.py main()), so bad arguments, unknown verbs, ReachErrors and unhandled exceptions all record correctly. What remains is only the case where the process cannot run code at all. + +DONE 2026-08-31. reach jobs list / status / log --follow / wait, all four working, plus service-level tests. + +A LATENT BUG FOUND BEFORE BUILDING ON IT, and it was already committed. typer.Exit is a RuntimeError, NOT a SystemExit — so @handle_errors caught it like any other unexpected exception. Verified: `raise typer.Exit(3)` inside a decorated command printed "unexpected Exit: 3" and exited 1, SILENTLY DISCARDING the requested code. Nothing hit it today because T-1267 had converted the check router to ReachError, but `jobs wait` needs exactly this and it is the natural thing anyone would write. +FIX: core/errors.ReachExit(code) as the sanctioned control-flow exit, passed straight through by handle_errors as SystemExit with no verdict. ReachError would have been wrong twice over for wait — it prints a failure verdict for a command that worked, and demands a fix= for a situation with no remedy. Kept typer out of core/, which the conformance test enforces. + +RECONCILIATION WORKS, AND WAS PROVEN ON A REAL CORPSE rather than a simulated one. The job left stranded by the T-1277 bug — status "running", process long gone — now reports as `died` with its true elapsed time. Status DIED is derived, never recorded: a process killed outright cannot write its own ending. + +EXIT-CODE RELAY, all three cases: done -> 0, failed -> 2 (the job''s own code), died -> 137. A died job must NEVER relay 0; it has no code of its own and borrowing success is the exit-0 trap pointed at whatever gated on the run. + +BYTE-OFFSET REATTACH verified directly: first read consumed one event to offset 169; resuming from 169 returned nothing and left the offset unmoved. That is the whole reason no daemon is needed. + +SECOND UTC BUG, same class as T-1276''s. jobs list reported a job started minutes earlier as running for 133m — _parse used time.mktime, which reads a UTC stamp as local time and silently adds the offset to every duration. Now calendar.timegm. Both directions of this conversion have now bitten once each; both have the reason in a comment. + +RENDERING: console.render() is now public and jobs log replays stored events through it. A second renderer for stored events would drift from the live one, and the divergence would surface exactly when someone is reading a log to work out what went wrong. + +SERVICE-LEVEL TESTS — tooling/test_jobs.py, and these close the gap T-1257 named. D-263 claims a service is transport-agnostic and callable without a CLI round trip; nothing had ever demonstrated it, which made the layering unverified decoration. Every test here imports service and calls a function directly. Covers the partial trailing line (a reader arriving mid-append must leave the incomplete line for next time, not discard the one event being written when it looked), offset stability, dead-pid reconciliation, and died-never-relays-0. Proven to fail: making a died job return 0 tripped the exit-0 assertion by name. +read_events was split into read_events(job_id) and read_events_from(path) so a test can drive it against a temp file — better shape regardless. + +NOT YET EXERCISED, and honest about it: `jobs log --follow` against a genuinely LONG-running job. Nothing in reach currently runs long enough to tail. The offset mechanics underneath it are tested directly, and the live loop gets its first real workout when a slow domain lands (planet or db, T-1250).', 'done', 'high', NULL, NULL, 'D-263', '2026-08-31 13:52:06.722', '2026-08-31 15:02:40.198', NULL, '9516817a097607051c57cedd37210d22', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5WNGDMENP7AY9VMB5G3XM', 'task', '06G1S7NVJR0GT9KWS9QVYNNFMM', 'Exit codes survive a detached run — the non-negotiable', 'A runner that reports started and then loses the failure is the exit-0 trap from the top of D-263 relocated somewhere nothing is watching, which is strictly worse than the original: at least a foreground exit 0 is in front of someone. Requirements: reach jobs wait EXITS WITH THE JOB EXIT CODE, so a Makefile or hook can gate on a detached run exactly as it would on a foreground one; a failed job that nobody waited on is visible as failed in reach jobs list rather than merely absent; and reach --detach itself exits 0 for SUCCESSFULLY STARTING, which is a different claim from the job succeeding, so the output must say so in words rather than leaving a reader to infer it. TEST THE FAILING PATH FIRST and give it a deliberately failing command, since a job runner that has only ever run successful jobs has never been tested — this is the same discipline that caught the gate tests. Also test the case where the job fails BEFORE the parent has exited, and where it fails long AFTER, since those exercise different halves of the recording path.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 13:52:15.747', '2026-08-31 15:06:54.261', NULL, '4ca7d9770f698bf0ecd7e925b624d916', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5WNGDMENP7AY9VMB5G3XM', 'task', '06G1S7NVJR0GT9KWS9QVYNNFMM', 'Exit codes survive a detached run — the non-negotiable', 'A runner that reports started and then loses the failure is the exit-0 trap from the top of D-263 relocated somewhere nothing is watching, which is strictly worse than the original: at least a foreground exit 0 is in front of someone. Requirements: reach jobs wait EXITS WITH THE JOB EXIT CODE, so a Makefile or hook can gate on a detached run exactly as it would on a foreground one; a failed job that nobody waited on is visible as failed in reach jobs list rather than merely absent; and reach --detach itself exits 0 for SUCCESSFULLY STARTING, which is a different claim from the job succeeding, so the output must say so in words rather than leaving a reader to infer it. TEST THE FAILING PATH FIRST and give it a deliberately failing command, since a job runner that has only ever run successful jobs has never been tested — this is the same discipline that caught the gate tests. Also test the case where the job fails BEFORE the parent has exited, and where it fails long AFTER, since those exercise different halves of the recording path.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 13:52:15.747', '2026-08-31 15:07:19.024', NULL, '3621fd2c6aa8fb8791b303ea74584f08', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5G5WNGDMENP7AY9VMB5G3XM', 'task', '06G1S7NVJR0GT9KWS9QVYNNFMM', 'Exit codes survive a detached run — the non-negotiable', 'A runner that reports started and then loses the failure is the exit-0 trap from the top of D-263 relocated somewhere nothing is watching, which is strictly worse than the original: at least a foreground exit 0 is in front of someone. Requirements: reach jobs wait EXITS WITH THE JOB EXIT CODE, so a Makefile or hook can gate on a detached run exactly as it would on a foreground one; a failed job that nobody waited on is visible as failed in reach jobs list rather than merely absent; and reach --detach itself exits 0 for SUCCESSFULLY STARTING, which is a different claim from the job succeeding, so the output must say so in words rather than leaving a reader to infer it. TEST THE FAILING PATH FIRST and give it a deliberately failing command, since a job runner that has only ever run successful jobs has never been tested — this is the same discipline that caught the gate tests. Also test the case where the job fails BEFORE the parent has exited, and where it fails long AFTER, since those exercise different halves of the recording path. + +DONE 2026-08-31. All four properties verified and pinned in tooling/test_job_exit_codes.py, wired into make test-tooling. + +BUILT A REAL VERB TO TEST WITH, rather than a test hook. The two timing cases this ticket names — fails BEFORE the parent exits, fails long AFTER — cannot be distinguished when every command in reach finishes in milliseconds. So `reach dev selftest --seconds N --fail --exit-code C` now exists: it emits progress events over N seconds then optionally fails. It is a genuine diagnostic ("does streaming work end to end on this machine, can I tail it, does a failure survive detach?"), it lives in the `dev` domain the map already planned, and it is the only command slow enough to answer those by observation instead of argument. A test-only hook would have been the same code with less value. + +VERIFIED, and the slow case is the one that proves the design: +- FAST failure (0s): wait relays exit 3. +- SLOW failure (6s real / 2s in the test): --detach returned in 75 ms while the child ran 6 seconds, so the parent was demonstrably gone long before the child failed — and wait still relayed exit 7. That is the half of the recording path only this case reaches, and it is why T-1277 moved completion recording to the child. +- SUCCESS: wait relays 0. +- A failed job nobody waited on shows as `failed` in jobs list with its full command. +- --detach exits 0 AND says "this exit status means STARTED, not succeeded" in words. The test asserts on that wording, not just the code, because a bare 0 invites exactly the wrong reading. +- Every event in a detached job''s log carries the job id. + +CLOSED T-1278''s OPEN GAP as a side effect. `jobs log --follow` had never been exercised against a genuinely long job because none existed. With selftest it now has been: attached to a running 6-second job mid-flight, streamed the remaining steps live as they happened, and caught the final failure verdict with its remedy after the job ended. The follow loop and its final post-completion read both work. + +PROVEN TO FAIL: made effective_exit_code always return 0 — the exit-0 trap itself — and both the fast and slow cases failed by name, reporting the expected code against the 0 they got. Restored, green. + +NOTE ON WHAT --follow RETURNS: `jobs log --follow` exits 0 even when the job it tailed failed. That is correct and deliberate — log succeeded at logging. `wait` is the verb that relays an outcome, and conflating the two would mean you could not read a failed job''s log without your shell treating the read as a failure.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 13:52:15.747', '2026-08-31 15:14:09.188', NULL, '3bd99b15e59df3b447ab404740ee63fa', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; diff --git a/Makefile b/Makefile index ac377e721..98ecffdca 100644 --- a/Makefile +++ b/Makefile @@ -307,6 +307,10 @@ test-tooling: @mkdir -p .cache @$(VENV_PY) tooling/test_lazy_domains.py 2> .cache/test-tooling-lazy-domains.log || \ { echo " FAIL: reach lazy registration — log follows:"; cat .cache/test-tooling-lazy-domains.log; exit 1; } + @echo " [test-tooling] reach detached-job exit codes (T-1279)..." + @mkdir -p .cache + @$(VENV_PY) tooling/test_job_exit_codes.py 2> .cache/test-tooling-job-exits.log || \ + { echo " FAIL: detached exit codes — log follows:"; cat .cache/test-tooling-job-exits.log; exit 1; } @echo " [test-tooling] reach jobs service units (T-1278)..." @mkdir -p .cache @$(VENV_PY) tooling/test_jobs.py 2> .cache/test-tooling-jobs.log || \ diff --git a/tooling/domains/dev/__init__.py b/tooling/domains/dev/__init__.py new file mode 100644 index 000000000..b79362a07 --- /dev/null +++ b/tooling/domains/dev/__init__.py @@ -0,0 +1,9 @@ +"""The `dev` domain — developer environment and self-diagnosis. + +Per the domain map, this eventually holds install-rust, install-godot, +worktree-setup, perf-baseline and clerk-review. It starts with `selftest`, +which exists because the streaming and job machinery had nothing slow enough to +exercise: every other command in reach finishes in milliseconds, so "does a long +run stream, and can I tail it, and does its failure survive?" was unanswerable +except by argument. +""" diff --git a/tooling/domains/dev/router.py b/tooling/domains/dev/router.py new file mode 100644 index 000000000..1745f9205 --- /dev/null +++ b/tooling/domains/dev/router.py @@ -0,0 +1,48 @@ +"""Transport for the `dev` domain — args in, delegate, format out.""" + +from __future__ import annotations + +import typer + +from tooling.core import cli, console +from tooling.core.command import command +from tooling.core.errors import ReachError +from tooling.domains.dev import service + +app = cli.domain("dev", "Developer environment and self-diagnosis.") + + +@app.callback() +def _domain() -> None: + """Keeps `dev` a group (Typer collapses a single-command app).""" + + +@app.command("selftest") +@command +def selftest( + seconds: float = typer.Option(2.0, "--seconds", "-s", help="Roughly how long to run."), + fail: bool = typer.Option(False, "--fail", help="Exit non-zero at the end."), + exit_code: int = typer.Option(1, "--exit-code", help="Which code to fail with."), +) -> None: + """Emit progress for a while, then succeed or fail on purpose. + + A real diagnostic — "does streaming work end to end on this machine, can I + tail it, does a failure survive a detached run?" — and the only command in + reach slow enough to answer those by observation rather than argument. + Every other verb finishes in milliseconds. + """ + for step, total in service.slow_work(seconds): + console.event( + f"step {step} of {total}", + phase="selftest", + progress=step / total, + ) + + if fail: + raise ReachError( + f"selftest failed on purpose after {seconds:g}s", + fix="this command failed because --fail was passed; drop it to succeed", + exit_code=exit_code, + ) + + console.verdict(f"selftest: OK — {seconds:g}s of progress, no failures") diff --git a/tooling/domains/dev/service.py b/tooling/domains/dev/service.py new file mode 100644 index 000000000..2323ef80d --- /dev/null +++ b/tooling/domains/dev/service.py @@ -0,0 +1,20 @@ +"""Logic for the `dev` domain. Transport-agnostic (D-263).""" + +from __future__ import annotations + +import time +from collections.abc import Iterator + + +def slow_work(seconds: float, steps: int = 10) -> Iterator[tuple[int, int]]: + """Yield (step, total) over roughly `seconds`, so a caller can report progress. + + A generator rather than something that takes a callback, because the service + must not know how progress is reported — that is the router's business, and + handing the service a printer is how a service learns it lives in a CLI. + """ + steps = max(1, steps) + interval = max(0.0, seconds) / steps + for step in range(1, steps + 1): + time.sleep(interval) + yield step, steps diff --git a/tooling/main.py b/tooling/main.py index aed46f166..0f3151a3b 100644 --- a/tooling/main.py +++ b/tooling/main.py @@ -55,6 +55,10 @@ DOMAINS: dict[str, tuple[str, str]] = { "tooling.domains.jobs.router:app", "Detached runs — status, logs and outcomes", ), + "dev": ( + "tooling.domains.dev.router:app", + "Developer environment and self-diagnosis", + ), } diff --git a/tooling/test_job_exit_codes.py b/tooling/test_job_exit_codes.py new file mode 100644 index 000000000..ba339ddf6 --- /dev/null +++ b/tooling/test_job_exit_codes.py @@ -0,0 +1,166 @@ +#!/usr/bin/env python3 +"""A detached job's exit code must survive (T-1279). + +This is the non-negotiable in D-263, pointed at its worst hiding place. A +foreground command that swallows a failure at least does it in front of someone. +A background runner that reports "started" and then loses the failure does it +where nobody is looking, and whatever gated on the run carries on as if it had +passed. + +Four properties, and the ordering of the cases matters: + + 1. `jobs wait` exits with the JOB's code, so a Makefile or hook can gate on a + detached run exactly as on a foreground one. + 2. It works whether the job fails almost immediately or long after the parent + has exited. Those are different halves of the recording path: the fast case + can finish while the parent is still alive, the slow case certainly cannot, + and only the second proves the child records its own ending. + 3. `--detach` exits 0 for STARTING, which is a different claim from the job + succeeding — so it has to say so in words rather than leave a reader to + infer it from a 0. + 4. A failed job nobody waited on is visible as failed, not merely absent. + +The failing cases come first deliberately. A job runner that has only ever run +successful jobs has never been tested. + +Run: python3 tooling/test_job_exit_codes.py +""" + +import json +import re +import shutil +import subprocess +import sys +import time +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +JOB_ID = re.compile(r"^\d{8}T\d{6}-[0-9a-f]{4}$") + + +def _reach(*args: str) -> subprocess.CompletedProcess[str]: + """Invoke by BARE NAME — never a path or an interpreter (T-1261).""" + return subprocess.run( + ["reach", *args], capture_output=True, text=True, cwd=REPO_ROOT + ) + + +def _detach(*args: str) -> tuple[str, subprocess.CompletedProcess[str], float]: + started = time.monotonic() + result = _reach("--detach", *args) + elapsed = time.monotonic() - started + job_id = result.stdout.strip().splitlines()[0] if result.stdout.strip() else "" + return job_id, result, elapsed + + +def test_failure_survives(failures: list[str], seconds: str, code: str, label: str) -> None: + """A job that fails is waited on and relays its own code.""" + job_id, spawn, elapsed = _detach( + "dev", "selftest", "--seconds", seconds, "--fail", "--exit-code", code + ) + + if not JOB_ID.match(job_id): + failures.append(f"[{label}] --detach did not print a job id, got {job_id!r}") + return + if spawn.returncode != 0: + failures.append( + f"[{label}] --detach exited {spawn.returncode}; starting a job succeeded, " + "so the launch itself must report 0" + ) + + # Property 3: the 0 above must not be mistakable for "the job succeeded". + if "not succeeded" not in spawn.stderr: + failures.append( + f"[{label}] --detach's output does not distinguish STARTED from SUCCEEDED " + "in words; a bare 0 invites exactly the wrong reading" + ) + + # Property 2, slow case: the parent must be long gone before the child ends. + if label == "slow" and elapsed > float(seconds) / 2: + failures.append( + f"[{label}] --detach took {elapsed:.2f}s for a {seconds}s job — it waited, " + "which means this case is not testing what it claims to" + ) + + waited = _reach("jobs", "wait", job_id) + if waited.returncode != int(code): + failures.append( + f"[{label}] jobs wait exited {waited.returncode}, expected {code} — " + "the failure was lost between the job and its caller, which is the " + "exit-0 trap in the place nobody watches" + ) + + # Property 4: visible as failed without anyone having waited. + listed = _reach("jobs", "list", "--limit", "20") + row = [line for line in listed.stdout.splitlines() if job_id in line] + if not row: + failures.append(f"[{label}] the job is absent from jobs list") + elif "failed" not in row[0]: + failures.append( + f"[{label}] jobs list does not show the job as failed: {row[0]!r}" + ) + + +def test_success_relays_zero(failures: list[str]) -> None: + job_id, spawn, _ = _detach("dev", "selftest", "--seconds", "0") + if spawn.returncode != 0: + failures.append(f"[success] --detach exited {spawn.returncode}") + return + waited = _reach("jobs", "wait", job_id) + if waited.returncode != 0: + failures.append( + f"[success] jobs wait exited {waited.returncode} for a job that succeeded" + ) + + +def test_events_are_correlated(failures: list[str]) -> None: + """Every event a detached job emits carries its job id.""" + job_id, _, _ = _detach("dev", "selftest", "--seconds", "0") + _reach("jobs", "wait", job_id) + + log = REPO_ROOT / ".cache" / "reach" / "jobs" / f"{job_id}.jsonl" + if not log.is_file(): + failures.append("[correlation] no log file was written for a detached job") + return + for line in log.read_text(encoding="utf-8").splitlines(): + if not line.strip(): + continue + event = json.loads(line) + if event.get("job") != job_id: + failures.append( + f"[correlation] an event carries job={event.get('job')!r}, expected " + f"{job_id!r} — an untagged line cannot be attributed when logs interleave" + ) + return + + +def main() -> int: + if shutil.which("reach") is None: + print( + "test_job_exit_codes: `reach` is not on PATH.\n Fix: make install-reach", + file=sys.stderr, + ) + return 1 + + failures: list[str] = [] + # Failing paths first — see the module docstring. + test_failure_survives(failures, seconds="0", code="3", label="fast") + test_failure_survives(failures, seconds="2", code="7", label="slow") + test_success_relays_zero(failures) + test_events_are_correlated(failures) + + if failures: + print("test_job_exit_codes: FAIL", file=sys.stderr) + for failure in failures: + print(f" - {failure}", file=sys.stderr) + return 1 + print( + "test_job_exit_codes: OK — codes relay through detach for fast and slow " + "failures, start is distinguished from success, events stay correlated" + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main())