Repository navigation
refactor(schema): explicit scaffold state and per-command migrate/refuse policy - #634
Conversation
|
@claude /review |
|
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 (
SummaryThis is a clean, well-scoped refactor. I read the full diff plus the complete contents of Correctness
Tests
Docs/specs
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. |
Thanks. The review raised no findings, so nothing changes. — AI Coding Agent |
9bec323 to
7926b60
Compare
b891ef4 to
0351c38
Compare
…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
0351c38 to
11c838f
Compare
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 howonboardandreportkept migrating after onlyinitwas meant to (#621). Stacked on #630.What changed
One classifier.
classifySchema(cwd)infilesystem/migrate.tsreturnsabsent,behind,currentorahead. It keepspendingMigration's directory-keyed rule: a.taskless/with no manifest isbehindfrom version 0.One policy switch.
ensureSchema(cwd, policy, { onNotice, allowVersionMismatches })infilesystem/directory.ts, withSchemaPolicy = "migrate" | "scaffold-if-absent" | "refuse":migrateSCAFFOLD_VERSION_MISMATCHscaffold-if-absentSCAFFOLD_MIGRATION_REQUIREDSCAFFOLD_VERSION_MISMATCHrefuseSCAFFOLD_MIGRATION_REQUIREDSCAFFOLD_VERSION_MISMATCHThe "ahead" message is built once, in
schemaAheadError.runMigrationsandensureSchemaboth throw it. The "behind" refusal is likewiseschemaMigrationRequiredError.requireCurrentSchemaandpendingMigrationare removed.ensureTasklessDirectory/runMigrationsstay as the mechanism behindmigrate.Call sites:
migrate:init(non-interactive) and the wizard.scaffold-if-absent:validate,verify,onboard,report. Each used to callrequireCurrentSchemaand thenensureTasklessDirectory. Now it makes one call, with the same--jsonnotice suppression.refuse:check,update --rules, and the rule-writing commands (below).grep -rn "ensureTasklessDirectory\|requireCurrentSchema" packages/cli/src/commandsnow returns nothing.Rule commands: decision
rule create,rule improve,rule restoreandrule rollbackreachwriteServedRule→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 withSCAFFOLD_MIGRATION_REQUIRED(orSCAFFOLD_VERSION_MISMATCH), under policyrefuse. The check runs incommands/rules.tsbefore identity is resolved or the service is called, including on--resume.refusewas chosen overscaffold-if-absentso that an absent.taskless/is still created by the write itself, only once there is a rule to write, as before.rule revisionsonly reads from the service, so it is unchanged.rules/files.tsis untouched, so this does not conflict with #632'swriteServedRulesplit.rules/verify.ts's internalensureTasklessDirectoryalso 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 tocli-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.mdgains one line. No second changeset.--allow-version-mismatcheswithinit/the wizard: kept as-isWith the override, an ahead scaffold still passes
migrateand the install writes into its manifest. I left that alone on purpose:writeInstallState,stampRulelessProject) merges over the raw object, soversionand any field this CLI does not know survive.pnpm clisetsTASKLESS_ALLOW_VERSION_MISMATCHES=1on every run, so skipping the write would change the dev loop.ensureSchemanow returns{ state, migrated }, so a caller can tellaheadapart 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 letsaheadthrough untouched under each policy.test/rule-commands-refuse-migration.test.ts(new) runs the built CLI. Each ofrule create/improve/restore/rollbackon a v3 scaffold returns theSCAFFOLD_MIGRATION_REQUIREDenvelope naminginitand leaves the tree unchanged (manifest still 3, flat rule not moved, norules/sg/). Each refuses a v99 scaffold.no-implicit-migration,migrate-engine-layout,onboard, …) pass unchanged.rule-recovery,rule-create-entitlement,rule-poll-transient,rule-guard-json-envelope,rule-from,error-envelope) seededversion: "2026-03-03". That reads as version 0, so they depended on the silent migration this PR removes. They now seedLATEST_SCHEMA_VERSION, the same fix fix(onboard): refuse a behind .taskless/ in onboard and report instead of migrating #630 applied toonboard.test.ts.pnpm typecheck,pnpm lint, andpnpm --filter @taskless/cli test(3008 passed, 11 skipped) all pass locally.Fixes #626
Built on top of #630
taskless onboardandtaskless reportmigrated.taskless/as a side effect, in every mode. Migration0005moves 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,validateandverifyalready refuse in that case, andinitis meant to be the only command that migrates.What changed
commands/onboard.ts: callsrequireCurrentSchemabeforeensureTasklessDirectory, in every mode (recipe,--force,--mark-complete). A behind scaffold exits 1 withError (SCAFFOLD_MIGRATION_REQUIRED): …naminginit; an ahead one withSCAFFOLD_VERSION_MISMATCH. The code is printed in prose becauseonboardhas no--jsonenvelope, the same formreportalready uses.commands/report.ts: the same check, in all three modes (draft,--from,--delete), routed through the command's existingguarded/failpath, so--jsongets the{ ok: false, code, message }envelope and prose getsError (CODE): ….ensureTasklessDirectoryafterwards, so a project with no.taskless/is scaffolded as before. That call can no longer migrate, because the refusal already ran.reportkeeps its--json-suppressingonNotice.onboardhas no JSON mode, so it keeps the default notice.cli-onboardandcli-reportboth 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.patch.Testing
test/no-implicit-migration.test.ts: on a version-3 scaffold, every mode ofonboardandreportexits 1 withSCAFFOLD_MIGRATION_REQUIREDon stderr, prints nothing on stdout, and leaves the tree unchanged (manifest still version 3, flat rule not moved, norules/sg/).report --jsonreturns the envelope. Both commands refuse a version-99 scaffold. With no.taskless/at all,onboardscaffolds to the latest version andreport --jsondrafts with no migration prose on stderr.test/onboard.test.ts: the fixtures seededversion: 2and relied on onboard migrating them. They now seedLATEST_SCHEMA_VERSION.pnpm typecheck,pnpm lint, andpnpm --filter @taskless/cli test(2977 passed) all pass locally.Audit of the other
ensureTasklessDirectorycallersrules/verify.ts:521(verifyRule) is only reached throughrules/inspect.ts, whose callers arecommands/verify.ts(validate/verify) andreport/draft.ts(reportdraft 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 byrule create/rule improve(rules/generate.ts) andrule restore/rule rollback(rules/recover.ts).commands/rules.tsnever callsrequireCurrentSchema, 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 "onlyinitmigrates" 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 changingwriteServedRule.Fixes #621