diff --git a/openspec/changes/cli-plan-aware-recovery/.openspec.yaml b/openspec/changes/cli-plan-aware-recovery/.openspec.yaml new file mode 100644 index 00000000..1aca8b91 --- /dev/null +++ b/openspec/changes/cli-plan-aware-recovery/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-30 diff --git a/openspec/changes/cli-plan-aware-recovery/design.md b/openspec/changes/cli-plan-aware-recovery/design.md new file mode 100644 index 00000000..8498477a --- /dev/null +++ b/openspec/changes/cli-plan-aware-recovery/design.md @@ -0,0 +1,90 @@ +## Context + +Two paths already call `GET /cli/api/v2/whoami` to pick the acting organization, both through +`resolveOrgSubject` in `packages/cli/src/auth/org.ts`: `resolveIdentity` (every `rule` +subcommand that talks to the service) and `planCheck` (`check`'s reconcile). Each discards +everything but the organization's `id`. + +Every recovery suggestion in `check` is rendered in `applyVerdicts` +(`packages/cli/src/rules/verdicts.ts`) through one callback, `restoreCommand(ruleId)`, that +`planCheck` supplies so the policy stays free of how the CLI was invoked. It is used at three +sites: `unsafe` (runtime notice, static failure), `missing` (notice), and a rename +(`applyCopy`, when the source is `missing`). `rule revisions`' closing line is rendered by +`describeRevisions` in `rules/recover.ts`. + +## Goals / Non-Goals + +**Goals:** + +- One place decides "restore or git", fed by the whoami call already being made. +- Unknown is byte-for-byte today's output, so every existing test keeps passing unchanged. + +**Non-Goals:** + +- Gating `rule restore` / `rule rollback` locally. See proposal.md. +- Reading `runtimeSignatures` from whoami. Reconcile already reports it authoritatively, per + run, and `check` uses that. +- Surfacing entitlements in `info` or `auth status`. Nothing in this change needs it. + +## Decisions + +**Resolve the organization once, return its entitlement with its subject.** Replace +`resolveOrgSubject(cwd, token): Promise` with +`resolveActingOrg(cwd, token): Promise<{ subject: string | number; restoreRules?: boolean }>`. +`restoreRules` is set only when an organization matched and its `entitlements.restoreRules` +is a boolean; the token-claim and nil-UUID fallbacks leave it `undefined`. `Identity` gains +`restoreRules?: boolean`. Considered: a second `fetchWhoami` from the suggestion sites. +Rejected because the issue requires no extra request, and a second call could disagree with +the first about which organization acted. + +**The generated whoami type is read defensively.** `entitlements` is optional in the schema +and "absent means unknown". Read it with `typeof === "boolean"`, so an older or partial +response degrades to unknown rather than throwing. + +**Replace `restoreCommand` with a `recovery` callback that returns a whole sentence.** Today +each site writes `Run \`${restoreCommand(id)}\` to .`The git alternative is two +commands and a reason, which does not fit a slot shaped like one command. The callback +takes`{ ruleId, engine?, purpose }`and returns the sentence.`planCheck` builds it from the +tri-state: + +- not `false`: `Run \` rule restore \` to .`, exactly today's text. +- `false`: `Restoring rules is not included in your organization's plan, so recover it from +git: \`git log -- \` lists the commits that changed it, and + \`git restore --source= -- \` puts it back as of one of them.` + +`` is `.taskless/rules///`, or the quoted glob pathspec +`'.taskless/rules/*//'` when a `missing` verdict carries no known engine. The +rendering lives in a small pure function beside `applyVerdicts` so it is tested as a table +like the rest of the policy. Considered: passing the tri-state into `applyVerdicts`. Rejected +to keep `verdicts.ts` free of CLI prefix and plan concerns, which is why the callback exists. + +For a `missing` rule the newest commit `git log` lists is the one that deleted it. The +sentence says "as of one of them", and the `check` recipe spells out choosing the commit +before the deletion. A notice line is not the place for a git tutorial. + +**`describeRevisions(list, restoreRules?)`.** When `false`, the closing line becomes +`Rolling back is not included in your organization's plan; earlier versions of this rule are +in the repository's git history.` Otherwise unchanged. The command passes +`identity.restoreRules`. + +**Recipes.** `check.md` (v4 → v5): an edited or missing rule is reported with either +`rule restore` or git steps; follow whichever `check` printed, and for a deleted rule restore +from the commit before the deletion. `recover-rule.md` (v2 → v3): before offering restore or +rollback, read what `check` or `rule revisions` offered. If it gave git steps, the plan +excludes recovery; follow them rather than running a command that will be refused. + +## Risks / Trade-offs + +- [Entitlement changes between whoami and the suggestion, e.g. an upgrade mid-session] → + Each run reads whoami fresh, and the recovery commands still call the service, so the worst + case is one run with a stale suggestion. +- [A `false` from whoami that the service would not refuse] → The user is sent to git, which + still works. Nothing is blocked, so a wrong hint costs a detour, never a capability. +- [Glob pathspec on an unknown engine] → Quoted so the shell does not expand it; git applies + glob pathspecs by default. Only reachable when reconcile omits the engine, which it rarely + does. + +## Migration Plan + +None. No config, schema, or `--json` change. Rolling back the release restores the old +suggestions. diff --git a/openspec/changes/cli-plan-aware-recovery/proposal.md b/openspec/changes/cli-plan-aware-recovery/proposal.md new file mode 100644 index 00000000..e8e93c8c --- /dev/null +++ b/openspec/changes/cli-plan-aware-recovery/proposal.md @@ -0,0 +1,77 @@ +## Why + +The CLI suggests `taskless rule restore` and `taskless rule rollback` to every user, and learns +that the organization's plan does not include them only when the service refuses the call. A +user on such a plan is told to run a command, runs it, and is then told to use git instead. +v2 `whoami` now returns each organization's `entitlements`, including `restoreRules` +(taskless/taskless#254, live, and already in the vendored `api-v2.schema.json`), so the CLI +can stop suggesting what the plan will not serve. The two changes this builds on, `rule +revisions` (#422) and the `copyOf` rename notice (#423), have both merged. + +## What Changes + +- The acting organization's `entitlements.restoreRules` is read from the `whoami` call the + CLI already makes to resolve the organization. No new request. It is tri-state: `true`, + `false`, or unknown (whoami failed, `entitlements` absent, or no organization matched the + repository and the CLI fell back to the token's claim). +- When, and only when, `restoreRules` is exactly `false`: + - `check`'s notice for an `unsafe` or `missing` rule gives the git recovery steps for the + rule's directory instead of naming `rule restore `. + - The rename notice (a copy of an issued rule whose source is `missing`) gives the git + steps for the source's directory instead of naming `rule restore `. + - `rule revisions` still lists revisions, since the listing is served on every plan. Its + closing line says rolling back is not included in the plan, instead of naming the + `rule rollback` command. +- Unknown behaves exactly as today: every suggestion above names `rule restore` / + `rule rollback`. +- **Suggestions only, never a gate.** `rule restore` and `rule rollback` keep calling the + service whatever `whoami` said, and keep relaying its refusal verbatim. The schema calls + `entitlements` "a hint for the client, never a gate", and the refusal is the better answer + anyway: it carries the exact commands and the upgrade link. +- The `recover-rule` and `check` agent recipes tell an agent to read which recovery `check` + offered before reaching for `rule restore` or `rule rollback`. Both topics bump their + version. +- No `--json` change. `integrity` in `check --json` already says what is wrong with each + rule, and a recovery command an agent runs anyway is answered by the refusal. Nothing found + in this proposal needs a machine-readable plan field. + +## Capabilities + +### New Capabilities + +_None._ + +### Modified Capabilities + +- `cli-rule-recovery`: adds a requirement that recovery suggestions follow the plan's + `restoreRules` entitlement, and modifies `rule revisions`' closing line. +- `cli-check`: the "never writes to the rules tree" requirement names the recovery step for + the plan rather than always `rule restore`. +- `cli-rule-reconciliation`: the verdict table and the rename requirement name the recovery + step for the plan rather than always `rule restore`. The issue expected only the first two + specs, but these two requirements state the `rule restore` wording normatively, so leaving + them would contradict the new requirement. + +## Impact + +- `packages/cli/src/auth/org.ts`, `auth/identity.ts`: resolve the acting organization's + `restoreRules` alongside its subject, carried as an optional field on `Identity`. +- `packages/cli/src/rules/verdicts.ts`, `rules/plan-check.ts`: the notice text is rendered + through one recovery callback that knows the plan, replacing `restoreCommand`. +- `packages/cli/src/rules/recover.ts`, `commands/rules.ts`: `describeRevisions`' closing line. +- `packages/cli/src/agent/recover-rule.md`, `agent/check.md`: topic version bumps. +- Tests: `org.test.ts`, `verdicts.test.ts`, `rule-recovery.test.ts`, and the recipe parity + tests. +- No schema, dependency, or `--json` change. + +**Delivery shape: stacked, merging forward.** Three PRs, each safe in production alone: + +1. Resolve `restoreRules` onto `Identity` from the existing whoami call, with this proposal. + Nothing reads it yet, so output is unchanged. +2. `check`'s suggestions (`unsafe`, `missing`, rename) and the `check` recipe. +3. `rule revisions`' closing line, the `recover-rule` recipe, and the archive. + +Unknown preserves today's output exactly, so no intermediate state suggests anything wrong: +a slice that has not landed yet keeps suggesting `rule restore`. The whole diff would fit +one PR; it is split so external reviewers can read each behavior on its own. Unit 1 changes +nothing a user can observe, so the changeset starts on unit 2, and unit 3 extends it. diff --git a/openspec/changes/cli-plan-aware-recovery/specs/cli-check/spec.md b/openspec/changes/cli-plan-aware-recovery/specs/cli-check/spec.md new file mode 100644 index 00000000..f362365e --- /dev/null +++ b/openspec/changes/cli-plan-aware-recovery/specs/cli-check/spec.md @@ -0,0 +1,28 @@ +## MODIFIED Requirements + +### Requirement: Check never writes to the rules tree + +`taskless check` SHALL NOT create, modify, or delete anything under `.taskless/rules/`. It +SHALL NOT call restore, rollback, or rule fetch. For an `unsafe` or `missing` verdict it SHALL +name how to repair the rule: `taskless rule restore `, or, when the organization's +plan is known to exclude rule recovery, the git steps for the rule's directory (see +`cli-rule-recovery`, "Recovery suggestions follow the plan"). The only files +`check` writes under `.taskless/` SHALL be under `.taskless/.run/`. + +#### Scenario: An edited rule is reported, not repaired + +- **WHEN** reconciliation returns `unsafe` for a rule, and the organization's plan is not known to exclude rule recovery +- **THEN** `.taskless/rules/` SHALL be byte-identical before and after the run +- **AND** the output SHALL name `taskless rule restore ` + +#### Scenario: A missing rule is not fetched + +- **WHEN** reconciliation returns `missing` for a rule +- **THEN** `check` SHALL NOT call any restore or fetch endpoint +- **AND** SHALL NOT create the rule's directory + +#### Scenario: An edited rule on a plan without recovery is reported with git steps + +- **WHEN** reconciliation returns `unsafe` for a rule and the organization's `restoreRules` entitlement is `false` +- **THEN** `.taskless/rules/` SHALL be byte-identical before and after the run +- **AND** the output SHALL give the git steps for the rule's directory and SHALL NOT name `taskless rule restore` diff --git a/openspec/changes/cli-plan-aware-recovery/specs/cli-rule-reconciliation/spec.md b/openspec/changes/cli-plan-aware-recovery/specs/cli-rule-reconciliation/spec.md new file mode 100644 index 00000000..535c80a6 --- /dev/null +++ b/openspec/changes/cli-plan-aware-recovery/specs/cli-rule-reconciliation/spec.md @@ -0,0 +1,99 @@ +## MODIFIED Requirements + +### Requirement: Reconcile verdicts are applied per engine + +The CLI SHALL read the v2 reconcile response as a list of per-rule verdicts +(`rules[]`, each `{ ruleId, engine, verdict }` with `verdict` one of `run`, `unsafe`, +`missing`), a list of `unknown` rules (each `{ ruleId, copyOf? }`), and +`entitlement.withheld`, and SHALL apply this policy: + +| Verdict | runtime | sg / vale | +| -------------------- | -------------------------------------------------- | ------------------------------------------- | +| `run` | execute | run | +| `withheld` | do not execute; fail `check` | (never sent) | +| `unsafe` | do not execute; name the recovery | do not run; fail `check`; name the recovery | +| `missing` | warn; name the recovery | warn; name the recovery | +| `unknown` | do not execute (needs `--dangerously-run-scripts`) | run | +| `unknown` + `copyOf` | do not execute; name the source | do not run; fail `check`; name the source | + +The engine SHALL be taken from the verdict's `engine` for `rules[]` entries and from the +reporting directory for `unknown` entries. An `unsafe` notice SHALL name the rule and each +differing path, saying whether it changed, was removed, or was added. "Name the recovery" +means `taskless rule restore `, or, when the organization's plan is known to exclude +rule recovery, the git steps for the rule's directory (see `cli-rule-recovery`, "Recovery +suggestions follow the plan"). A signature SHALL +authorize running a runtime rule only through a `run` verdict, never by local comparison. + +#### Scenario: An edited static rule fails and does not run + +- **WHEN** reconcile returns `{ ruleId: "no-simply-1a2b3c4d", engine: "vale", verdict: "unsafe", files: [{ path: ".vale.ini", expected, got }] }` +- **THEN** the rule SHALL NOT run +- **AND** `check` SHALL exit non-zero naming the rule and `.vale.ini` as changed + +#### Scenario: A locally written static rule runs + +- **WHEN** reconcile lists a static rule's id in `unknown` without `copyOf` +- **THEN** that rule SHALL run +- **AND** the CLI SHALL emit no notice for it + +#### Scenario: A locally written runtime rule does not execute + +- **WHEN** reconcile lists a runtime rule's id in `unknown` and `--dangerously-run-scripts` is not set +- **THEN** the rule SHALL NOT execute +- **AND** its skip reason SHALL say it was not issued by the rule service + +#### Scenario: Missing warns and does not fail + +- **WHEN** reconcile returns a `missing` verdict for any engine, and no `unknown` rule names it in `copyOf`, and the organization's plan is not known to exclude rule recovery +- **THEN** the CLI SHALL warn naming the rule and `taskless rule restore ` +- **AND** SHALL NOT change the exit code because of it + +#### Scenario: An edited runtime rule is withheld, not failed + +- **WHEN** reconcile returns an `unsafe` verdict for a runtime rule +- **THEN** the rule SHALL NOT execute +- **AND** the exit code SHALL NOT change because of that verdict alone + +### Requirement: A copy of an issued rule does not run as a local rule + +When reconcile returns an `unknown` rule carrying `copyOf` (taskless/taskless#255), the CLI +SHALL NOT run or execute it, and SHALL remove it from the snapshot the engines read. For an +`sg` or `vale` rule, `check` SHALL fail with one message naming the rule, the source rule +`copyOf.ruleId`, and each path in `copyOf.files` as changed, removed, or added. For a runtime +rule, the exit code SHALL NOT change, and its skip reason SHALL name the source and say it +was not issued by the rule service. + +When `copyOf.ruleId` is also returned as `missing`, the CLI SHALL report the pair as one +rename: the copy's message SHALL say the source was deleted and SHALL name +`taskless rule restore `, or, when the organization's plan is known to exclude +rule recovery, the git steps for the source's directory, and the CLI SHALL NOT print a separate `missing` +warning for the source. A runtime rename SHALL be one notice and SHALL NOT change the exit +code. `copyOf` absent or `null` SHALL be treated as no copy. A `copyOf` that is present but +has no non-empty string `ruleId` SHALL fail closed: the rule SHALL be treated as +unaccounted. + +#### Scenario: A renamed and loosened Vale rule fails check as one rename + +- **WHEN** reconcile returns vale rule `bar-2` in `unknown` with `copyOf.ruleId` `foo-1` and `copyOf.files` listing `.vale.ini` changed, and returns `foo-1` as `missing`, and the organization's plan is not known to exclude rule recovery +- **THEN** `bar-2` SHALL NOT run +- **AND** `check` SHALL exit non-zero with one message saying `bar-2` is a copy of `foo-1`, which was deleted, naming `.vale.ini` as changed and `taskless rule restore foo-1` +- **AND** the CLI SHALL NOT print a separate warning that `foo-1` is missing + +#### Scenario: A copy beside its present source fails check + +- **WHEN** reconcile returns sg rule `bar-2` in `unknown` with `copyOf.ruleId` `foo-1`, and `foo-1` is not `missing` +- **THEN** `bar-2` SHALL NOT run +- **AND** `check` SHALL exit non-zero naming `bar-2` as a copy of `foo-1` + +#### Scenario: A runtime copy is not executed and does not fail + +- **WHEN** reconcile returns a runtime rule in `unknown` with `copyOf` +- **THEN** the rule SHALL NOT execute +- **AND** its skip reason SHALL name the source rule +- **AND** the exit code SHALL NOT change because of it + +#### Scenario: An unreadable copyOf fails closed + +- **WHEN** reconcile returns a rule in `unknown` whose `copyOf` is present but has no string `ruleId` +- **THEN** the rule SHALL NOT run or execute +- **AND** `check` SHALL exit non-zero naming it diff --git a/openspec/changes/cli-plan-aware-recovery/specs/cli-rule-recovery/spec.md b/openspec/changes/cli-plan-aware-recovery/specs/cli-rule-recovery/spec.md new file mode 100644 index 00000000..c4ac3da2 --- /dev/null +++ b/openspec/changes/cli-plan-aware-recovery/specs/cli-rule-recovery/spec.md @@ -0,0 +1,126 @@ +## ADDED Requirements + +### Requirement: Recovery suggestions follow the plan + +The CLI SHALL read the acting organization's `entitlements.restoreRules` from the +`GET /cli/api/v2/whoami` response it already fetches to resolve the organization, and SHALL +NOT make another request for it. The value SHALL be treated as tri-state: + +- `true`: the plan includes rule recovery. +- `false`: the plan is known to exclude rule recovery. +- unknown: whoami failed, the matched organization carried no `entitlements` or no boolean + `restoreRules`, or no organization matched the repository's remotes and the CLI fell back + to the token's claim. + +Only `false` SHALL change what the CLI suggests. Wherever the CLI would name +`taskless rule restore ` as the way to repair a rule (an `unsafe` or `missing` rule +reported by `check`, or the source of a rename), a `false` plan SHALL instead be told that +restoring rules is not included in the plan, and given git steps for the rule's directory +under `.taskless/rules/`: a `git log` command that lists the commits that changed it, and a +`git restore --source=` command that puts it back as of one of them. When the rule's +engine is not known, the directory SHALL be given as a pathspec matching the rule id under any +engine. `true` and unknown SHALL produce the suggestions the CLI produced before this +requirement. + +This is a suggestion, never a gate. `rule restore` and `rule rollback` SHALL call the service +whatever `restoreRules` says, and SHALL relay a plan refusal as "A plan refusal is an answer, +not a failure of the service" requires. `--json` output SHALL NOT change. + +#### Scenario: An edited rule on a plan without recovery gets git steps + +- **WHEN** `check` reports sg rule `no-eval-3fa9c21b` as `unsafe` and `restoreRules` is `false` +- **THEN** the message SHALL say restoring rules is not included in the plan +- **AND** SHALL give `git log` and `git restore --source=` commands for `.taskless/rules/sg/no-eval-3fa9c21b/` +- **AND** SHALL NOT name `taskless rule restore` + +#### Scenario: A missing rule of unknown engine gets a pathspec for any engine + +- **WHEN** `check` reports rule `foo-1` as `missing` with no known engine and `restoreRules` is `false` +- **THEN** the git steps SHALL name a pathspec matching `foo-1` under any engine directory in `.taskless/rules/` + +#### Scenario: A rename on a plan without recovery gets git steps for the source + +- **WHEN** `check` reports vale rule `bar-2` as a copy of `foo-1`, `foo-1` is `missing`, and `restoreRules` is `false` +- **THEN** the message SHALL give the git steps for `foo-1`'s directory, then say to delete `.taskless/rules/vale/bar-2/` +- **AND** SHALL NOT name `taskless rule restore` + +#### Scenario: Unknown keeps today's suggestion + +- **WHEN** whoami fails, or the matched organization has no `entitlements`, or no organization matches the repository +- **THEN** every suggestion SHALL name `taskless rule restore ` as before + +#### Scenario: The entitlement never blocks a recovery command + +- **WHEN** `restoreRules` is `false` and the user runs `taskless rule restore no-eval-3fa9c21b` +- **THEN** the CLI SHALL call the service's restore endpoint +- **AND** SHALL relay its refusal as it does today + +#### Scenario: No extra request is made + +- **WHEN** `check` or a `rule` subcommand resolves the acting organization +- **THEN** the CLI SHALL call `GET /cli/api/v2/whoami` at most once for that resolution + +## MODIFIED Requirements + +### Requirement: Rule revisions lists what rollback can choose from + +The CLI SHALL provide `taskless rule revisions `, which requires authentication and a +resolvable repository and calls `GET /cli/api/v2/rule/{ruleId}/revisions` with +`repositoryUrl` and, when known, `orgId`. It SHALL write nothing to the working tree. It SHALL +NOT treat the plan as a precondition: the listing is served on every plan, so a plan without +rule recovery SHALL still get the list. + +The CLI SHALL present the revisions in the order the service returns them and SHALL identify +the current revision by its `current` flag, never by its position, because the service appends +a current revision older than the newest ten after them. Human output SHALL show, for each +revision, its `revisionId`, `createdAt`, `delivery`, and `prUrl` when present, SHALL mark the +current one, and SHALL say when `truncated` is `true` that older revisions exist and are listed +on the Taskless dashboard. It SHALL name `rule rollback ` as the way to +make a listed revision current, unless the organization's plan is known to exclude rule +recovery, in which case it SHALL instead say that rolling back is not included in the plan. +The listing itself SHALL be the same either way. + +`404 rule_not_found` SHALL be reported as `RULE_NOT_FOUND`, a rejected token as +`AUTH_REQUIRED`, and any other failure as `NETWORK_ERROR`. + +#### Scenario: Revisions are listed with the current one marked + +- **WHEN** the service returns revisions `r3`, `r2`, `r1` with `r2` current and `truncated: false` +- **THEN** the CLI SHALL list all three in that order and mark `r2` as current +- **AND** SHALL NOT say that older revisions were omitted + +#### Scenario: A current revision older than the listed ones is still marked + +- **WHEN** the service returns ten revisions none of which is current, followed by an eleventh with `current: true` +- **THEN** the CLI SHALL mark the eleventh as current + +#### Scenario: A rule with no current revision marks none + +- **WHEN** the service returns a single revision with `current: false`, delivered by a pull request that has not merged +- **THEN** the CLI SHALL list it with its `prUrl`, mark no revision as current, and say that the rule has no current revision until a pull request delivering it merges + +#### Scenario: Truncation is reported + +- **WHEN** the service returns `truncated: true` +- **THEN** the CLI SHALL say that older revisions exist and are listed on the Taskless dashboard + +#### Scenario: A plan without recovery still lists revisions + +- **WHEN** the organization's plan does not include rule recovery and the service returns a listing +- **THEN** the CLI SHALL print the listing and exit zero + +#### Scenario: A plan known to exclude recovery is not told to roll back + +- **WHEN** the organization's `restoreRules` entitlement is `false` and the service returns a listing +- **THEN** the CLI SHALL print the listing +- **AND** SHALL say that rolling back is not included in the plan, and SHALL NOT name `rule rollback` + +#### Scenario: An unknown plan is told to roll back + +- **WHEN** the organization's `restoreRules` entitlement is unknown and the service returns a listing +- **THEN** the CLI SHALL name `rule rollback ` + +#### Scenario: An unknown rule is reported as not found + +- **WHEN** the service answers `404 rule_not_found` +- **THEN** the CLI SHALL exit non-zero with `RULE_NOT_FOUND` diff --git a/openspec/changes/cli-plan-aware-recovery/tasks.md b/openspec/changes/cli-plan-aware-recovery/tasks.md new file mode 100644 index 00000000..45bf2eb5 --- /dev/null +++ b/openspec/changes/cli-plan-aware-recovery/tasks.md @@ -0,0 +1,24 @@ +## 1. Resolve the entitlement + +- [x] 1.1 Replace `resolveOrgSubject` with `resolveActingOrg` in `auth/org.ts`, returning `{ subject, restoreRules? }` from the one whoami call; `restoreRules` only when an organization matched and its `entitlements.restoreRules` is a boolean; verify with `org.test.ts` cases for `true`, `false`, absent `entitlements`, whoami failure, and no matching organization +- [x] 1.2 Add `restoreRules?: boolean` to `Identity` and set it in `resolveIdentity`; verify `pnpm typecheck` passes and whoami is still called once per resolution + +## 2. Suggestions in check + +- [ ] 2.1 Replace `applyVerdicts`' `restoreCommand` with a `recovery({ ruleId, engine?, purpose })` sentence callback, and add the pure renderer for the restore and git variants (engine-less `missing` uses the quoted any-engine pathspec); verify every existing `verdicts.test.ts` case passes unchanged with `restoreRules` unknown +- [ ] 2.2 Pass the tri-state from `resolveActingOrg` into the callback in `plan-check.ts`; verify with `verdicts.test.ts` cases for `unsafe` (runtime and static), `missing` (with and without engine), and a rename, each under `false` giving git steps and not naming `rule restore` + +## 3. Suggestions in rule revisions + +- [ ] 3.1 Give `describeRevisions` an optional `restoreRules` and pass `identity.restoreRules` from `commands/rules.ts`; verify with `rule-recovery.test.ts` that `false` keeps the listing and replaces only the closing line, and unknown names `rule rollback` +- [ ] 3.2 Confirm `rule restore` / `rule rollback` still call the service under `restoreRules: false`; verify with a `rule-recovery.test.ts` case that relays the refusal + +## 4. Guidance + +- [ ] 4.1 Update `agent/check.md` (v5) and `agent/recover-rule.md` (v3) as design.md describes, hand-edited, no prettier; verify `prompts.test.ts` and `recipe-cross-references.test.ts` pass +- [ ] 4.2 Add a patch changeset for `@taskless/cli` on unit 2, extended on unit 3; verify it is `patch` (pre-1.0) + +## 5. Verify + +- [ ] 5.1 Run `pnpm typecheck`, `pnpm lint`, and `pnpm test`; all pass +- [ ] 5.2 Pre-archive scenario check for all three capabilities, then archive the change on this PR (`pnpm openspec archive cli-plan-aware-recovery -y`) diff --git a/packages/cli/src/auth/identity.ts b/packages/cli/src/auth/identity.ts index 148d6e2e..d6723ef7 100644 --- a/packages/cli/src/auth/identity.ts +++ b/packages/cli/src/auth/identity.ts @@ -1,5 +1,5 @@ import { getToken } from "./token"; -import { resolveOrgSubject } from "./org"; +import { resolveActingOrg } from "./org"; import { CLIError } from "../util/cli-error"; import { resolveRepositoryUrl } from "../util/git-remote"; import { getCliPrefix } from "../util/package-manager"; @@ -9,10 +9,15 @@ export interface Identity { token: string; /** * Org subject to send on write calls: the current org's Taskless UUID - * (preferred) or the deprecated numeric `orgId` claim. See `resolveOrgSubject`. + * (preferred) or the deprecated numeric `orgId` claim. See `resolveActingOrg`. */ orgSubject: string | number; repositoryUrl: string; + /** + * Whether the acting org's plan includes rule recovery; `undefined` when + * unknown. Chooses which recovery to suggest, never whether to try one. + */ + restoreRules?: boolean; } /** @@ -21,6 +26,8 @@ export interface Identity { * repo's remotes, falling back to the token's canonical id claim, and finally * to the nil-UUID `NIL_ORG_ID` so a subject is always present * - repositoryUrl: inferred from `git remote get-url origin` + * - restoreRules: the matched org's plan entitlement, from the same `whoami` + * call; absent when unknown * * Throws a `CLIError` carrying a stable `CLIErrorCode` if auth is missing * (`AUTH_REQUIRED`) or the repository URL cannot be resolved, in which case @@ -39,9 +46,16 @@ export async function resolveIdentity(cwd: string): Promise { } const repositoryUrl = await resolveRepositoryUrl(cwd); - const orgSubject = await resolveOrgSubject(cwd, token); + const org = await resolveActingOrg(cwd, token); - return { token, orgSubject, repositoryUrl }; + return { + token, + orgSubject: org.subject, + repositoryUrl, + ...(org.restoreRules === undefined + ? {} + : { restoreRules: org.restoreRules }), + }; } /** @@ -52,7 +66,7 @@ export async function resolveIdentity(cwd: string): Promise { * prose. It is shared by every caller so the mapping cannot drift between * them. * - * `resolveOrgSubject` is deliberately absent from the list of expected + * `resolveActingOrg` is deliberately absent from the list of expected * failures: it cannot throw. `fetchWhoami` swallows every network and HTTP * error and returns `undefined`, and `decodeOrgId` falls back to the nil-UUID * `NIL_ORG_ID`, so a repo with no matching org resolves a subject rather than diff --git a/packages/cli/src/auth/org.ts b/packages/cli/src/auth/org.ts index e41290b6..2960922f 100644 --- a/packages/cli/src/auth/org.ts +++ b/packages/cli/src/auth/org.ts @@ -65,26 +65,50 @@ export async function resolveCurrentOrg( return selectOrgForOwners(ownerUrls, orgs); } +/** The organization the CLI acts as, and what its plan is known to grant. */ +export interface ActingOrg { + /** The org subject to send on write calls. See `resolveActingOrg`. */ + subject: string | number; + /** + * The matched organization's `entitlements.restoreRules`, or `undefined` + * when it is unknown: whoami failed, the organization carried no boolean + * for it, or no organization matched and the subject came from the token. + * A hint for which recovery to suggest, never a gate. The service refuses + * a recovery the plan lacks on its own. + */ + restoreRules?: boolean; +} + /** - * The org subject to send on write calls. Prefers the current org's Taskless - * UUID (`id`), resolved by matching the repo's remotes against `whoami`; falls + * The org to act as. `subject` prefers the current org's Taskless UUID + * (`id`), resolved by matching the repo's remotes against `whoami`; falls * back to the token's canonical id (or its deprecated numeric `orgId` claim) * when whoami is unavailable or no org owns the repo. A new client thus routes * multi-org users correctly, while older single-org behaviour is preserved via * the claim. * - * Never `undefined`: when neither a matched org nor a token claim resolves, - * returns the nil-UUID `NIL_ORG_ID` so the canonical subject is always known - * (unattributed usage is sent under one stable, known org id). + * `subject` is never `undefined`: when neither a matched org nor a token claim + * resolves, it is the nil-UUID `NIL_ORG_ID` so the canonical subject is always + * known (unattributed usage is sent under one stable, known org id). + * + * `restoreRules` rides along from the same whoami call, so knowing the plan + * costs no request of its own. */ -export async function resolveOrgSubject( +export async function resolveActingOrg( cwd: string, token: string -): Promise { +): Promise { const whoami = await fetchWhoami(token); if (whoami && whoami.orgs.length > 0) { const org = await resolveCurrentOrg(cwd, whoami.orgs); - if (org) return org.id; + if (org) { + // Read defensively: the schema says an absent `entitlements` means + // unknown, and an older service may not send it at all. + const restoreRules: unknown = org.entitlements?.restoreRules; + return typeof restoreRules === "boolean" + ? { subject: org.id, restoreRules } + : { subject: org.id }; + } } - return decodeOrgId(token) ?? NIL_ORG_ID; + return { subject: decodeOrgId(token) ?? NIL_ORG_ID }; } diff --git a/packages/cli/src/rules/plan-check.ts b/packages/cli/src/rules/plan-check.ts index 6d7ce628..2db16831 100644 --- a/packages/cli/src/rules/plan-check.ts +++ b/packages/cli/src/rules/plan-check.ts @@ -1,5 +1,5 @@ import { getToken } from "../auth/token"; -import { resolveOrgSubject } from "../auth/org"; +import { resolveActingOrg } from "../auth/org"; import { reconcileRules } from "../api/v2"; import { resolveRepositoryUrl } from "../util/git-remote"; import { getCliPrefix } from "../util/package-manager"; @@ -191,9 +191,9 @@ export async function planCheck( }; } - const orgSubject = await resolveOrgSubject(cwd, token); + const org = await resolveActingOrg(cwd, token); const outcome = await reconcileRules(token, { - orgId: orgSubject, + orgId: org.subject, repositoryUrl, rules: report.rules.map(({ ruleId, files }) => ({ ruleId, files })), }); diff --git a/packages/cli/test/error-envelope.test.ts b/packages/cli/test/error-envelope.test.ts index 2c738b80..a523533d 100644 --- a/packages/cli/test/error-envelope.test.ts +++ b/packages/cli/test/error-envelope.test.ts @@ -6,7 +6,7 @@ import { join, resolve } from "node:path"; import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { resolveOrgSubject } from "../src/auth/org"; +import { resolveActingOrg } from "../src/auth/org"; import { cliRejectionToResult } from "./support/spawn-cli"; const execFileAsync = promisify(execFile); @@ -290,7 +290,7 @@ describe("standardized error envelope (--json)", () => { * rewording any of these strings fails a test instead of silently telling a * machine consumer to log in when the real problem is the git remote. * - * Origin 4, a `resolveOrgSubject` failure, is absent on purpose and is + * Origin 4, a `resolveActingOrg` failure, is absent on purpose and is * covered by its own test below: it cannot throw. */ describe("resolveIdentity failure codes (--json)", () => { @@ -501,14 +501,14 @@ describe("resolveIdentity failure codes (--json)", () => { }); /** - * Origin 4 of #181: a `resolveOrgSubject` failure. There is no such failure + * Origin 4 of #181: a `resolveActingOrg` failure. There is no such failure * to give a code to. `fetchWhoami` returns `undefined` on any network or HTTP * error, and `decodeOrgId` falls back to the nil-UUID `NIL_ORG_ID`, so the * step resolves a subject rather than throwing. This test holds that shape in - * place: if `resolveOrgSubject` ever grows a throw, it needs a code of its + * place: if `resolveActingOrg` ever grows a throw, it needs a code of its * own and this test says so by failing. */ -describe("resolveOrgSubject", () => { +describe("resolveActingOrg", () => { let cwd: string; beforeEach(async () => { @@ -525,7 +525,7 @@ describe("resolveOrgSubject", () => { process.env.TASKLESS_API_URL = "http://127.0.0.1:1/cli"; try { await expect( - resolveOrgSubject(cwd, "not-a-real-token") + resolveActingOrg(cwd, "not-a-real-token") ).resolves.toBeDefined(); } finally { if (previous === undefined) { diff --git a/packages/cli/test/org.test.ts b/packages/cli/test/org.test.ts index 2fea8103..212d7a01 100644 --- a/packages/cli/test/org.test.ts +++ b/packages/cli/test/org.test.ts @@ -6,7 +6,7 @@ import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { fetchWhoami } from "../src/auth/whoami"; import { - resolveOrgSubject, + resolveActingOrg, selectOrgForOwners, type WhoamiOrg, } from "../src/auth/org"; @@ -39,6 +39,15 @@ const org = (id: string, url: string, source = "github"): WhoamiOrg => url, }) as WhoamiOrg; +const entitled = (restoreRules: boolean): WhoamiOrg => ({ + ...org("uuid-acme", "https://github.com/acme"), + entitlements: { + remoteGeneration: true, + runtimeSignatures: true, + restoreRules, + }, +}); + describe("selectOrgForOwners", () => { const orgs = [ org("uuid-acme", "https://github.com/acme"), @@ -78,7 +87,7 @@ describe("selectOrgForOwners", () => { }); }); -describe("resolveOrgSubject", () => { +describe("resolveActingOrg", () => { let directory: string; const token = makeJwt({ orgId: 4242 }); @@ -100,25 +109,84 @@ describe("resolveOrgSubject", () => { mockedFetchWhoami.mockResolvedValue( whoamiWith([org("uuid-acme", "https://github.com/acme")]) ); - expect(await resolveOrgSubject(directory, token)).toBe("uuid-acme"); + await expect(resolveActingOrg(directory, token)).resolves.toMatchObject({ + subject: "uuid-acme", + }); }); it("falls back to the numeric claim when no whoami org matches the repo", async () => { mockedFetchWhoami.mockResolvedValue( whoamiWith([org("uuid-other", "https://github.com/other")]) ); - expect(await resolveOrgSubject(directory, token)).toBe(4242); + await expect(resolveActingOrg(directory, token)).resolves.toMatchObject({ + subject: 4242, + }); }); it("falls back to the numeric claim when whoami is unavailable", async () => { mockedFetchWhoami.mockResolvedValue(WHOAMI_UNAVAILABLE); - expect(await resolveOrgSubject(directory, token)).toBe(4242); + await expect(resolveActingOrg(directory, token)).resolves.toMatchObject({ + subject: 4242, + }); }); it("falls back to the nil-UUID when whoami is unavailable and the token has no org claim", async () => { mockedFetchWhoami.mockResolvedValue(WHOAMI_UNAVAILABLE); - expect(await resolveOrgSubject(directory, makeJwt({ sub: "u" }))).toBe( - "00000000-0000-0000-0000-000000000000" + await expect( + resolveActingOrg(directory, makeJwt({ sub: "u" })) + ).resolves.toMatchObject({ + subject: "00000000-0000-0000-0000-000000000000", + }); + }); + + it.each([true, false])( + "carries the matched org's restoreRules (%s) from the same whoami call", + async (restoreRules) => { + mockedFetchWhoami.mockResolvedValue(whoamiWith([entitled(restoreRules)])); + expect(await resolveActingOrg(directory, token)).toEqual({ + subject: "uuid-acme", + restoreRules, + }); + expect(mockedFetchWhoami).toHaveBeenCalledTimes(1); + } + ); + + it("leaves restoreRules unknown when the matched org has no entitlements", async () => { + mockedFetchWhoami.mockResolvedValue( + whoamiWith([org("uuid-acme", "https://github.com/acme")]) ); + expect(await resolveActingOrg(directory, token)).toEqual({ + subject: "uuid-acme", + }); + }); + + it("leaves restoreRules unknown when entitlements carries no boolean for it", async () => { + mockedFetchWhoami.mockResolvedValue( + whoamiWith([ + { + ...org("uuid-acme", "https://github.com/acme"), + entitlements: { remoteGeneration: true } as WhoamiOrg["entitlements"], + }, + ]) + ); + expect(await resolveActingOrg(directory, token)).toEqual({ + subject: "uuid-acme", + }); + }); + + it("leaves restoreRules unknown when no org matches, even if another org has it", async () => { + mockedFetchWhoami.mockResolvedValue( + whoamiWith([ + { ...entitled(false), url: "https://github.com/other" } as WhoamiOrg, + ]) + ); + expect(await resolveActingOrg(directory, token)).toEqual({ + subject: 4242, + }); + }); + + it("leaves restoreRules unknown when whoami is unavailable", async () => { + mockedFetchWhoami.mockResolvedValue(WHOAMI_UNAVAILABLE); + expect(await resolveActingOrg(directory, token)).toEqual({ subject: 4242 }); }); });