feat(cli): discover runtime rules under the runtime engine directory by thecodedrift · Pull Request #81 · taskless/skills

thecodedrift · 2026-08-02T17:35:44Z

Stack (root → tip):

feat(cli): add migration 0004 for the engine-partitioned .taskless layout #79
- feat(cli): dispatch rules by engine directory #80
  - ➡️ feat(cli): discover runtime rules under the runtime engine directory #81 (you are here)
    - feat(cli): run ast-grep against the committed sg config #73

Runtime discovery now reads .taskless/runtime/rules/, deriving the path from ENGINE_LAYOUTS.runtime rather than repeating the string. Only the search path moves — runtime-check.test.ts and runtime-harness.test.ts seed the engine layout and otherwise pass unmodified, which is the evidence that rule semantics are untouched.

This is what closes the loop on directory-based dispatch: a rule under runtime/rules/ can now actually reach the harness, so both engines are asserted end to end. A new test files a runtime-shaped capture under sg/rules/ and confirms it stays static — the directory decides, not the rule's shape.

Runtime deliberately gets no legacy tolerance. The pre-0004 fallback covers .taskless/rules/ only. Since 0004 moves the runtime tree and check migrates before discovering, a leftover runtime-rules/ is never a second source. A test asserts this, so the asymmetry with ast-grep is visible rather than accidental.

Part of a merge-down stack (see the base branch).

Refs OSS-24

Built on top of #79

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

Built on top of #80

The directory a rule sits in under .taskless/ is its engine. Dispatch reads the path and never parses a rule file to decide who owns it, so adding an engine is a directory, not a discriminator.

planEngineDispatch resolves which engine directories are present; an unrecognized directory is ignored rather than guessed at, so a .taskless/ written by a newer CLI degrades to running the engines this one understands instead of handing a rule to the wrong parser.

Two compatibility behaviors are deliberate and worth reviewing as such:

Pre-0004 .taskless/rules/ still runs as ast-grep. An unmigrated checkout, or a producer that keeps naming the old path, executes rather than being silently ignored. Both sources are scanned and findings de-duplicated on the match itself, since a rule id can legitimately exist in only one of them.
A delivered rule that names no engine is ast-grep. The delivery API carries no engine discriminator — rules[].content is documented as an ast-grep definition — so absence means ast-grep, permanently, not for a migration window. An unrecognized engine is the opposite case: it means the payload is newer than the CLI, so it throws and writes nothing rather than filing the rule where the wrong parser will read it.

Part of a merge-down stack (see the base branch). Not independently shippable: rules have moved but runtime discovery still reads the old path until the next PR.

Refs OSS-24

On a stack, one changeset describes the whole change and lives on the bottom PR — the one targeting `main`, which is where the check has to pass. Branches above inherit that file, so it is never *added* in a child's own diff and every mid-stack PR failed. Labelling them `skip-changeset` records a deliberate "ships no release note," which is false, so the workflow bypasses on the base ref instead. The `on: branches: [main]` filter does not hold for stacked PRs in practice — the workflow still runs on a PR based on a feature branch — so the base is re-checked inside the step rather than trusted. Observed on #80, #81, and #73, all of which ran and failed the check with a non-main base.

…laim Review pushed back on three statements, and two of them were wrong. `propagate_stack.py` does return you to the branch you started on — normal completion, rebase conflict, balloon guard, and push failure all check it out before returning. Only a failure to check out a *child* (exit 6, typically that branch being held by a worktree) returns early without it. That is the path that actually bit, so the warning now names it instead of describing the script's normal behavior. The `branches: [main]` claim is reframed rather than dropped. It is documented to filter on the base branch, and the guard should be unreachable — but #73, #80, and #81 each produced a failing `Require a changeset` check run with an `openspec/partition-engine-*` base, and a workflow that never triggers produces no check run at all. The comment now cites that evidence and presents the check as a defensive guard, without teaching that Actions filters are unreliable in general. Also replaces the "structural failures are acceptable" guidance with the opposite rule: a mid-stack PR that is red is incomplete, and the fix belongs in it. That advice let this stack sit on 20 failures that turned out to be two real defects.

