feat(cli): add openship deployment bisect to find the first bad deployment - #542
Open
Rish-it wants to merge 3 commits into
Open
feat(cli): add openship deployment bisect to find the first bad deployment#542Rish-it wants to merge 3 commits into
openship deployment bisect to find the first bad deployment#542Rish-it wants to merge 3 commits into
Conversation
`deployment list` built its own query string inline: project scope from --project or the linked project, optional environment filter, and perPage clamped to the API's 100-row ceiling. Any other subcommand needing the same page had to copy all of it. Pull it into `fetchDeployments()` and point `list` at it. No behaviour change: same query parameters, same clamp, same `data ?? []` fallback. `deployment list` had no test coverage at all, so add two before touching it — one asserting the query it builds for --project/--env/--limit and the rendered row, one pinning the 100-row clamp. Both pass before and after the extraction.
Narrowing a good/bad range is the only real logic in a bisect: pick the midpoint, then move the good boundary up, the bad boundary down, or drop a candidate that cannot be judged. Keep it in its own module with no I/O so it is unit-testable without mocking prompts. `bisectMidpoint` returns -1 once only the boundary pair is left, which is also the termination signal; for a range of three or more it never lands on a boundary, so an already-known deployment is never re-asked. A skipped candidate is never labelled good or bad, so the final bracket can be wider than the minimal transition pair. That is intended: the invariant the caller reports on is that the lower bound is genuinely good and the upper bound genuinely bad, not that they are adjacent. Covered by a test that skips a midpoint and asserts the surviving bracket still straddles the real transition.
`deployment list` shows the history and `deployment rollback` reverts to a chosen deployment, but nothing narrows "one of these 40 deployments broke it" to a single culprit — that meant walking the history one rollback at a time. Add a binary search over the same history: fetch one page via the shared `fetchDeployments`, keep the deployments that actually serve something, sort oldest-first, then ask good/bad/skip for each midpoint until only the boundary pair remains. Reports the first bad deployment and the last known-good one, and offers a rollback to the good one. Roughly log2(n) checks instead of n. No new API route and no new dependency: it reads GET /deployments and writes POST /deployments/:id/rollback, both already used by `list` and `rollback`, and reuses the existing run()/report()/confirm()/shortSha() helpers plus the @clack/prompts and open packages the CLI already ships. Only `ready` and `partial_failure` are treated as testable — queued/building/ deploying have not finished and failed/cancelled/rejected have nothing to visit. Note the API persists `ready`; `success` is a dashboard-side display mapping and never appears in an API response. Refuses to run without a TTY or under --json, since neither can answer a prompt. Aborting (menu or Ctrl-C) and declining the final offer both leave the active deployment untouched.
There was a problem hiding this comment.
Pull request overview
Adds an interactive openship deployment bisect CLI subcommand that binary-searches a project’s deployment history to identify the first bad deployment and optionally roll back to the last known-good deployment.
Changes:
- Extracts a shared
fetchDeployments()helper fordeployment listanddeployment bisect. - Introduces a pure, unit-tested bisect core (
bisectMidpoint,bisectDone,bisectStep). - Implements
deployment bisectcommand flow and adds E2E/unit test coverage for key branches.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/cli/src/commands/deployment.ts | Adds deployment bisect, factors out shared deployment listing fetch logic, wires command registration. |
| apps/cli/src/lib/bisect.ts | New pure bisect core utilities (no I/O) to support unit testing. |
| apps/cli/test/e2e/deployment.test.ts | Adds E2E coverage for deployment list and deterministic deployment bisect branches/validation. |
| apps/cli/test/unit/bisect.test.ts | Adds unit tests validating bisect math and convergence behavior (including skip). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+190
to
+198
| run(async (opts) => { | ||
| if (!process.stdin.isTTY || isJsonMode()) { | ||
| err("`deployment bisect` is interactive — it needs a TTY and cannot run under --json."); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Only ready/partial_failure actually deployed something visitable — | ||
| // queued/building/failed/cancelled/rejected have nothing to look at. | ||
| const testable: BisectCandidate[] = (await fetchDeployments(opts)) |
Comment on lines
+178
to
+180
| function describeCandidate(d: BisectCandidate): string { | ||
| return `${d.id} (${d.version ? `v${d.version}` : shortSha(d.commitSha)}, ${d.branch}, ${d.createdAt})`; | ||
| } |
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.
Summary
Adds
openship deployment bisect, a binary search over deployment history that finds the first bad deployment in roughly log2(n) checks instead of n, then offers a rollback to the last known-good one. No new API route, no schema change, no new dependency.Motivation
deployment listshows the history anddeployment rollback <id>reverts to a chosen deployment, but nothing connects them: there is no supported way to find which deployment introduced a regression. In practice that means walking the history one deployment at a time — O(n) rollbacks, each one mutating the active deployment just to take a look.Everything needed was already stored and already served.
deployment.versionis a monotonic per-project counter,createdAtorders the history,urlis the visitable address, and all three already come back onGET /api/deployments—presentDeploymentsmasks env only, it does not project columns away. Nothing consumed them for this.On a project with 40 deployments between the last good release and the current broken one, this is 6 questions instead of 40.
Related issue
Closes #541.
To be straight about the ordering: I opened #541 and this PR together rather than getting scope agreed first, so treat the issue as the proposal, not a settled decision. Happy to cut or reshape anything — the interactive flow, the rollback offer, the flag surface — if the approach isn't what you want.
Changes
All in
apps/cli, three commits, each building and passing on its own.2663003crefactor(cli): extract the shared deployment-list fetchsrc/commands/deployment.ts—listbuilt its query inline (project scope from--projector the linked project, optional env filter,perPageclamped to the API's 100-row ceiling). Pulled intofetchDeployments()so a second caller doesn't copy it. No behaviour change.test/e2e/deployment.test.ts—deployment listhad no test coverage at all. Added two before touching it: one asserting the query built for--project/--env/--limitplus the rendered row, one pinning the 100-row clamp. Both pass before and after the extraction, which is what makes the refactor safe.911f9875feat(cli): add the pure binary-search coresrc/lib/bisect.ts(new, 30 lines) —bisectMidpoint/bisectDone/bisectStep. No I/O, so the actual search logic is unit-testable without mocking prompts.bisectMidpointreturns -1 once only the boundary pair is left, which doubles as the termination signal; for a range of 3 or more it never lands on a boundary, so an already-known deployment is never re-asked.test/unit/bisect.test.ts(new) — 7 tests: midpoint/termination boundaries, all three transitions, and two full convergence simulations.9661debdfeat(cli): addopenship deployment bisectsrc/commands/deployment.ts— the command, plus registration and one row in the module's route table.test/e2e/deployment.test.ts— 7 tests over the deterministic branches.Deliberately reused rather than rebuilt:
run(),report(),confirm(),shortSha(),readProjectLink(), theoutputhelpers,fetchDeployments()from commit 1,@clack/prompts, and theopenpackage with the same dynamic-import-and-swallow pattern asopenship open.Status filter. Only
readyandpartial_failureare treated as testable —queued/building/deployinghaven't finished andfailed/cancelled/rejectedhave nothing to visit. Note for reviewers: the API persistsready;successis a dashboard-side display mapping (mapRowToDeployment) and never appears in an API response, so a filter onsuccesswould match zero rows. Same distinction #410 is about, and the same eligibility set #414 settles on.Skip semantics. A skipped candidate is never labelled, so the final bracket can be wider than the minimal transition pair. That is intended: the invariant reported on is that the lower bound is genuinely good and the upper bound genuinely bad, not that they are adjacent. There is a test that skips a midpoint and asserts the surviving bracket still straddles the real transition.
Refuses to run without a TTY or under
--json, since neither can answer a prompt. Aborting (menu or Ctrl-C) and declining the final offer both leave the active deployment untouched.Verification
Each commit independently, on top of
mainat30074018:bun run test(full monorepo):Tasks: 7 successful, 7 total.Against a real instance, not a mock — API on :4000 with Postgres,
DEPLOY_MODE=docker, a project deployed 7 times as real Docker containers where v5 onward serves HTTP 500, plus onefaileddeployment in the history:Three questions for seven candidates, correct culprit, and the
faileddeployment excluded by the status filter. The rollback was verified at the application layer, not just by exit code — it created a new deployment and the live container then answered:The interactive loop itself was driven through a real PTY (
expect), with aPATHshim standing in foropenso the browser calls were captured rather than launched:Not covered by automated tests, and stated rather than hidden: the live good/bad/skip loop and the final rollback prompt read real stdin, and this suite has no mock seam for
@clack/promptsor a TTY — no other interactive command here has one either. That is what the PTY runs above are standing in for. The search logic those branches drive is fully unit-tested.Checklist
bun run test,bun run --cwd apps/cli lint, andprettier --checkall pass locally