Repository navigation
feat(lifecycle): stream --progress ndjson from up, down and restart - #853
Merged
Merged
Conversation
`hack up|down|restart --json --progress` now writes an ndjson stream on
the Compose backend: a start line carrying the
hack.lifecycle-progress.v1 schema, progress lines
{type, ts, service, phase}, and the ordinary --json envelope as one
compact closing line. Phases come only from signals the Compose path
already observes: the detached startup phase callback (images,
starting), down/restart stop (stopping, stopped), dependency cache
markers, and the envelope's final per-service states.
--progress requires --json. The native backend and native-authored
projects refuse it before any work. Refusals before the stream starts
keep the single pretty envelope.
Documented in docs/reference/lifecycle-progress.md (HACK-2218 R6).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An exception that escapes after the start line is rendered as the stream's compact closing envelope instead of the global pretty one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Author
|
@codex review |
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
SIGINT/SIGTERM after start now ends the stream with one compact E_CANCELLED envelope while lifecycle cleanup and signal exit stay as before; a closed stream swallows later envelopes. Remote (dispatch) targets refuse --progress before dispatch, since dispatch output would precede the start line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Install the signal handlers when the stream is armed rather than at start, and write stream lines synchronously, so a signal right after start cannot kill the process before the closing envelope is written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
roodboi
marked this pull request as ready for review
November 14, 2026 18:24
Contributor
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Author
|
Ready for lead: dfa6af8 Review: one persistent Codex reviewer (gpt-6.1-sol, codex-dio) ran 2 rounds and returned CLEAN at this head. Round 1 had two blockers:
I also fixed one gap before review: an exception thrown after The implementation was drafted by a subagent; I reviewed and fixed it. Bots:
Hosted checks at this head: 29 pass and 2 fail.
This is the same failure as on #795's head. This PR does not touch that file or the download code. It is not one of the named flakes (HACK-2235, HACK-2243), so I did not rerun it. Please rerun the failed job, or tell me to. Ran locally (Linux, pinned Bun):
Not verified:
Decisions to check:
|
roodboi
added a commit
that referenced
this pull request
Nov 15, 2026
…p turn (HACK-2250) (#858) ## What `tests/python-free-download.test.ts`, "bounded cancellation refuses uncertain read-error settlement and preserves the primary failure", hit Bun's 5 s default timeout on hosted macOS shard 1/8 (#853). The next scenario then failed `toHaveBeenCalledTimes(1)` with 2, and the global-state guard reported `globalThis.fetch replaced`. ## What it was really waiting on The test drove a fake clock after a single `setTimeout(0)`: ```ts await drain(); // one event-loop turn clock.advance(5); // expire the 5 ms cleanup grace ``` `transferAttempt` (`src/lib/release-transfer.ts`) awaits a real `open()` of the target file before reading. Only after the size failure does `settleCancellation` arm the 5 ms grace on the fake clock. If `open()` takes more than one turn (a loaded runner), the advance fires nothing. The grace is armed afterwards at the new time and never expires, so the download never settles. The test then hangs until the 5 s timeout, and its `finally { mock.mockRestore() }` never runs. The next scenario's `spyOn(globalThis, "fetch")` reuses the leaked spy, which already has one call: hence 2, and the guard error. ```mermaid sequenceDiagram participant T as test participant D as download participant C as fake clock T->>D: start T->>T: one setTimeout(0) T->>C: advance(5): nothing armed yet D->>D: open() completes (real I/O, late) D->>C: arm 5 ms cleanup grace (never advanced) Note over T: hangs to the 5 s timeout, finally skipped, fetch spy leaks ``` The same assumption (one turn is enough) sat in three sibling tests in this file: "HTTP transfer budget … progress / total" and "five-file … cleanup". They fail the same way when `open()` is slow. ## Fix (test-only) - `until(ready, what)` drains in a loop with a **2 s real-time bound**, well under the 5 s default. A missed step fails with `Fixture step did not happen within 2000 ms: <what>` instead of a timeout. `within(promise, what)` bounds the final settlement the same way. - Before advancing the 5 ms cleanup grace, the test waits for `clock.isArmed(5)`. `FixtureClock` now records each timer's delay; the other timers are 30 and 100 ms. - Before an inactivity tick, the test waits for an actual pending read. The fixture streams use `highWaterMark: 0` and flag `pull()`, which runs only while the download is reading. A file-existence check was not enough: the file exists before `open()` returns its handle (found in review). - The five-file spin loops, capped at 100 turns, use the same real-time bound. - `afterEach` calls `mock.restore()`, so a test that fails before its `finally` cannot leak the `fetch` spy into the next test. The existing per-test `mockRestore()` calls stay. No assertion is weakened or removed. ## Validation (Linux, 16 cores) | Check | Before | After | | --- | --- | --- | | `-t "bounded cancellation"` under 16-way parallel load | 5 of 400 failed, with the hosted signature (5 s timeout, then 2 calls and the guard error) | **400 of 400** pass at 5f16f8b | | Whole file under 16-way parallel load | 2 of 191 failed at an intermediate head (progress/total false stall, found in review) | **208 of 208** pass (15/15 each) at 5f16f8b | | Preload delaying `open()` by 50 ms | 4 failures, incl. read-error and stalled-read timeouts with the guard error | 15/15 | | Preload delaying `open()` by 200 ms | — | 15/15 | | Preload delaying after `open()` creates the file | progress/total fail | 15/15 | `tsc -p packages/cli` (it includes `tests/`) and `biome check` pass. Not verified: macOS. The lead runs the 200 on the Mac. ## Release signal `test`: test-only. Refs HACK-2250. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Kit <344842+roodboi@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
taking R6 (HACK-2218). This PR is the Compose slice; the native part is listed under "Not in this PR".
The desktop app's actions need to show which service is at which step while
hack up,downandrestartrun. With--jsontoday they print only the final envelope.With
hack up|down|restart --json --progress, stdout becomes ndjson:{"schema":"hack.lifecycle-progress.v1","type":"start","ts","action","project"}. It is written once the project is resolved.{"type":"progress","ts","service","phase"}lines.{ok,data}/{ok:false,error}envelope that--jsonprints today, on one line. Exit codes are unchanged.serviceimages,startingstopping,stoppeddownandrestartwaiting,installing,verifying,ready,failedrunning,completed,stopped,failedOnly signals the code already observed are used: no TTY parsing and no new polling. Some phases can be skipped (see
docs/reference/lifecycle-progress.md).Design (please check):
emitLifecycleResult(every lifecycle envelope goes through it).startis closed by the top-level handler with one compact envelope line, never the pretty one.renderCliResultgains an optionalcompactflag. The default output of other commands is unchanged.--progressrequires--json.Not in this PR (R6 follow-up, native):
up(its detached IPC carries no per-service events today)restart --json, which is refused todaydown --json, which prints bare JSONNative backend and native-authored projects refuse
--progressbefore doing any work.Validation (Linux, pinned Bun 1.4.4):
docker:upsuccess and failure,down,restart,--progresswithout--json, and the native refusal.nexton this host.bun run typecheck,bun run check(warnings only),docs:prepareanddocs:checkpass.Release intent:
feat.🤖 Generated with Claude Code
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.