feat(shorebird_cli): add patches rollback / rollforward#3750
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Adds CLI surface for the rollback and rollforward operations that were
previously dashboard-only. Mirrors the shape of the existing
`patches set-track` command (release + patch lookup, --json envelope,
deferred channel/state checks before the network call).
shorebird patches rollback --release 1.0.0+1 --patch 1
shorebird patches rollforward --release 1.0.0+1 --patch 1
Both commands also accept --app-id / --flavor for callers that don't
have a shorebird.yaml on hand (CI scripts, monorepos with multiple
apps, etc.).
Three layers of changes:
1. shorebird_code_push_client: new `rollbackPatch` and `rollforwardPatch`
methods on CodePushClient. Each POSTs to
`/apps/<id>/releases/<id>/patches/<id>/{rollback,rollforward}` and
treats HTTP 304 (server's "already in target state" response) as
a no-op success so the call is idempotent.
2. shorebird_cli code_push_client_wrapper: wraps each with the standard
logger.progress + _handleErrorAndExit pattern. Optional patchNumber
parameter feeds into the progress label since the caller usually has
it on hand.
3. shorebird_cli commands: RollbackCommand and RollforwardCommand,
registered under the existing `patches` command group. Pre-checks
`is_rolled_back` from getReleasePatches before issuing the POST so
the no-op case prints a helpful message instead of relying on the
server's 304 path.
Tests:
- code_push_client_test.dart: request shape, 204 + 304 success paths,
unknown-error path, structured-error-response path.
- code_push_client_wrapper_test.dart: progress success/failure, optional
patchNumber label.
- rollback_command_test.dart, rollforward_command_test.dart: full
matrix mirroring set_track_command_test — validation failure,
--app-id override, missing patch, already-in-target-state, success,
fetch-failure (text + json), API-failure (text + json), and the json
envelope shape on each branch.
Verified end-to-end: `shorebird patches --help` lists the new
subcommands; `shorebird patches rollback --help` and
`shorebird patches rollforward --help` both render the expected usage.
Word appears in a comment on rollforwardPatch in code_push_client_wrapper — 'the server resends the same patch artifact on rollforward'. Same fix as shorebirdtech/updater#356 needed in this repo.
1535ce4 to
945e8e7
Compare
nickshorebird
left a comment
There was a problem hiding this comment.
LGTM overall - Two nits for behavior. Thanks for putting this together.
| if (isJsonMode) { | ||
| emitJsonError( | ||
| code: JsonErrorCode.usageError, | ||
| message: 'Patch $patchNumber is already rolled back.', |
There was a problem hiding this comment.
Take it or leave it - End state is the same is the same on success or hitting this route. Returning a non zero exit suggests that something went wrong even though the end state is correct. Consider adding some flag IE --require-change, I am awful at naming to please feel free to change it, if you want the behavior to be there on default or non default. Changing to this would also match other CLI tool behaviors. This is in line w/ current set-track behavior hence take it or leave it.
| 'release_version': releaseVersion, | ||
| 'patch_number': patchNumber, | ||
| 'action': 'rollback', |
There was a problem hiding this comment.
Take it or leave it - Consider printing the patch object similar to patches info on rollback and rollforward. Would reduce the need to do follow up patches info call.
Summary
Adds CLI surface for the
rollbackandrollforwardpatch operations thatwere previously dashboard-only. Both invocations are single network calls
gated by a pre-flight
is_rolled_backcheck so the no-op case prints ahelpful message instead of relying on the server's
304response.Both accept
--app-id/--flavorand the global--jsonflag.Why
The HTTP endpoints have existed for a while, but the only way to invoke
them today is through the dashboard. Two recent uses of these operations
have been workflow-friction:
device required scripting raw
curlcalls againstapi.shorebird.devin a side repo, because the dashboard isn't scriptable.
regression detected, kill-switch flipped, etc.) have to roll their own
HTTP client + token handling.
A CLI command is the natural shape for both.
Implementation
Three layers, mirroring the existing
patches set-trackshape end-to-end.Layer 1 —
shorebird_code_push_clientNew
rollbackPatch/rollforwardPatchmethods onCodePushClientthatPOST to:
Both treat
HTTP 304 Not Modified(the server's "already in target state"response — see
code_push_server/routes/.../rollback.dart) as success, sothe call is idempotent at the client level too.
Layer 2 —
code_push_client_wrapperWraps each with the standard
logger.progress+_handleErrorAndExitpattern, matching
promotePatch. An optionalpatchNumberparameterfeeds into the progress label since the caller usually has it on hand
(
"Rolling back patch 5"reads better than"Rolling back patch").Layer 3 —
RollbackCommand/RollforwardCommandRegistered under the existing
patchescommand group. Each pre-checksis_rolled_backfromgetReleasePatchesbefore issuing the POST so:Patch 1 is already rolled backand exits with usage error, instead ofsilently 304-ing and printing success.
rollforwardagainst an already-active patch.--jsonenvelope on success:{"release_version":"1.0.0+1","patch_number":1,"action":"rollback"}(
actiondistinguishes the two commands for callers that machine-read.)Naming + flag conventions
rollback/rollforwardmatch the HTTP endpoint paths,the server-side filenames, and the dashboard JS client's
rollBackPatch/
rollForwardPatchmethods. The dashboard UI labels them "Rollback"and "Roll Forward" but everything below the UI uses the single-token
forms.
--release-version(viaCommonArguments.releaseVersionArg)and
--patch-number, matchingpatches info(feat(shorebird_cli): add patches list/info; --json for set-track #3740). The olderpatches set-trackuses--release/--patchwhich is a pre-existinginconsistency; this PR doesn't propagate it into new commands.
Test plan
dart analyzeclean across all changed files.dart testgreen:code_push_client_test.dart— request shape, 204 + 304 success paths,unknown-error path, structured-error-response path. 8 cases.
code_push_client_wrapper_test.dart— progress success + failure forboth verbs, optional
patchNumberlabel. 5 cases.rollback_command_test.dart/rollforward_command_test.dart—validation failure,
--app-idoverride, missing patch,already-in-target-state, success, fetch-failure (text + json),
POST-failure (text + json), JSON envelope shape on each branch.
30 cases.
api.shorebird.devwhilereproducing bug: checkForUpdate reports upToDate after patch-to-release rollback (regression of #3206) #3728: rollback returns
204,device picks up the rollback signal on the next poll, then rollforward
returns
204and the device's auto-update thread either reinstalls(rc2 / fixed) or refuses (rc1 / broken) the patch as expected.
shorebird patches --helplists the new subcommands; each--helprenders the flag table with(mandatory)annotations.Refs
motivated this CLI surface)
patches list/patches info)code_push_server'sroutes/api/v1/apps/[appId]/releases/[releaseId]/patches/[patchId]/rollback.dartand
rollforward.dart