feat: complete the tasks action family on one shared runner - #113
Merged
Conversation
approve, deny, comment, close and reassign each hand-rolled the same thirty lines: resolve the task id, build a body, POST /action/<verb>, parse, print one line. Five more actions would have made ten copies, so the runner comes first and all ten share it. The five existing commands are byte-identical before and after — help text and dry-run output diffed against origin/main. New: restart, reset, skip-step, process, update-grant-duration. Each was driven against a live task and verified by its effect, not its exit code: restart and skip-step rotate the current policy step and append one history entry, reset appends four (it restarts the policy rather than the step), and update-grant-duration lands as grantDuration on the task. Deferred: escalate refuses with "action not permitted" even with emergency grants enabled on the entitlement and a valid step id, so its behaviour cannot be demonstrated here; update-request-data takes a free-form object that wants a body-file flag; approve-with-step-up needs a step-up transaction id the CLI cannot obtain. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only claims that were observed live: which actions a task accepts depends on its state, every action rotates the current policy step so a captured --policy-step-id goes stale, restart re-runs a step while reset restarts the policy, neither reopens a closed task, process changes nothing observable on a healthy task, and update-grant-duration lands as grantDuration but is refused once the task reaches provisioning. The README guard caught "<protobuf duration>" parsing as two positionals. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the docs The runner previewed --dry-run before resolving the URL, so close and comment stopped rejecting a malformed --url and stopped warning when the target came from config rather than the flag. A typo'd tenant previewed happily at exit 0. The URL is resolved first now; credentials are still deferred, which is what each command did before. Parity with origin/main re-diffed for all five. The test tables covered 7 of 10 and 6 of 10 commands, so four plausible regressions passed the whole suite: update-grant-duration posting to /action/reset, update-grant-duration gaining a policy step, deny's stepOptional becoming stepRequired, and comment losing the extraBody that makes an explicit --comment "" reach the wire. One table now seeds from tasksCmd.Commands(), so a command added without a row fails instead of going untested, and deny's omit path and comment's always-send are asserted on the wire. All four regressions now fail. "Every action rotates the current policy step" was both unmeasured and contradicted two paragraphs later by process. Narrowed to the three measured. The stalled-task effect of process is stated as intent, not observation. --comment on restart, reset and skip-step is confirmed to land: commentCount incremented on each. addTaskActionFlags is gone -- three of ten callers, and nothing to enforce since two actions deliberately take no comment. Also drops an unreachable --duration branch, two stale counts in a comment, and refreshes resolvePolicyStepID's doc, which described approve/deny only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seven findings from the delta review. The fix this branch made to --dry-run had no test: reverting it, or deleting either dry-run short-circuit so a preview really POSTs, all passed the suite. Every action is now driven with dry_run set against a server that fails the test on any POST, and the preview path is asserted. A bool could not distinguish stepOptional from stepRequired, so flipping approve or skip-step to optional survived — they would have silently posted with no policyStepId instead of erroring. The table carries the mode itself now, and a second test drives each mode with no derivable step: required must error before sending, optional must send without the field. update-grant-duration's one payload key was unpinned, and "grantDuration" is the plausible wrong name because that is what the response carries and what the docs quote. Pinned both ways. Its unset --duration path is pinned through the root command, since the direct-RunE harness never sees cobra's required check. Docs: reset rotates the step but takes no --policy-step-id, and the narrowed wording had put it in the group that does. The changelog said the opposite of the README about reset, and process's stalled-task effect was hedged in only one of four places. One wording now, true on each surface. resolvePolicyStepID's doc claimed restart passes required=true; it is stepOptional, which is what lets it act on a closed task. Also drops a doubled "(required)" marker caused by a usage string containing "(required;". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six of the seven mutations the review found were closed by the previous commit; reverting the URL fix itself still passed. The other dry-run tests stub the client and pass a valid URL, so the ordering is invisible to them — it only matters when the URL cannot be resolved. This drives a stepless and a step-using action through the root command with a malformed --url under --dry-run, and requires exit 2 with no preview printed. Verified against the mutation: reverting the ordering now fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The flag enumeration in README and the agent doc was wrong twice: it named two commands as accepting --policy-step-id when five do. The synopsis block above it already states this per command, so the sentence is gone rather than fixed a third time. Pins two contracts that were stated only in prose: a stepless action previews without credentials, and a bad --url still fails under --dry-run. Both were shown to regress silently. A leaked SetOut on package-level commands had made one of those assertions unreachable; dry-run state is now saved and restored like every other site in the repo. Documents that restart and skip-step authenticate under --dry-run, since they fetch the step. Comments trimmed throughout to the non-obvious fact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 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
1 participant
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.
c1i tasks restart,reset,skip-step,processandupdate-grant-duration. The task action family wrapped 5 of the API's 13action routes; these add the five whose behaviour could be demonstrated.
All ten now share one runner.
approve,deny,comment,closeandreassigneach hand-rolled the same request-and-confirm sequence, so addingfive more would have meant ten copies. Their help text, dry-run output and
exit codes are unchanged — diffed against a binary built from
mainacross aURL and credential matrix.
What each one does, measured
restartresetskip-stepprocessupdate-grant-durationgrantDurationon the taskWhich actions a task accepts depends on its state; the rest are refused with
action not permitted. Read the task's own list withc1i api --path /api/v1/tasks/<task-id> --fields actions.--durationtakes a protobuf duration (3600s, not1h) and only appliesbefore the task reaches provisioning, after which the server answers
cannot update grant duration for a ticket in a provision step.Validation was by effect rather than exit code, which caught three claims that
were wrong:
restartdoes not reopen a closed task, it printed an emptypolicy_step_idwhen no step resolved, and the provision-step precondition wasundocumented.
Not included
escalate— the fixture was built end to end (an entitlement withemergencyGrantEnabledpointing at a real emergency policy, made requestablethrough an access profile, with a task pending approval) and the endpoint
still returns
action not permitted.emergencyGrant*is settable only atentitlement create, and the app-level
access_request_defaultsroute is inthe spec but returns 404.
update-request-data— itsdatafield is a free-form object, so it wants abody-file flag rather than flags.
approve-with-step-up— needs a step-up transaction id the CLI cannot obtain.🤖 Generated with Claude Code