Skip to content

refactor(schema): explicit scaffold state and per-command migrate/refuse policy - #634

Merged
theCodeDrift merged 3 commits into
mainfrom
refactor/626-explicit-schema-policy
Nov 8, 2026
Merged

theCodeDrift merged 3 commits into
mainfrom
refactor/626-explicit-schema-policy

Conversation

@theCodeDrift

@theCodeDrift theCodeDrift commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

Stack (root → tip):

Every command now names what it may do to .taskless/, instead of getting a policy from whichever helper it happened to call. That accident is how onboard and report kept migrating after only init was meant to (#621). Stacked on #630.

What changed

  • One classifier. classifySchema(cwd) in filesystem/migrate.ts returns absent, behind, current or ahead. It keeps pendingMigration's directory-keyed rule: a .taskless/ with no manifest is behind from version 0.

  • One policy switch. ensureSchema(cwd, policy, { onNotice, allowVersionMismatches }) in filesystem/directory.ts, with SchemaPolicy = "migrate" | "scaffold-if-absent" | "refuse":

    absent behind current ahead (no override) ahead (override)
    migrate scaffold migrate nothing SCAFFOLD_VERSION_MISMATCH proceed
    scaffold-if-absent scaffold SCAFFOLD_MIGRATION_REQUIRED nothing SCAFFOLD_VERSION_MISMATCH proceed
    refuse nothing SCAFFOLD_MIGRATION_REQUIRED nothing SCAFFOLD_VERSION_MISMATCH proceed
  • The "ahead" message is built once, in schemaAheadError. runMigrations and ensureSchema both throw it. The "behind" refusal is likewise schemaMigrationRequiredError. requireCurrentSchema and pendingMigration are removed. ensureTasklessDirectory/runMigrations stay as the mechanism behind migrate.

  • Call sites:

    • migrate: init (non-interactive) and the wizard.
    • scaffold-if-absent: validate, verify, onboard, report. Each used to call requireCurrentSchema and then ensureTasklessDirectory. Now it makes one call, with the same --json notice suppression.
    • refuse: check, update --rules, and the rule-writing commands (below).
  • grep -rn "ensureTasklessDirectory\|requireCurrentSchema" packages/cli/src/commands now returns nothing.

Rule commands: decision

rule create, rule improve, rule restore and rule rollback reach writeServedRule → ensureTasklessDirectory. On a behind scaffold that ran the migration after the service call, with the migration prose on stderr even under --json. They now refuse with SCAFFOLD_MIGRATION_REQUIRED (or SCAFFOLD_VERSION_MISMATCH), under policy refuse. The check runs in commands/rules.ts before identity is resolved or the service is called, including on --resume. refuse was chosen over scaffold-if-absent so that an absent .taskless/ is still created by the write itself, only once there is a rule to write, as before. rule revisions only reads from the service, so it is unchanged.

rules/files.ts is untouched, so this does not conflict with #632's writeServedRule split. rules/verify.ts's internal ensureTasklessDirectory also stays: as #630's audit shows, every caller already refuses a behind or ahead scaffold before reaching it.

This is the one user-visible change, so it gets an OpenSpec change, archived here. It ADDs one requirement to cli-rules (rule-writing commands refuse) and one to cli-taskless-bootstrap (the states and policies). Both are ADDED, nothing MODIFIED, and scenario counts went 61→65 for each. The existing changeset .changeset/onboard-report-refuse-migrations.md gains one line. No second changeset.

--allow-version-mismatches with init/the wizard: kept as-is

With the override, an ahead scaffold still passes migrate and the install writes into its manifest. I left that alone on purpose:

  • Every manifest write (writeInstallState, stampRulelessProject) merges over the raw object, so version and any field this CLI does not know survive.
  • The override is an explicit request to proceed.
  • This repository's pnpm cli sets TASKLESS_ALLOW_VERSION_MISMATCHES=1 on every run, so skipping the write would change the dev loop.

ensureSchema now returns { state, migrated }, so a caller can tell ahead apart if this is revisited. Both call sites carry a comment explaining the choice.

Testing

  • test/schema-policy.test.ts (new) is a table test over every (state × policy) pair: absent, behind, behind-with-no-manifest, current, ahead. It checks the classifier, the outcome, that a refusal leaves the manifest and emits no notices, and that the override lets ahead through untouched under each policy.
  • test/rule-commands-refuse-migration.test.ts (new) runs the built CLI. Each of rule create/improve/restore/rollback on a v3 scaffold returns the SCAFFOLD_MIGRATION_REQUIRED envelope naming init and leaves the tree unchanged (manifest still 3, flat rule not moved, no rules/sg/). Each refuses a v99 scaffold.
  • The existing refusal and migration tests (no-implicit-migration, migrate-engine-layout, onboard, …) pass unchanged.
  • Six rule-command test files (rule-recovery, rule-create-entitlement, rule-poll-transient, rule-guard-json-envelope, rule-from, error-envelope) seeded version: "2026-03-03". That reads as version 0, so they depended on the silent migration this PR removes. They now seed LATEST_SCHEMA_VERSION, the same fix fix(onboard): refuse a behind .taskless/ in onboard and report instead of migrating #630 applied to onboard.test.ts.
  • pnpm typecheck, pnpm lint, and pnpm --filter @taskless/cli test (3008 passed, 11 skipped) all pass locally.

Fixes #626

Built on top of #630

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 12s —— View job


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

No prior inline threads, review summaries, or top-level comments exist on this PR (.prior-review.json was empty of substantive content other than the trigger comment), so this is the first review of this PR — everything below is assessed fresh and prefixed [New] only where worth calling out; otherwise it's a clean pass.

  • Read .prior-review.json
  • Fetch PR diff and metadata
  • Read changed files in full where needed
  • Review correctness, security, performance, test adequacy, clarity
  • Post findings

Summary

This is a clean, well-scoped refactor. I read the full diff plus the complete contents of filesystem/migrate.ts, filesystem/directory.ts, commands/rules.ts, and the other touched commands (check.ts, init.ts, onboard.ts, report.ts, verify.ts, wizard/index.ts) to check call-site wiring beyond what the diff hunks show.

Correctness

  • classifySchema/ensureSchema's state×policy matrix matches the table in the PR description exactly, and test/schema-policy.test.ts exercises every cell plus the override — I traced the absent/behind/ahead branches in directory.ts:101-126 against the test table and they agree.
  • Verified the one subtle case: an absent scaffold under migrate/scaffold-if-absent goes through ensureTasklessDirectory → runMigrations, which reads the (nonexistent) manifest as version 0 and runs every migration — so migrated is always populated for that cell, consistent with the test's "scaffolded" expectation.
  • commands/rules.ts: schemaOrFail is called after resumeRequestId but before identityOrFail in both create and improve, so --resume is covered, matching the PR's claim. runForIssuedRule's new { writes?: boolean } option is correctly threaded — runRecovery (backing restore/rollback) passes writes: true, while revisionsCommand (read-only) does not, matching "rule revisions ... is unchanged."
  • Confirmed by grep that ensureTasklessDirectory/requireCurrentSchema no longer appear anywhere under packages/cli/src/commands, as claimed. The two remaining unconditional ensureTasklessDirectory calls (rules/files.ts:80, rules/verify.ts:521) are each only reachable after a command-level ensureSchema call has already refused a non-current scaffold, so they're no-ops on the happy path — the PR's reasoning holds up when traced through rules/recover.ts and verify.ts's runOverPath.
  • The ahead-with-override pass-through in init.ts/wizard/index.ts is intentional and documented at both call sites; the manifest-merge behavior in writeManifest/writeInstallState does preserve unknown fields including version, backing up the stated rationale.

Tests

  • schema-policy.test.ts and rule-commands-refuse-migration.test.ts are thorough — table-driven coverage of the matrix, explicit assertions that a refusal leaves the manifest version and tree untouched, and coverage of the behind-with-no-manifest edge case.
  • The six fixture files that moved from seeding version: "2026-03-03" to LATEST_SCHEMA_VERSION are consistent with the stated reason (the old string read as version 0, silently relying on migration-on-the-fly).

Docs/specs

  • The OpenSpec archive, cli-rules/cli-taskless-bootstrap spec deltas, and changeset addition all match the described behavior change; nothing in the specs contradicts the code.

No correctness, security, or clarity issues found. Nothing to flag as [New] beyond what's already covered above — this is a straightforward, internally consistent refactor backed by solid tests.

@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @github-actions — "Claude finished @theCodeDrift's task in 2m 12s"
#634 (comment)

Thanks. The review raised no findings, so nothing changes.

— AI Coding Agent

@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 force-pushed the refactor/626-explicit-schema-policy branch from b891ef4 to 0351c38 Compare November 8, 2026 05:48
Base automatically changed from fix/621-refuse-migrations-in-reporting-commands to main November 8, 2026 05:52
…n explicit policy

Add classifySchema (absent/behind/current/ahead) and one ensureSchema(cwd,
policy) with policies migrate, scaffold-if-absent and refuse. The ahead and
behind refusal messages are each built in one place. check, validate,
verify, onboard, report, init and the wizard now call ensureSchema;
requireCurrentSchema and pendingMigration are gone.

Refs #626
… and rollback

These reached the rule write path, which migrates a behind .taskless/
before writing, so they moved and deleted tracked files after the service
call. They now refuse with SCAFFOLD_MIGRATION_REQUIRED (policy refuse)
before resolving identity. Rule-command test fixtures seeded an old
date-string version, which reads as 0, and relied on that silent
migration; they now seed the current schema version.

Refs #626
@theCodeDrift
theCodeDrift force-pushed the refactor/626-explicit-schema-policy branch from 0351c38 to 11c838f Compare November 8, 2026 05:52
@theCodeDrift
theCodeDrift merged commit 6153acf into main Nov 8, 2026
4 checks passed
@theCodeDrift
theCodeDrift deleted the refactor/626-explicit-schema-policy branch November 8, 2026 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant