Skip to content

feat(rule): list a rule's revisions, to choose one for rollback - #422

Merged
thecodedrift merged 6 commits into
mainfrom
openspec/cli-rule-revisions
Sep 30, 2026
Merged

thecodedrift merged 6 commits into
mainfrom
openspec/cli-rule-revisions

Conversation

@thecodedrift

@thecodedrift thecodedrift commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

taskless rule rollback <ruleId> <revisionId> needed a revision id that nothing in the CLI could produce. The recover-rule recipe sent the user to the dashboard to copy one, so an agent asked to "roll this rule back" could not finish on its own. The service now lists a rule's revisions (GET /cli/api/v2/rule/{ruleId}/revisions, taskless/taskless#261), deployed 2026-09-30. This PR adds the CLI side.

What

  • taskless rule revisions <ruleId> [--json]: lists the rule's recent revisions and marks the current one. It reads only, and it works on every plan: the listing carries no rule bytes, so the service never reads the plan for it.
    • The current revision is found by its current flag, never by position. The service appends a current revision older than the newest ten after them.
    • When no revision is current, the rule exists only on an unmerged PR. The output says so, and shows the prUrl.
    • truncated: true sends the user to the dashboard, which lists every revision.
    • --json prints { success, ruleId, revisions, truncated }.
  • listRevisions in api/v2.ts. Its error codes are checked against the schema at compile time. A 200 that isn't a listing is unavailable, never an empty history, because an empty list would tell the user the rule has no history.
  • REVISION_NOT_FOUND's message names rule revisions <ruleId> instead of the dashboard. rule rollback's arguments and behavior are unchanged.
  • recover-rule moves to topic v2. It gains a "Rolling back" section: list, pick what the user described (ask if more than one fits), roll back.
  • The vendored api-v2.schema.json: purely additive, one new path.

runRecovery's identity and error-reporting prelude became runForIssuedRule, which restore, rollback and revisions share.

Notes for review

  • rule_not_found gets its own message on this route. On restore, the service also returns it for a rule that exists only on an open PR, and the restore message says so. The listing has no such case: taskless/taskless#261's spec lists a PR-only rule with no revision marked current. So the listing drops that clause.
  • The output schema is internal, like rules-recover.ts. @taskless/cli/schemas publishes only the verify / test envelopes. The proposal originally said otherwise, and was corrected during implementation.
  • OpenSpec cli-rule-revisions is archived on this PR. The delta is two ADDED requirements on cli-rule-recovery. The pre-archive check held: 10 scenarios before, 18 after, none dropped.

Verification

  • pnpm typecheck, pnpm lint (including the house-style check), and pnpm test: all pass, 1888 tests.

  • New tests: api-v2.test.ts covers the wire, rule_not_found, and a malformed 200. rule-recovery.test.ts has one test per spec scenario, driven through the real command.

  • Production round trip, with 0.12.0-next-0fb5221 against app.taskless.io, in taskless-sandbox/nextjs-sass-starter (Free plan), on a rule freshly generated through v2 (no-console-log-234c30fd):

    Command Result
    rule revisions <id> one revision, cli delivery, marked (current), names rule rollback; exit 0
    rule revisions <id> --json { success: true, ruleId, revisions: [{ revisionId, createdAt, delivery, requestId, current: true }], truncated: false }; exit 0
    rule rollback <id> <listed revision> --json RULE_RECOVERY_NOT_IN_PLAN with the service's git guidance and upgrade link; exit 1
    rule rollback <id> not-a-revision --json REVISION_NOT_FOUND, naming rule revisions <id>; exit 1
    rule revisions no-such-rule-00000000 --json RULE_NOT_FOUND, without restore's pull-request clause; exit 1

    The listing works on Free, as the spec requires. A rollback to a real revision was refused for the plan, so a successful write through a listed id was not exercised against production. The existing rollback tests cover that path.

Merge note

#423 regenerates the same two files, packages/cli/src/generated/api-v2.schema.json and api-v2.d.ts, so whichever of #422 and #423 merges second will conflict there. Don't hand-merge them. Instead, rerun pnpm --filter @taskless/cli generate:api against production, then prettier --write on api-v2.d.ts by explicit path, since the generator writes it unformatted. Both changes are additive, and the live schema already contains both.

Refs taskless/taskless#253

Vendors GET /cli/api/v2/rule/{ruleId}/revisions (taskless/taskless#261)
and adds listRevisions beside restore and rollback. Proposes
cli-rule-revisions.
Lists a rule's recent revisions on every plan, marking the current one
by its flag. REVISION_NOT_FOUND now names the command instead of the
dashboard.
recover-rule moves to topic v2: it lists revisions instead of sending
the user to the dashboard for an id. Adds the changeset.
@thecodedrift
thecodedrift marked this pull request as ready for review September 30, 2026 04:34
@thecodedrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Claude finished @thecodedrift's task in 5m 37s —— View job


Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no reviewThreads and no reviews, only a single top-level @claude /review comment (the trigger itself). This is treated as the first review of this PR; nothing prior to classify as addressed/still-open.

Todo list

  • Read .prior-review.json (empty — first review)
  • Gather PR diff via gh pr diff
  • Read changed files for context (api/v2.ts, commands/rules.ts, rules/recover.ts, schemas/rules-revisions.ts, agent/recover-rule.md, tests)
  • Run correctness/security/performance/test/clarity review
  • Post inline comments for concrete issues
  • Post final top-level summary

Assessment

This is a clean, well-scoped change. The service-mapping logic (listRevisions → V2Outcome → failure() → CLIError), the "find current by flag, never by position" handling in describeRevisions, and the runRecovery/runForIssuedRule refactor all check out — I traced every call site of the refactored runForIssuedRule/runRecovery (only restoreCommand, rollbackCommand, revisionsCommand use them) and the behavior is preserved. Error-code mapping (rule_not_found → RULE_NOT_FOUND without the restore-specific PR clause, any other failure → NETWORK_ERROR) matches the spec exactly, and the new tests exercise each spec scenario (current-by-flag past the cap, PR-only rule with no current, truncation, --json envelope, 404 mapping) plus the wire-level api-v2.test.ts cases (200/401/404/malformed-200-never-read-as-empty). I didn't find any correctness, security, or data-handling bugs.

Three minor [New] findings posted inline, all documentation/cleanup rather than functional bugs:

  1. packages/cli/src/agent/recover-rule.md:83 — "Read the result as in step 3 above" is ambiguous: it sits inside step 3 of the new "Rolling back" list, so it reads as pointing at itself rather than at step 3 of the ## Steps section above it. The document now has two "step 3"s with different meanings; since this text is consumed literally by an agent, the reference is worth naming explicitly instead of by number.
  2. packages/cli/src/agent/recover-rule.md:105 — the new REVISION_NOT_FOUND row's fix column (`rule revisions <ruleId>`) is the only command mention in the file that omits the %(TASKLESS_CLI)s prefix every other row/section uses, so as rendered it isn't a runnable invocation. recipe-cross-references.test.ts's hardcoded-invocation guard won't catch this since the string never contains the literal word "taskless".
  3. packages/cli/src/api/v2.ts:311 — RevisionEntry is exported but has no consumers anywhere in the codebase (not recover.ts, not the schema, not tests). Not harmful, just unused.

Verification claims in the PR body (typecheck/lint/test, plus the production round trip against app.taskless.io) are consistent with what the diff shows; I did not re-run the build/lint/test suite myself, per review instructions.

Comment thread packages/cli/src/agent/recover-rule.md Outdated
Comment thread packages/cli/src/agent/recover-rule.md Outdated
Comment thread packages/cli/src/api/v2.ts Outdated
Name the recipe step rollback defers to instead of a number that reads
as itself, give REVISION_NOT_FOUND's fix the CLI prefix so it runs, and
drop the unused RevisionEntry export.
@thecodedrift

Copy link
Copy Markdown
Member Author

Re: @claude[bot] — "Claude finished @thecodedrift's task in 5m 37s — Review complete…"
#422 (comment)

All three findings are addressed in c9710cd: the rollback step now names the ## Steps section rather than "step 3 above", the REVISION_NOT_FOUND fix carries the %(TASKLESS_CLI)s prefix, and the unused RevisionEntry export is removed.

— AI Coding Agent

@thecodedrift
thecodedrift merged commit e1f084a into main Sep 30, 2026
7 checks passed
@thecodedrift
thecodedrift deleted the openspec/cli-rule-revisions branch September 30, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant