Skip to content

fix(onboard): refuse a behind .taskless/ in onboard and report instead of migrating - #630

Merged
theCodeDrift merged 2 commits into
mainfrom
fix/621-refuse-migrations-in-reporting-commands
Nov 8, 2026
Merged

theCodeDrift merged 2 commits into
mainfrom
fix/621-refuse-migrations-in-reporting-commands

Conversation

@theCodeDrift

@theCodeDrift theCodeDrift commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

Stack (root → tip):

taskless onboard and taskless report migrated .taskless/ as a side effect, in every mode. Migration 0005 moves and deletes tracked files, so an agent that fetched the onboarding recipe, or drafted a report, rewrote the repository of any project whose scaffold was behind the CLI. check, validate and verify already refuse in that case, and init is meant to be the only command that migrates.

What changed

  • commands/onboard.ts: calls requireCurrentSchema before ensureTasklessDirectory, in every mode (recipe, --force, --mark-complete). A behind scaffold exits 1 with Error (SCAFFOLD_MIGRATION_REQUIRED): … naming init; an ahead one with SCAFFOLD_VERSION_MISMATCH. The code is printed in prose because onboard has no --json envelope, the same form report already uses.
  • commands/report.ts: the same check, in all three modes (draft, --from, --delete), routed through the command's existing guarded/fail path, so --json gets the { ok: false, code, message } envelope and prose gets Error (CODE): ….
  • Both still call ensureTasklessDirectory afterwards, so a project with no .taskless/ is scaffolded as before. That call can no longer migrate, because the refusal already ran. report keeps its --json-suppressing onNotice. onboard has no JSON mode, so it keeps the default notice.
  • OpenSpec: cli-onboard and cli-report both required bootstrapping .taskless/. The change restates each requirement in full with the refusal and two new scenarios, and is archived in this PR (single-PR delivery). Scenario counts before and after archive: 42→44 and 53→55.
  • Changeset: patch.

Testing

  • test/no-implicit-migration.test.ts: on a version-3 scaffold, every mode of onboard and report exits 1 with SCAFFOLD_MIGRATION_REQUIRED on stderr, prints nothing on stdout, and leaves the tree unchanged (manifest still version 3, flat rule not moved, no rules/sg/). report --json returns the envelope. Both commands refuse a version-99 scaffold. With no .taskless/ at all, onboard scaffolds to the latest version and report --json drafts with no migration prose on stderr.
  • test/onboard.test.ts: the fixtures seeded version: 2 and relied on onboard migrating them. They now seed LATEST_SCHEMA_VERSION.
  • pnpm typecheck, pnpm lint, and pnpm --filter @taskless/cli test (2977 passed) all pass locally.

Audit of the other ensureTasklessDirectory callers

  • rules/verify.ts:521 (verifyRule) is only reached through rules/inspect.ts, whose callers are commands/verify.ts (validate/verify) and report/draft.ts (report draft and --from). All three now refuse a behind or ahead scaffold before getting there. On a current scaffold the call is a no-op, and on an absent one the command has already scaffolded it. No other caller exists, so it cannot migrate.
  • rules/files.ts:80 (writeServedRule) is reached by rule create/rule improve (rules/generate.ts) and rule restore/rule rollback (rules/recover.ts). commands/rules.ts never calls requireCurrentSchema, so on a behind scaffold these commands still migrate, after the service call, with migration notices going to stderr even under --json. That breaks the "only init migrates" rule. These are writing commands, and the migration produces the layout the served rule is written into, so the output is correct, but the user did not ask for the rewrite of unrelated tracked files. My conclusion is that they should refuse, and the natural place to decide that is Make the .taskless/ schema state and each command's migrate/refuse policy explicit #626, which assigns every command an explicit schema policy. I left it out of this PR to keep onboard and report run .taskless/ migrations, which move and delete tracked files #621 scoped, and because Multi-rule generation writes earlier rules before a later one is rejected #623 is changing writeServedRule.

Fixes #621

@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 2m 17s —— View job


Review: fix(onboard): refuse a behind .taskless/ in onboard and report instead of migrating

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained only the triggering @claude /review comment itself, with no prior review threads, review bodies, or top-level comments recorded — this is the first review of this PR.

  • Read prior-review data and PR diff
  • Read full contents of onboard.ts, report.ts, filesystem/migrate.ts, filesystem/directory.ts
  • Review correctness, security, performance, test adequacy, clarity
  • Checked for inline findings (none met the bar for an inline comment)
  • Post final top-level summary

Assessment: no concrete issues found.

Walked the full chain for both commands:

  • commands/onboard.ts: requireCurrentSchema(cwd) now runs before ensureTasklessDirectory, wrapped in a try/catch that prints Error (CODE): message to stderr, sets exitCode = 1, and rethrows a CLIError with reported: true — matching the reported-flag convention the top-level handler in index.ts:237 relies on to avoid double-printing. Runs identically across the recipe, --force, and --mark-complete paths, before any of them branch.
  • commands/report.ts: the same check is routed through the command's existing guarded/fail helpers, so it gets prose or the { ok:false, code, message } envelope exactly like every other refusal in that file, and fires telemetry.capture("cli_report_refused", { code }) the same way. All three modes (draft, --from, --delete) hit it before ensureTasklessDirectory.
  • filesystem/migrate.ts's requireCurrentSchema (pre-existing, already used by check/verify/init) correctly handles all three states: absent .taskless/ (no-op), ahead (SCAFFOLD_VERSION_MISMATCH, honoring the existing --allow-version-mismatches override), and behind (SCAFFOLD_MIGRATION_REQUIRED, naming init). Confirmed ensureTasklessDirectory/runMigrations is now unreachable in a migrating state for these two commands, since the refusal always runs first — a behind scaffold never reaches the migrator through either command.
  • Test coverage (no-implicit-migration.test.ts) drives the actual built CLI binary, not a mock, across every mode of both commands against behind (v3), ahead (v99), and absent scaffolds, and asserts both the exit code/stderr/stdout content and that the working tree is byte-for-byte untouched (manifest version, rules/no-eval.yml still flat, no rules/sg/). report --json's envelope and the no-.taskless/ scaffolding path are both covered.
  • onboard.test.ts's fixture updates from hard-coded version: 2 to LATEST_SCHEMA_VERSION are complete — grepped the file for stray numeric version: literals and found none left behind that would now trip the new refusal unintentionally.
  • OpenSpec: the two modified requirements in openspec/specs/cli-onboard and cli-report match the archived change's delta specs verbatim, and match the shipped error strings/codes.
  • The PR body's own audit of rules/files.ts's writeServedRule (reached by rule create/improve/restore/rollback) is accurate — commands/rules.ts indeed never calls requireCurrentSchema, so those commands still migrate silently on a behind scaffold. Deferring that to Make the .taskless/ schema state and each command's migrate/refuse policy explicit #626 is a reasonable scope boundary given it's a different, write-oriented set of commands and Multi-rule generation writes earlier rules before a later one is rejected #623 is already touching that function.

No correctness, security, or clarity concerns surfaced. Nothing to flag inline.

@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @github-actions — "Claude finished @theCodeDrift's task in 2m 17s … Assessment: no concrete issues found."
#630 (comment)

Thanks. No changes needed. rule create/improve/restore/rollback still migrate, and that is deferred to #626 as the PR body says.

— AI Coding Agent

…d of migrating

onboard and report ran ensureTasklessDirectory in every mode, so fetching
the onboarding recipe or drafting a report migrated .taskless/, and 0005
moves and deletes tracked files. Both now call requireCurrentSchema first,
like check and verify, and still scaffold a .taskless/ that is absent.
@theCodeDrift
theCodeDrift force-pushed the fix/621-refuse-migrations-in-reporting-commands branch from 9bec323 to 7926b60 Compare November 8, 2026 05:48
@theCodeDrift
theCodeDrift merged commit 33a77a5 into main Nov 8, 2026
4 checks passed
@theCodeDrift
theCodeDrift deleted the fix/621-refuse-migrations-in-reporting-commands branch November 8, 2026 05:52
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.

onboard and report run .taskless/ migrations, which move and delete tracked files

1 participant