c924b0934e45fbbd0d253d09fa285b3e00045397
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  jpmschweitzerandClaude Opus 5
|
c924b0934e |
feat(config): T-1277 — detach, and a failed job that looked busy
core/process.py spawns a child that outlives its parent: its own session, so a signal to the parent's group or a timeout kill does not take the work with it; re-execing reach by BARE NAME, because an absolute path would freeze the child to whichever checkout was current at spawn time and silently run the wrong source after a repoint; and streams kept separate exactly as in the foreground, events to <id>.jsonl and real output to <id>.out. Testing a case the ticket did not name found a real hole. Recording completion inside @command looked right and was wrong: a child that fails BEFORE any command runs — bad arguments, an unknown verb, an import error — never reaches that decorator. `reach --detach check bogus` left its metadata reading "running" forever with the process long gone. That is the exit-0 trap wearing a new disguise and worse than the original, because a failed job that looks busy sits somewhere nobody is watching, and a caller polling for completion would wait indefinitely on something that failed in milliseconds. So completion is recorded at the PROCESS's exit instead. main.py gains main(), wrapping cli() in a single try/finally, and the entry point moves to main:main. Every exit path now passes through one place. Removed from @command rather than left in both — two writers of one field is how they drift. Verified on three paths: success records done/0, a real drift failure records failed/1, and the parse failure that exposed the hole now records failed/2. One narrow conformance exemption, with its reason inline so it does not read as an oversight: the no-domain-imports-core.jobs invariant fired on main.py, correctly by its letter and wrongly by its purpose. main.py is not a command; it is the entry point, and it already owns --detach. Still open, and carried to T-1278: a child killed outright cannot record anything, so jobs list must reconcile against process liveness rather than trusting the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
jpmschweitzerandClaude Opus 5
|
5d83e1d2eb |
feat(config): T-1276 — every invocation is a job, carried ambiently
Streaming as a decorator, first half. Each invocation of reach gets an id and every event it emits is tagged with it, which is what will let a detached run's log be read back and what correlates the lines of a run that streamed for nine minutes. No command signature changed and no command imports core.jobs — that is the point, per the D-263 amendment: a command must not know jobs exist, because the alternative is call-site discipline wearing a different hat. A ContextVar rather than a module global. A global is correct only until something runs two invocations in one process — which a test harness or a future batch verb does immediately, and which would then interleave two jobs' events under one id with nothing reporting an error. The job context is the OUTERMOST wrapper, and it has to be. @logged emits from its finally and @handle_errors emits its verdict while unwinding, so a context established inside either would already be reset by the time the two most important events are written — leaving them the only untagged lines in the log, and they are precisely the ones a detached run gets read back for. Fixed in passing: the job id used local time while every event's ts is UTC, so an id read 155327 beside its own first log line reading 13:53:27. Two hours apart reads as a logging bug every time someone correlates them by eye. New conformance invariant — nothing outside core/ may import core.jobs. My first version of it inspected only the module path, so it missed `from tooling.core import jobs`, where the name is in the import LIST and which is the form anyone would actually write. It passed while checking nothing. Rewritten to catch all three reachable forms and then verified by committing a real violation, which it named by file and line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
jpmschweitzerandClaude Opus 5
|
b5beda0df7 |
feat(config): T-1275 — bare reach is discovery, so it exits 0
Bare `reach` and bare `reach <domain>` printed help and exited 2, Click's usage-error convention. Running reach with no arguments is the DISCOVERY action — it is how the tool gets learned from nothing — and a caller that branches on exit status would read its own onboarding as a failure. Now they exit 0. D-263's exit-code contract is untouched: it governs failures, and printing a command list is not one. Verified across the whole matrix, because this change flirts with the exit-0 trap that record opens with — bare 0, bare domain 0, --help 0, unknown domain 2, unknown verb 2, real failure 1. All five are now pinned as a sixth conformance invariant, since an exit code regresses silently and nothing else would notice. Proven to fail by putting the 2 back. The implementation also collapses a duplicated class. core/cli.py holds ReachGroup with both shared behaviours — no-args-prints-help-and-exits-0, and unknown-name-enumerates — and LazyDomainGroup now extends it instead of subclassing TyperGroup directly, keeping only the laziness and the domain-specific wording. The enumeration logic previously existed twice in slightly different forms, which is how the root and the domains would have drifted into disagreeing about their own conventions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
jpmschweitzerandClaude Opus 5
|
49fa6ada95 |
feat(config): T-1249 — the contract is a decorator, and now a test
Every non-zero exit names the command that would fix it, and still exits non-zero. Both halves matter; the second is the one that gets lost, because a tool that explains itself beautifully and exits 0 looks MORE correct while having silently disabled its own gate. core/errors.py holds ReachError(message, fix=) and @handle_errors. core/logging.py holds @logged, emitting through console rather than a second sink — one output path, so there is nothing to drift. core/command.py composes them, and the order is load-bearing: handle_errors wraps logged, so the logger sees the original exception. Inverted, every failure would be recorded as "SystemExit" and the log would say nothing about what went wrong while looking like it worked. core/ raises SystemExit, not typer.Exit. A service must be callable from a test, another service, or a future second front end, and an exception type that only makes sense inside a CLI leaks the transport into every layer. The check router is retrofitted off its hand-rolled verdict-and-exit pattern — exactly the boilerplate this removes — and test_check_parity.py passes unchanged across the retrofit. That test predates the decorators and pins exit codes against the old script, so it is independent evidence, not a test tuned to match new behaviour. Unknown domains and unknown verbs now enumerate what exists instead of only saying no. That needed a shared group class, which collided with "no typer outside main.py and router.py" — resolved by sharpening the invariant rather than breaking it, since its purpose is that a SERVICE never knows it was called from a CLI. Transport now lives in main.py, router.py and core/cli.py; never in service.py, schemas.py or helpers.py. The upside is that cli.domain() carries the settings that were previously per-router decisions, including the load-bearing rich_markup_mode=None that one forgetful domain could have undone. test_conformance.py makes five invariants executable, AST-based rather than grep. Scoped to the package, not the 123 legacy scripts — and deliberately so: as T-1250 moves each script into domains/, it lands inside the scope and the rules start applying automatically, so the test's reach grows with the migration. Proven to fail before being trusted: removing @command and removing a fix= each produced a failure naming the file, the line and the reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
jpmschweitzerandClaude Opus 5
|
559f3d82dc |
chore(config): T-1258 — tooling/ becomes an importable package
The skeleton the reach CLI hangs off. Nothing moves yet: this adds the package, the bounded core/, and explicit setuptools discovery. core/console.py is the single output path, and the split it enforces is the whole design — stdout carries the command's actual output so `reach ... | jq` keeps working, stderr carries the event stream as JSONL. Rendering happens at the sink: a terminal gets human text, anything else gets raw JSONL, so a live view and a job log are one artefact in two presentations. Emitting is optional — the gates emit nothing — and verdict() prints once, last, carrying its remedy as a structured field. core/config.py resolves the repo root from __file__ against a project.yaml sentinel, with an SR_REPO_ROOT override. No subprocess and no git call: this is on the gate path, and cwd is not a reliable signal anyway since a hook runs from the root and an agent call may not. Both paths are validated, because a silent fallback is how you end up editing one checkout and checking another. Discovery is configured explicitly rather than left to flat-layout auto-discovery, which would have had to choose between erroring on the ambiguity and quietly shipping client/ or docs/. Verified: top_level.txt contains exactly "tooling". Verified beyond the happy path — the sentinel rejects SR_REPO_ROOT=/tmp and names both remedies; debug events are suppressed at the default threshold while the verdict is not; stdout stays clean with stderr redirected away; and the three unconditional push-gate checks still pass now that tooling/ is a package, which was the real regression risk. Two findings recorded on the tickets. make setup-venv is stale — it calls .venv/bin/pip, but the venv was created by uv and has no pip, so the recorded procedure and the actual state have already diverged (T-1261 owns the fix). And settled-reach-tooling had never actually been installed: site-packages held the dependencies but no dist-info, which follows from there being no __init__.py to expose. This is the first commit where `import tooling` means anything. .venv/ was only ignored via .git/info/exclude, which is machine-local, so a fresh clone or a new worktree did not ignore it at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |