feat(check): give git steps instead of rule restore on a plan that does not serve it

[theCodeDrift]

Stack (root → tip):

• feat(auth): carry the acting org's restoreRules entitlement from whoami #433
  • ➡️ feat(check): give git steps instead of rule restore on a plan that does not serve it #434 (you are here)
    • feat(rule): stop naming rule rollback on a plan that does not serve it #435

Stack 2 of 3, merging forward. Based on #433.

When the acting organization's restoreRules is exactly false, check no longer tells the user to run taskless rule restore. Every notice that named it gives the git steps for the rule's directory instead. That covers an edited (unsafe) rule, a missing rule, and a rename (a copy whose source is missing). When the plan is unknown, the output is unchanged: the existing verdicts.test.ts and runtime-check.test.ts string assertions pass as they were.

sg rule no-eval-3fa9c21b was edited since Taskless issued it (changed no-eval-3fa9c21b.yml), so it did not run and `check` fails. Restoring rules is not included in your organization's plan, so recover no-eval-3fa9c21b from git: `git log -- .taskless/rules/sg/no-eval-3fa9c21b/` lists the commits that changed it, and `git restore --source=<commit> -- .taskless/rules/sg/no-eval-3fa9c21b/` puts it back as of one of them.

What this PR does

• New rules/recovery-advice.ts, which renders the full recovery sentence for a plan. applyVerdicts takes it in place of the old restoreCommand callback, so verdicts.ts still knows nothing about the CLI prefix or the plan.
• A missing rule whose engine is unknown gets the quoted pattern '.taskless/rules/*/<id>/*'.
• The check agent recipe goes to v5, with a section on these notices and on choosing a commit. For a deleted rule, that means restoring from the parent of the commit that deleted it.
• Adds the changeset. Unit 3 extends it.

What to review

• The git commands were measured, not assumed. In a scratch repository, the exact directory form worked for both log and restore. The first pattern I tried for an unknown engine ended in the directory's /, and git log found no commits with it. Ending it in /* found both the add and the delete, and restore from the delete's parent brought the files back. The shipped pattern uses /*.
• plan-check.ts passes org.restoreRules from the same resolveActingOrg call that supplies the reconcile orgId. The new end-to-end test in runtime-check.test.ts serves a whoami with restoreRules: false, and asserts both the orgId sent to reconcile and the git steps in the notices.
• Wording of the git sentence, which is user-facing.

Refs GH-425

[theCodeDrift]

@claude /review

[claude]

Claude finished @theCodeDrift's task in 6m 8s —— View job

---

Review mode: incremental — read 0 prior review thread(s) before reviewing.

.prior-review.json had no prior threads, reviews, or comments (other than the trigger itself), so this is the first review of this PR. I assessed the whole diff.

Todo

• Read prior review data — empty, first review
• Gather PR diff (gh pr diff 434)
• Read changed files in full context (plan-check.ts, verdicts.ts, recovery-advice.ts, check.md, both test files, auth/org.ts, openspec design.md/tasks.md)
• Review correctness, security, performance, tests, clarity
• Post inline comments for concrete issues
• Post final summary

Summary

The core change is solid. recoveryAdvice is a small, well-factored pure function, applyVerdicts threads it through all three notice sites (unsafe, missing, rename) correctly, and every call site of the old restoreCommand/applyVerdicts signature was updated consistently. I traced the git-pathspec claim ('.taskless/rules/*/<ruleId>/*' vs. a plain directory) against how git actually matches glob vs. literal pathspecs, and the reasoning in design.md holds up: a literal directory path is prefix-matched, but a glob pathspec needs the /* suffix to match files, which is why the two forms differ. I hand-traced every new test's expected output string against the actual recovery/applyVerdicts code paths (unsafe static, unsafe runtime, missing with/without engine, rename) and they match exactly, including the worked example in the PR body. The new runtime-check.test.ts end-to-end test correctly wires a whoami mock matching the fixture's github.com/acme remote, and asserts the orgId sent to reconcile came from the same acting-org resolution that supplied restoreRules.

One process finding worth resolving before merge, and a couple of minor/low-severity notes, posted inline:

1. [New] Changeset placed on the wrong PR in the stack (.changeset/plan-aware-recovery.md). The repo's CLAUDE.md is explicit that in a forward-merging stack the changeset belongs on the bottom PR (the one targeting main), because a child branched before the file exists doesn't inherit it, and a changeset missing from the bottom PR means that PR ships with no release note. This PR's stack is #433 → #434 (here) → #435, declared "merging forward," and I confirmed #433 (base: main) currently carries no .changeset/*.md. The changeset is being introduced here, on the middle PR. tasks.md (from feat(auth): carry the acting org's restoreRules entitlement from whoami #433) does explicitly plan "changeset on unit 2, extended on unit 3," so this looks like a deliberate call made at the proposal stage — but it's worth a second look against the standing convention rather than assuming the proposal's plan overrides it silently. See inline comment for the exact rule text.

2. [New] Minor test-coverage gap in verdicts.test.ts: the rename scenario (copy + missing source) is only tested with a known source engine (vale); the quoted any-engine pathspec branch is exercised for a plain missing notice but not combined with a rename. Code looks correct by inspection — this is a coverage suggestion, not a suspected bug.

3. Low-confidence documentation nuance (not inline, informational only): check.md's new "Choosing the commit" guidance says "the newest commit [git log lists] is the one that deleted it, so restore from its parent (<commit>~1)." That's correct for linear history, but ~1 follows the first parent, so on a branch with merge commits the "newest commit touching this path" and "the commit whose first parent chain lets ~1 reach the pre-deletion state" aren't guaranteed to be the same commit. Very edge-case for an agent-facing recipe; flagging only for awareness.

Nothing else stood out — no correctness bugs found in recovery-advice.ts, verdicts.ts, or plan-check.ts beyond what's noted above, and the new tests accurately pin down the described behavior.

[theCodeDrift]

Re: @claude[bot] — "Claude finished @theCodeDrift's task in 6m 8s" (point 3, <commit>~1 in agent/check.md)
#434 (comment)

Declined by the author, after measuring it in a scratch repository. In the first case the rule was deleted on a branch that was merged with --no-ff. git log -- <dir> names the branch's delete commit, and ~1 restores the rule. In the second case the rule was deleted inside the merge commit itself. git log names the merge, and ~1, its first parent, still has the rule and restores it. The advice holds in both. The test-coverage point in this review is addressed in 71bfe3d.

— AI Coding Agent