Skip to content

feat(cli): add migration 0004 for the engine-partitioned .taskless layout - #79

Merged
thecodedrift merged 9 commits into
mainfrom
openspec/partition-engine-1-migration
Aug 4, 2026
Merged

feat(cli): add migration 0004 for the engine-partitioned .taskless layout#79
thecodedrift merged 9 commits into
mainfrom
openspec/partition-engine-1-migration

Conversation

@thecodedrift

@thecodedrift thecodedrift commented Aug 2, 2026

Copy link
Copy Markdown
Member

Stack (root → tip):

Migration 0004 moves .taskless/ to an engine-partitioned layout: ast-grep rules go to sg/rules/ and sg/rule-tests/, the runtime tree to runtime/rules/ and runtime/rule-tests/, and vale/ is scaffolded but inert until the Vale engine lands. Files move without their bytes being touched, so runtime rule signatures survive the migration.

Also anchors the sgconfig.yml gitignore pattern to .taskless/. The pattern was unanchored, so it would match a same-named file anywhere in the tree.

This PR carries the OpenSpec change docs for the whole effort, including the spec deltas the later PRs implement.

This is the bottom of a merge-down stack — the units are only correct together, since this migration relocates rules that nothing yet knows how to find. Nothing here reaches main on its own: the PRs above merge down into this branch, and the accumulated result is what merges once. The change directory stays unarchived here by design; the archive gate skips non-tip PRs and the tip archives it.

Refs OSS-24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds schema migration 0004 to relocate .taskless/ into an engine-partitioned directory layout (sg/, runtime/, scaffolded vale/), bumps the scaffold schema to v4, and introduces explicit version-mismatch gating (with an override flag). This is the stack root PR that carries the OpenSpec change docs for the broader multi-PR effort.

Changes:

  • Add migration 0004 to mechanically move legacy rule trees under .taskless/sg/ and .taskless/runtime/, scaffold .taskless/vale/, and ensure empty engine dirs are tracked via .gitkeep.
  • Update migration runner to register schema v4 and throw a SCAFFOLD_VERSION_MISMATCH error when the on-disk schema is newer than the CLI (unless --allow-version-mismatches is set).
  • Update/extend tests for schema v4 + new layout, and add the OpenSpec change (proposal/design/specs/tasks).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/cli/test/onboard.test.ts Updates expected scaffold schema version to 4.
packages/cli/test/migrate-install.test.ts Updates expected schema version to 4 and expands the version matrix.
packages/cli/test/migrate-engine-layout.test.ts New test suite validating 0004’s byte-preserving moves, gitignore anchoring, scaffolding, idempotency, and version gating override.
packages/cli/test/init-no-interactive.test.ts Updates expected scaffold schema version to 4.
packages/cli/test/bootstrap.test.ts Updates directory assertions to match sg/ engine layout.
packages/cli/src/types/errors.ts Adds SCAFFOLD_VERSION_MISMATCH error code.
packages/cli/src/index.ts Adds global --allow-version-mismatches flag definition.
packages/cli/src/filesystem/migrations/0004-vale-engine.ts New migration implementing the engine-partitioned .taskless/ layout and gitignore anchoring.
packages/cli/src/filesystem/migrations/0001-init.ts Switches gitignore entry to anchored /sgconfig.yml and documents the rationale.
packages/cli/src/filesystem/migrate.ts Registers migration v4 and enforces “scaffold newer than CLI” gating with override support.
packages/cli/src/filesystem/directory.ts Threads allowVersionMismatches option through to runMigrations.
openspec/changes/partition-rules-by-engine/tasks.md New task breakdown for the multi-PR change.
openspec/changes/partition-rules-by-engine/specs/cli-runtime-rule-execution/spec.md New/updated spec describing runtime rule location/semantics in the partitioned layout.
openspec/changes/partition-rules-by-engine/specs/cli-rule-format/spec.md New spec for engine-partitioned on-disk rule format and migration requirements.
openspec/changes/partition-rules-by-engine/specs/cli-check/spec.md New spec for check reading committed sg/sgconfig.yml and engine-based dispatch.
openspec/changes/partition-rules-by-engine/proposal.md New proposal capturing motivation, delivery shape (merge-down stack), and impact.
openspec/changes/partition-rules-by-engine/design.md New design doc detailing layout decisions, risks, and migration plan.
openspec/changes/partition-rules-by-engine/.openspec.yaml New OpenSpec metadata for the change directory.