…l target `branches: [main]` reads as "only PRs whose base is main," and filtered that way when a PR had one base. Under GitHub's stacked-PR support a stacked PR targets `main` eventually, so the filter matches the eventual target and these workflows run on mid-stack PRs too — which is why #73, #80, and #81 each produced a failing `Require a changeset` run with an `openspec/partition-engine-*` base. That makes the base-ref guard required rather than defensive, and generalizes: a workflow whose correctness depends on "is this the PR that merges to main" has to establish that itself. Corrects the OpenSpec archive-check section too, which told readers a stacked PR would not run that check at all.

On a stack, one changeset describes the whole change and lives on the bottom PR — the one targeting `main`, which is where the check has to pass. Branches above inherit that file, so it is never *added* in a child's own diff and every mid-stack PR failed. Labelling them `skip-changeset` records a deliberate "ships no release note," which is false, so the workflow bypasses on the base ref instead. The `on: branches: [main]` filter does not hold for stacked PRs in practice — the workflow still runs on a PR based on a feature branch — so the base is re-checked inside the step rather than trusted. Observed on #80, #81, and #73, all of which ran and failed the check with a non-main base.

…laim Review pushed back on three statements, and two of them were wrong. `propagate_stack.py` does return you to the branch you started on — normal completion, rebase conflict, balloon guard, and push failure all check it out before returning. Only a failure to check out a *child* (exit 6, typically that branch being held by a worktree) returns early without it. That is the path that actually bit, so the warning now names it instead of describing the script's normal behavior. The `branches: [main]` claim is reframed rather than dropped. It is documented to filter on the base branch, and the guard should be unreachable — but #73, #80, and #81 each produced a failing `Require a changeset` check run with an `openspec/partition-engine-*` base, and a workflow that never triggers produces no check run at all. The comment now cites that evidence and presents the check as a defensive guard, without teaching that Actions filters are unreliable in general. Also replaces the "structural failures are acceptable" guidance with the opposite rule: a mid-stack PR that is red is incomplete, and the fix belongs in it. That advice let this stack sit on 20 failures that turned out to be two real defects.

…l target `branches: [main]` reads as "only PRs whose base is main," and filtered that way when a PR had one base. Under GitHub's stacked-PR support a stacked PR targets `main` eventually, so the filter matches the eventual target and these workflows run on mid-stack PRs too — which is why #73, #80, and #81 each produced a failing `Require a changeset` run with an `openspec/partition-engine-*` base. That makes the base-ref guard required rather than defensive, and generalizes: a workflow whose correctness depends on "is this the PR that merges to main" has to establish that itself. Corrects the OpenSpec archive-check section too, which told readers a stacked PR would not run that check at all.

Copilot

Pull request overview

Updates runtime rule discovery to follow the engine-partitioned .taskless/ layout by deriving the runtime rules directory from ENGINE_LAYOUTS.runtime, aligning the CLI implementation and tests with migration 0004 without changing runtime rule semantics.

Changes:

Switch runtime rule discovery to read .taskless/runtime/rules/ via ENGINE_LAYOUTS.runtime.rulesDirectory (and remove the now-redundant filesystem/layout.ts constants).
Update/extend CLI tests to assert runtime vs static dispatch based on directory placement and to validate the pre-migration runtime path is not treated as a secondary source.
Refresh inline/documentation references to the new runtime rules path.

Reviewed changes

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

Show a summary per file

File	Description
packages/cli/src/rules/runtime/discover.ts	Derives runtime rules directory from `ENGINE_LAYOUTS.runtime` and updates discovery docs/comments accordingly.
packages/cli/src/filesystem/layout.ts	Removes old layout constants now superseded by `ENGINE_LAYOUTS`.
packages/cli/src/rules/runtime/run-set.ts	Updates comments to reflect the new live runtime rules location.
packages/cli/src/types/runtime-rule.ts	Updates format documentation to the new runtime rules directory.
packages/cli/test/runtime-check.test.ts	Updates runtime check path fixtures to `.taskless/runtime/rules/...`.
packages/cli/test/engine-dispatch.test.ts	Adds assertions that directory placement determines runtime vs static discovery, and rejects legacy runtime path.
openspec/changes/partition-rules-by-engine/tasks.md	Marks group completion and updates narrative around the group test-failure counts.

thecodedrift · 2026-08-03T21:40:25Z

@claude /review

claude · 2026-08-03T21:40:36Z

Claude finished @thecodedrift's task in 2m 31s —— View job

Review: discover runtime rules under the runtime engine directory

Gather context (diff, changed files)
Review discover.ts changes
Review run-set.ts / runtime-rule.ts doc updates
Review test changes (runtime-check.test.ts, engine-dispatch.test.ts)
Review layout.ts deletion
Post final assessment

Overall assessment: looks good, no blocking issues

Correctness

RUNTIME_RULES_DIR = ENGINE_LAYOUTS.runtime.rulesDirectory (packages/cli/src/rules/runtime/discover.ts:15) correctly resolves to "runtime/rules", matching ENGINE_LAYOUTS.runtime in packages/cli/src/rules/engines.ts and the MOVES table in filesystem/migrations/0004-vale-engine.ts:75 (["runtime-rules"] → ["runtime", "rules"]). The docstring's claim that 0004 moves the tree "without touching a byte" checks out — movePreservingContent is a rename/copy, never a rewrite.
Verified no dangling references to the deleted packages/cli/src/filesystem/layout.ts constants anywhere in src/ or test/ — the deletion is clean.
discoverRuntimeRulesIn (discover.ts:114) is unchanged in behavior — it just gets called with the new root — so the "signatures/semantics untouched" claim in the PR description holds.
Deliberately no legacy fallback for runtime (unlike sg, which keeps .taskless/rules/ as a secondary ast-grep source via discoverAstGrepRuleSources). The new test at engine-dispatch.test.ts:255-262 correctly asserts a leftover .taskless/runtime-rules/ is never picked up post-migration, which matches the intentional asymmetry called out in the PR body and in engines.ts's LEGACY_RULES_DIRECTORY (which has no runtime counterpart).

Test adequacy

engine-dispatch.test.ts:222-262 adds three good boundary tests: real path discovery, cross-engine non-leakage (a runtime-shaped capture filed under sg/rules/ is inert for runtime discovery, and still picked up correctly by discoverAstGrepRuleSources), and legacy-path non-tolerance.
runtime-check.test.ts fixture path updates (.taskless/runtime-rules/... → .taskless/runtime/rules/...) are mechanical and match the new layout; runtime-harness.test.ts needing no changes is consistent with it already operating against an explicit root rather than the module constant.

Clarity

Doc comments in discover.ts, run-set.ts, and runtime-rule.ts are consistently updated to the new path. One straggler, already flagged by the Copilot reviewer: the tasks.md blockquote ("...and group 3 to zero") elides a verb and reads awkwardly — minor, non-blocking.
RUNTIME_RUN_DIR = ".run/runtime-rules" in run-set.ts:10 still uses the old runtime-rules naming, but that's a separate, gitignored materialization directory (not part of the engine-partitioned static layout ENGINE_LAYOUTS describes), so leaving it as-is is reasonable and out of scope here.

Not checked: build/lint/test execution and CI status — those are reported on the PR itself.

Branch: openspec/partition-engine-3-runtime → openspec/partition-engine-2-dispatch

thecodedrift · 2026-08-06T04:56:59Z

Re: @claude[bot] — "Claude finished @thecodedrift's task — Review: discover runtime rules under the runtime engine directory … Overall assessment: looks good, no blocking issues"
#81

Thanks — no action needed from this one, and the specific check you did (that RUNTIME_RULES_DIR resolves to runtime/rules and matches both ENGINE_LAYOUTS.runtime and the MOVES table in 0004) is exactly the invariant that matters here, since a mismatch would relocate rules to a path discovery never looks at and report "no findings" rather than an error.

One thing did change on this branch since your review: the stale note in tasks.md that claimed group 1 alone leaves 20 tests failing. That has been corrected — it described a state that no longer exists, and the failures it described turned out to be two real defects rather than a consequence of the split.

— AI Coding Agent

thecodedrift · 2026-08-06T04:57:07Z

Re: @copilot-pull-request-reviewer — "## Pull request overview — Updates runtime rule discovery to follow the engine-partitioned .taskless/ layout…"

The one inline finding is addressed and resolved. It was flagged as grammar, but the sentence was stale rather than awkward — it documented group 1 leaving 20 tests red, which stopped being true once that group was made self-consistent. Replaced with an accurate note.

— AI Coding Agent

Runtime discovery reads `.taskless/runtime/rules/<name>/`, deriving the path from `ENGINE_LAYOUTS.runtime` so the engine layout stays the single definition rather than a string repeated per module. Migration 0004 moved the tree here byte-for-byte, so signatures and reconciliation are unaffected — only the search path changed. Nothing reads `runtime/rule-tests/` yet; 0004 moves it and the fixtures wait for a consumer. This closes out task 2.3: a rule under `runtime/rules/` now reaches the harness, so directory-based dispatch is asserted for both engines. A runtime-shaped capture filed under `sg/rules/` is covered too — it stays static, because the directory decides. pnpm test is green: 416 passed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The constants existed so migration 0004's PR could repoint every reader without the engine model. `engines.ts` states the same paths as part of the layout each engine owns, and the last reader (runtime discovery) now reads them from there, so a second definition would only be able to disagree.

…eality The note said group 1 alone leaves 20 tests failing until later groups land. That stopped being true when group 1 was made self-consistent: it now repoints every reader as well as moving the trees, and is green on its own at 396. Copilot flagged the sentence for grammar; the substance was the bigger problem, since the note documented an expected-red state that no longer exists and that turned out to be two real defects rather than a consequence of the split.

This PR became the bottom of the stack when #80 merged, so the release note has to be visible in its own diff. One change gets one changeset: this extends the existing note rather than adding a second entry for something that ships once.

#73 became the bottom of the stack when #81 merged, so the release note has to be visible in its own diff. Extends the existing note rather than adding a second entry for a change that ships once.

This was referenced Aug 2, 2026

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

Merged

feat(cli): dispatch rules by engine directory #80

Merged

feat(cli): run ast-grep against the committed sg config #73

Merged

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch from f8f600b to 8b73474 Compare August 2, 2026 18:31

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

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch from 8b73474 to cfabc37 Compare August 2, 2026 21:36

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

thecodedrift mentioned this pull request Aug 2, 2026

fix(ci): bypass the changeset requirement on mid-stack PRs #83

Merged

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch from cfabc37 to 1f2a539 Compare August 3, 2026 06:28

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch from 1f2a539 to 4b16774 Compare August 3, 2026 18:58

thecodedrift marked this pull request as ready for review August 3, 2026 19:20

Copilot AI review requested due to automatic review settings August 3, 2026 19:20

Copilot started reviewing on behalf of thecodedrift August 3, 2026 19:21 View session

Copilot AI reviewed Aug 3, 2026

View reviewed changes

Comment thread openspec/changes/partition-rules-by-engine/tasks.md Outdated

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch from 4b16774 to a57073b Compare August 3, 2026 21:06

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch from a57073b to 5e68a4d Compare August 4, 2026 15:05

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch 3 times, most recently from cc682c6 to 94e71b5 Compare August 6, 2026 02:23

Base automatically changed from openspec/partition-engine-2-dispatch to main August 6, 2026 02:34

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch 2 times, most recently from 489d6f9 to b21b706 Compare August 6, 2026 04:45

Copilot started work on behalf of thecodedrift August 6, 2026 04:57 View session

Copilot finished work on behalf of thecodedrift August 6, 2026 04:57

thecodedrift and others added 4 commits August 5, 2026 22:14

thecodedrift force-pushed the openspec/partition-engine-3-runtime branch from b21b706 to 5546c4c Compare August 6, 2026 05:21

thecodedrift merged commit cbf81f8 into main Aug 6, 2026
4 checks passed

thecodedrift deleted the openspec/partition-engine-3-runtime branch August 6, 2026 05:57

Navigation Menu

Uh oh!

feat(cli): discover runtime rules under the runtime engine directory - #81

feat(cli): discover runtime rules under the runtime engine directory#81
thecodedrift merged 4 commits into
mainfrom
openspec/partition-engine-3-runtime

thecodedrift commented Aug 2, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

thecodedrift commented Aug 3, 2026

Uh oh!

claude Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

thecodedrift commented Aug 6, 2026

Uh oh!

thecodedrift commented Aug 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

thecodedrift commented Aug 2, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built on top of #79

Built on top of #80

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

thecodedrift commented Aug 3, 2026

Uh oh!

claude Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review: discover runtime rules under the runtime engine directory

Overall assessment: looks good, no blocking issues

Uh oh!

thecodedrift commented Aug 6, 2026

Uh oh!

thecodedrift commented Aug 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thecodedrift commented Aug 2, 2026 •

edited by github-actions Bot

Loading

claude Bot commented Aug 3, 2026 •

edited

Loading