Comment thread packages/cli/src/filesystem/migrations/0004-vale-engine.ts Outdated
thecodedrift added a commit that referenced this pull request Aug 2, 2026
`movePreservingContent` documented a bail-out for a file at the *destination*
but stat'd the source, so a directory source with a file destination recursed
and then failed part-way through the migration. Found in review by Copilot on
#79.

The guard now requires both sides to be directories, and the migration checks
every engine directory up front and refuses with SCAFFOLD_CONFLICT naming the
path. Bailing out of a single move was not enough on its own — scaffolding
still ran `mkdir` over the same file and died with a bare EEXIST, after earlier
moves had already landed. Validating first keeps `.taskless/` in a state the
user can still reason about.
@thecodedrift

Copy link
Copy Markdown
Member Author

Re: @copilot-pull-request-reviewer — "## Pull request overview — Adds schema migration 0004 to relocate .taskless/ into an engine-partitioned directory layout…"

The one finding in this review was real and is fixed in 959e636 (replied on the thread). The pre-flight conflict check it led to also covers the scaffolding step, which failed the same way for the same reason.

One note for anyone reading CI here: Validate is expected to be red on this PR, and it is not a regression. This is the bottom of a merge-down stack — group 1 relocates the rule trees, and nothing knows how to find them until #80 lands engine dispatch. The 20 failures in check.test.ts are exactly that, and they clear as the stack accumulates: the tip is at 422 passing. Nothing reaches main until this branch has absorbed #80, #81, and #73 and Validate passes on the accumulated result.

— AI Coding Agent

@thecodedrift thecodedrift added skip-changeset PR intentionally ships no release note (bypasses the changeset requirement) and removed skip-changeset PR intentionally ships no release note (bypasses the changeset requirement) labels Aug 2, 2026
@thecodedrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @thecodedrift's task in 5m 30s —— View job


Review complete

  • Gather context (PR diff, comments, prior review)
  • Review migration 0004 implementation (0004-vale-engine.ts)
  • Review migrate.ts version gating changes
  • Review other touched CLI files (check.ts, scan.ts, sgconfig.ts, etc.)
  • Review test coverage
  • Post inline comments for concrete issues
  • Post final summary

Overall assessment

This is a well-scoped migration with good invariants: moves are content-preserving (verified by hash comparisons in tests), idempotent, and the version-mismatch gate has clear semantics with test coverage for both the throw and override paths. The sgconfig.yml gitignore anchoring fix is correct and the anchorSgConfigIgnore collapse-duplicate logic is sound.

One concrete issue, posted inline:

  • assertNoDirectoryConflicts (packages/cli/src/filesystem/migrations/0004-vale-engine.ts:182-201) only validates the leaf paths in SCAFFOLD_DIRECTORIES (sg/rules, vale/rules, etc.), not the engine-root segments (sg, vale, runtime) themselves. pathExists swallows ENOTDIR as "doesn't exist," so a plain file at .taskless/runtime (or .taskless/vale/.taskless/sg) slips past the pre-flight guard entirely. The migration then partially succeeds — sg/ moves complete — before failing later with a raw, unhandled ENOTDIR from mkdir instead of the intended SCAFFOLD_CONFLICT error. This is the same failure class the 959e636 fix (referenced in the review thread above) closed for the leaf case, just one level up the tree. Given the explicit goal ("a conflict cannot leave .taskless/ half-migrated"), this seems worth closing before this becomes the tip of the stack.

Other notes (not blocking):

  • packages/cli/src/help/check.txt and help/ci.txt still document the legacy .taskless/rules/ and .taskless/runtime-rules/ paths. I confirmed this is intentionally deferred — openspec/changes/partition-rules-by-engine/tasks.md task 5.3 explicitly tracks updating help/onboarding text as unchecked, later-phase work — so no action needed here.
  • RUNTIME_RULE_TESTS_DIRECTORY in the new filesystem/layout.ts is exported but currently unused anywhere (runtime rule-tests are intentionally never enumerated per discover.ts's comment). Harmless, just noting it in case it's meant to be wired up by a later PR in the stack.

Per this repo's Validate-red note on the PR: I did not attempt to run tests/lint/build myself, and did not evaluate CI status — that's handled by CI on the PR directly, and the 20 failing check.test.ts cases are explained as expected by the merge-down stack structure.

Branch: openspec/partition-engine-1-migration

Comment thread packages/cli/src/filesystem/migrations/0004-vale-engine.ts
thecodedrift added a commit that referenced this pull request Aug 3, 2026
`movePreservingContent` documented a bail-out for a file at the *destination*
but stat'd the source, so a directory source with a file destination recursed
and then failed part-way through the migration. Found in review by Copilot on
#79.

The guard now requires both sides to be directories, and the migration checks
every engine directory up front and refuses with SCAFFOLD_CONFLICT naming the
path. Bailing out of a single move was not enough on its own — scaffolding
still ran `mkdir` over the same file and died with a bare EEXIST, after earlier
moves had already landed. Validating first keeps `.taskless/` in a state the
user can still reason about.
@thecodedrift
thecodedrift force-pushed the openspec/partition-engine-1-migration branch from c6b03d0 to c3c1c13 Compare August 3, 2026 18:32
thecodedrift added a commit that referenced this pull request Aug 3, 2026
The guard only checked the leaf scaffold paths, so a file at an engine root
(`.taskless/runtime`) slipped past it: `stat` throws ENOTDIR rather than ENOENT
when an ancestor is a file, and `pathExists` treated any error as "nothing
there." The migration then ran and died with a bare EEXIST part-way through —
exactly the failure the pre-flight exists to prevent.

It now checks every ancestor as well as the leaves, shallow first, so the root
is named and the unreachable paths below it are not reported twice. Found in
review by Claude on #79; the added test reproduces the original EEXIST.
@thecodedrift

Copy link
Copy Markdown
Member Author

Re: @claude[bot] — "Claude finished @thecodedrift's task — Review complete … Overall assessment: This is a well-scoped migration with good invariants … One concrete issue"
#79

The concrete issue was real and is fixed in this branch (replied on the thread). assertNoDirectoryConflicts scanned only the leaf scaffold paths, so a file at an engine root — .taskless/runtime — escaped it, because stat throws ENOTDIR rather than ENOENT when an ancestor is a file and pathExists treated every error as "nothing there." The migration then ran and failed part-way, which is precisely what that pre-flight exists to prevent. It now walks every ancestor, shallow first, and a test reproduces the original failure.

— AI Coding Agent

@thecodedrift

Copy link
Copy Markdown
Member Author

Re: @copilot-pull-request-reviewer — "## Pull request overview — Adds schema migration 0004 to relocate .taskless/ into an engine-partitioned directory layout…"

No actionable findings in this pass beyond the one already addressed. Since your earlier review of this PR, the branch also picked up the fix for the reader paths (migration 0004 relocated the trees while every reader kept the old ones) and the check ordering bug where rules were discovered before the migration that creates them. The suite is green on this branch alone at 397.

— AI Coding Agent

thecodedrift and others added 6 commits August 4, 2026 08:05
Split out of add-vale-rule-engine, which spanned 44 tasks across ten
groups — more than one PR should carry, and the archive gate requires
a change to land complete in a single PR.

This half is the part that carries the risk: the relayout, migration
0004, directory dispatch, ingest defaulting, legacy-path tolerance,
and running ast-grep over the committed config. It adds no engine and
changes nothing a user observes, so a difference in check output after
it lands has exactly one candidate explanation.

Vale itself and the engine-selection topic stay in add-vale-rule-engine,
which needs this layout to exist first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
Move `.taskless/rules/`, `rule-tests/`, and `sgconfig.yml` under `sg/`, and
the runtime tier to `runtime/rules/` and `runtime/rule-tests/`. Every move is
content-preserving: runtime capture bytes determine their server-side
reconciliation hashes, and `sgconfig.yml`'s `ruleDirs: [rules]` is relative to
the config file, so nothing needs rewriting.

Scaffold an inert `vale/` engine directory with its native `.vale.ini`, and
`.gitkeep` every otherwise-empty scaffolded directory.

`runMigrations` now throws when `taskless.json`'s version exceeds the highest
migration this CLI knows, instructing the user to upgrade, unless the global
`--allow-version-mismatches` flag is passed — previously it returned silently
and let an older CLI half-read a newer scaffold.

This is task group 1 of `partition-rules-by-engine`. The readers (`check`,
`verify`, runtime discovery) are updated in groups 2-4; until then 20 tests in
check/verify/runtime-check fail because the rules moved out from under them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Group 1 alone leaves 20 tests failing, so no unit of this change is
independently shippable: the stack merges down and reaches main as one commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An unanchored gitignore pattern matches at any depth, so the `sgconfig.yml`
line migration 0001 wrote also matches `.taskless/sg/sgconfig.yml` — the
committed config this layout makes the source of truth would never be tracked.

0001 now writes `/sgconfig.yml`, and 0004 rewrites the unanchored line in
existing checkouts, collapsing it with the anchored form when both are present.
Belongs to task group 1; kept as its own commit since group 1 was already
reviewed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`movePreservingContent` documented a bail-out for a file at the *destination*
but stat'd the source, so a directory source with a file destination recursed
and then failed part-way through the migration. Found in review by Copilot on
#79.

The guard now requires both sides to be directories, and the migration checks
every engine directory up front and refuses with SCAFFOLD_CONFLICT naming the
path. Bailing out of a single move was not enough on its own — scaffolding
still ran `mkdir` over the same file and died with a bare EEXIST, after earlier
moves had already landed. Validating first keeps `.taskless/` in a state the
user can still reason about.
Lives at the base of the stack: Require Changeset runs only on PRs based on
`main` and reads the PR's own diff, so on a merge-down stack the bottom PR is
where it has to be. Every branch above inherits it, and it describes the whole
change since that is what reaches `main` in one merge.
Migration `0004` relocated the trees but left every reader pointing at the flat
pre-migration paths, so this PR on its own found no rules: an empty scan reports
success, so the symptom was "no findings" rather than an error, across 20 tests.

Repoints the scan config, rule read/write/delete, `verify`, the `detect` probe,
and runtime discovery at `sg/` and `runtime/`, via one small `layout` module so
the paths are stated once.

`check` also has to migrate BEFORE it discovers. It read the rules directory
first and only migrated later inside `generateSgConfig`, so a project still on
the flat layout saw an empty `sg/rules/`, printed "No rules configured", and
returned before the migration that would have populated it ever ran. This was
invisible before `0004` because the path did not move. Only an existing
`.taskless/` is migrated, so `check` in a project without one still says so
rather than scaffolding one as a side effect.

The suite is green on this branch alone: 396 passing.
The guard only checked the leaf scaffold paths, so a file at an engine root
(`.taskless/runtime`) slipped past it: `stat` throws ENOTDIR rather than ENOENT
when an ancestor is a file, and `pathExists` treated any error as "nothing
there." The migration then ran and died with a bare EEXIST part-way through —
exactly the failure the pre-flight exists to prevent.

It now checks every ancestor as well as the leaves, shallow first, so the root
is named and the unreachable paths below it are not reported twice. Found in
review by Claude on #79; the added test reproduces the original EEXIST.
@thecodedrift
thecodedrift force-pushed the openspec/partition-engine-1-migration branch from 1e17924 to b4bcabe Compare August 4, 2026 15:05
@thecodedrift
thecodedrift merged commit 3d754b3 into main Aug 4, 2026
4 checks passed
thecodedrift added a commit that referenced this pull request Aug 4, 2026
`movePreservingContent` documented a bail-out for a file at the *destination*
but stat'd the source, so a directory source with a file destination recursed
and then failed part-way through the migration. Found in review by Copilot on
#79.

The guard now requires both sides to be directories, and the migration checks
every engine directory up front and refuses with SCAFFOLD_CONFLICT naming the
path. Bailing out of a single move was not enough on its own — scaffolding
still ran `mkdir` over the same file and died with a bare EEXIST, after earlier
moves had already landed. Validating first keeps `.taskless/` in a state the
user can still reason about.
@thecodedrift
thecodedrift deleted the openspec/partition-engine-1-migration branch August 4, 2026 15:07
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.

2 participants