Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/engine-partitioned-layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ Partition `.taskless/` by rule engine. Migration `0004` moves ast-grep rules to

The directory a rule sits in now **is** its engine: dispatch reads the path and never parses a rule file to decide who owns it. `check` runs ast-grep against the committed `.taskless/sg/sgconfig.yml` instead of generating an ephemeral config each run.

A rule engine the CLI does not recognize is now rejected with a message instead of failing silently: an unsupported engine from the server previously exited 0 with no output, which read as success.

Existing projects keep working without action. The pre-`0004` `.taskless/rules/` still runs as ast-grep, and a delivered rule that names no engine is still treated as ast-grep — a rule engine this CLI does not recognize is rejected rather than guessed at. A migration that would have to merge a file into an engine directory now refuses up front with `SCAFFOLD_CONFLICT` rather than failing part-way.
13 changes: 10 additions & 3 deletions .github/workflows/require-changeset.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,16 @@ jobs:
;;
esac

# A changeset is any new `.changeset/*.md` added by this PR, other than
# the template README.
ADDED=$(git diff --name-only --diff-filter=A "$BASE_SHA...$HEAD_SHA" -- '.changeset/*.md' \
# A changeset is any `.changeset/*.md` this PR adds OR modifies, other
# than the template README.
#
# Modifications count because one change gets ONE changeset: when a
# stack lands forward, the second PR onward finds the release note
# already on `main` from the PR below it, and the right move is to
# extend that file rather than add a second entry for something that
# ships once. Counting additions only made "grow the changeset as the
# stack lands" impossible to satisfy.
ADDED=$(git diff --name-only --diff-filter=AM "$BASE_SHA...$HEAD_SHA" -- '.changeset/*.md' \
| grep -viE '/README\.md$' || true)

if [ -z "$ADDED" ]; then
Expand Down
30 changes: 16 additions & 14 deletions openspec/changes/partition-rules-by-engine/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,32 @@
- [x] 1.5 Add version-mismatch gating: `runMigrations` throws when `taskless.json.version > maxVersion` with an "upgrade the CLI" message, unless a global `--allow-version-mismatches` flag is set
- [x] 1.6 Tests: `0004` moves each tree correctly, `.gitkeep` present, runtime contents byte-identical; gating throws and the flag overrides

- [x] 1.7 Anchor the `sgconfig.yml` entry in `.taskless/.gitignore` (`/sgconfig.yml`), so the unanchored pattern `0001` wrote no longer also ignores the committed `.taskless/sg/sgconfig.yml`; `0004` rewrites it in existing checkouts

> After group 1 alone, `check`/`verify`/runtime discovery still read the pre-move paths, so 20 tests in
> `check.test.ts`, `verify.test.ts`, and `runtime-check.test.ts` fail until groups 2–4 land. `.taskless/.gitignore`
> also still ignores `sgconfig.yml`, which now matches the committed `sg/sgconfig.yml` (task 5.3).
> `check.test.ts`, `verify.test.ts`, and `runtime-check.test.ts` fail until groups 2–4 land. Group 2 brings
> that to 9, all in `runtime-check.test.ts`, which group 3 fixes.

## 2. Engine dispatch (directory model)

- [ ] 2.1 Implement directory-based engine discovery: enumerate `.taskless/<engine>/` and route rules by directory, no per-file parsing. `sg` and `runtime` get executors here; `vale/` is recognized as an engine directory but has no executor yet
- [ ] 2.2 In `commands/check.ts`, call `ensureTasklessDirectory(cwd)` directly (preserving the migration trigger now that `generateSgConfig` leaves the check path)
- [ ] 2.3 Tests: a rule under `sg/rules/` dispatches to ast-grep and one under `runtime/rules/` to the harness, by directory alone; an unknown engine directory is ignored rather than misrouted
- [ ] 2.4 Treat the legacy `.taskless/rules/` path as an ast-grep source alongside `sg/rules/`, so an unmigrated checkout still runs; de-duplicate when both are present
- [ ] 2.5 Tests: a `.taskless/` with only `rules/` dispatches to ast-grep; with both `rules/` and `sg/rules/`, findings merge without duplicates
- [x] 2.1 Implement directory-based engine discovery: enumerate `.taskless/<engine>/` and route rules by directory, no per-file parsing. `sg` and `runtime` get executors here; `vale/` is recognized as an engine directory but has no executor yet — `rules/engines.ts` (`planEngineDispatch`, `discoverAstGrepRuleSources`)
- [x] 2.2 In `commands/check.ts`, call `ensureTasklessDirectory(cwd)` directly (preserving the migration trigger now that `generateSgConfig` leaves the check path) — `rules/verify.ts` does the same, since the migration moves rules between the paths it resolves
- [ ] 2.3 Tests: a rule under `sg/rules/` dispatches to ast-grep and one under `runtime/rules/` to the harness, by directory alone; an unknown engine directory is ignored rather than misrouted — **partially done**: the `sg/rules/` and unknown-directory scenarios are covered (`test/engine-dispatch.test.ts`), and the dispatch plan asserts `runtime` → harness; a rule _under `runtime/rules/`_ cannot reach the harness until 3.1 moves discovery, so that half is covered by 3.3
- [x] 2.4 Treat the legacy `.taskless/rules/` path as an ast-grep source alongside `sg/rules/`, so an unmigrated checkout still runs; de-duplicate when both are present
- [x] 2.5 Tests: a `.taskless/` with only `rules/` dispatches to ast-grep; with both `rules/` and `sg/rules/`, findings merge without duplicates

## 2b. Service-delivered rule ingest

- [ ] 2b.1 Update `rules/files.ts` — `writeRuleFile` writes `.taskless/sg/rules/<id>.yml` and `writeRuleTestFile` writes `.taskless/sg/rule-tests/`, replacing the hardcoded `.taskless/rules` / `.taskless/rule-tests` (both call sites are `commands/rules.ts:241,245,482,486`)
- [ ] 2b.2 Resolve the destination from an engine the payload identifies, defaulting to `sg` when the payload identifies none — permanently, since the API carries no engine discriminator today
- [ ] 2b.3 Fail loudly on an engine the CLI does not recognize: error naming the engine, instruct upgrade, write nothing (do NOT fall back to `sg`)
- [ ] 2b.4 Audit the remaining `.taskless/rules` string literals for the same defect — at minimum `rules/verify.ts:246`, `rules/files.ts:99`, `commands/check.ts:314`, `commands/rules.ts:661`, and the `detect/scan.ts:428` layout probe
- [ ] 2b.5 Tests: an engine-less payload lands in `sg/rules/` and is dispatched to ast-grep by `check`; a migrated rule and a freshly delivered one come to rest at the same path; an unrecognized engine errors and writes nothing
- [x] 2b.1 Update `rules/files.ts` — `writeRuleFile` writes `.taskless/sg/rules/<id>.yml` and `writeRuleTestFile` writes `.taskless/sg/rule-tests/`, replacing the hardcoded `.taskless/rules` / `.taskless/rule-tests` (both call sites are `commands/rules.ts:241,245,482,486`)
- [x] 2b.2 Resolve the destination from an engine the payload identifies, defaulting to `sg` when the payload identifies none — permanently, since the API carries no engine discriminator today
- [x] 2b.3 Fail loudly on an engine the CLI does not recognize: error naming the engine, instruct upgrade, write nothing (do NOT fall back to `sg`)
- [x] 2b.4 Audit the remaining `.taskless/rules` string literals for the same defect — at minimum `rules/verify.ts:246`, `rules/files.ts:99`, `commands/check.ts:314`, `commands/rules.ts:661`, and the `detect/scan.ts:428` layout probe. All five now resolve through `rules/engines.ts`; `help/*.txt` still names the legacy path and belongs to 5.3
- [x] 2b.5 Tests: an engine-less payload lands in `sg/rules/` and is dispatched to ast-grep by `check`; a migrated rule and a freshly delivered one come to rest at the same path; an unrecognized engine errors and writes nothing

## 2c. Reconcile compatibility

- [ ] 2c.1 Confirm reported reconcile paths follow the moved trees (`rules/runtime/run-set.ts:57` builds repo-relative POSIX paths from the discovered location)
- [ ] 2c.2 Test: after `0004`, signatures are unchanged and the signature-based join resolves every moved rule — nothing reports as new or missing
- [x] 2c.1 Confirm reported reconcile paths follow the moved trees (`rules/runtime/run-set.ts:57` builds repo-relative POSIX paths from the discovered location) — confirmed unchanged; `relative(cwd, rule.checkFile)` is derived from the discovery root, so no edit was needed
- [x] 2c.2 Test: after `0004`, signatures are unchanged and the signature-based join resolves every moved rule — nothing reports as new or missing. Asserted at the reporting layer (same signature, moved path) rather than against a live server

## 3. Runtime discovery path

Expand Down
71 changes: 46 additions & 25 deletions packages/cli/src/commands/check.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import { resolve, join, isAbsolute, relative } from "node:path";
import { readdir, stat } from "node:fs/promises";
import { stat } from "node:fs/promises";
import { defineCommand } from "citty";

import { runAstGrepScan } from "../rules/scan";
import type { CheckResult } from "../types/check";
import { formatText } from "../util/format";
import { generateSgConfig } from "../filesystem/sgconfig";
import { ensureTasklessDirectory } from "../filesystem/directory";
import {
dedupeFindings,
discoverAstGrepRuleSources,
planEngineDispatch,
} from "../rules/engines";
import { getTelemetry } from "../telemetry";
import { outputSchema as checkOutputSchema } from "../schemas/check";
import { makeErrorEnvelope } from "../types/errors";
Expand All @@ -25,8 +31,6 @@ import {
signRuntimeChecks,
} from "../rules/runtime/run-set";
import { executeRuntimeRules } from "../rules/runtime/harness";
import { SG_RULES_DIRECTORY } from "../filesystem/layout";
import { ensureTasklessDirectory } from "../filesystem/directory";

async function pathExists(absolutePath: string): Promise<boolean> {
try {
Expand Down Expand Up @@ -311,30 +315,38 @@ export const checkCommand = defineCommand({
return;
}

// Migrate before discovering anything. Rules are read from their
// engine directory, which migration `0004` is what creates — discovering
// first would find an empty `sg/rules/` on any project still on the flat
// layout, report "No rules configured", and return before the migration
// that would have populated it ever ran. Only an existing `.taskless/` is
// migrated, so `check` in a project that has none still says so instead
// of scaffolding one as a side effect.
// Rules dispatch by the engine directory that contains them. This is also
// the migration trigger: `generateSgConfig` is leaving the check path, so
// without this call an upgraded CLI would keep reading a stale layout.
//
// Only an existing `.taskless/` is migrated. `ensureTasklessDirectory`
// creates the scaffold, and `check` is a read-only command — running it in
// a project that has none should report that, not write one (and not fail
// on a read-only filesystem).
if (await pathExists(join(cwd, ".taskless"))) {
await ensureTasklessDirectory(cwd);
}
const dispatch = await planEngineDispatch(cwd);

// Static rules (trusted ast-grep YAML) always run; runtime rules
// (untrusted check.ts) are gated separately.
const rulesDirectory = join(cwd, ".taskless", SG_RULES_DIRECTORY);
let staticRuleFiles: string[] = [];
try {
const entries = await readdir(rulesDirectory);
staticRuleFiles = entries.filter((f) => f.endsWith(".yml"));
} catch {
// .taskless/ or rules/ directory doesn't exist
}
const runtimeRules = await discoverRuntimeRules(cwd);
// (untrusted check.ts) are gated separately. An engine directory this CLI
// has no executor for (vale) contributes nothing, and a directory that is
// not a known engine is ignored rather than handed to someone's parser.
const astGrepSources = await discoverAstGrepRuleSources(cwd);
// Both halves matter: `executor` alone is read from the static layout
// table and is therefore always `runtime-harness`, so gating on it only
// would make this unconditionally true and the presence check decorative.
const runtimeDispatch = dispatch.find(
(entry) => entry.engine === "runtime"
);
const runtimeEnabled =
runtimeDispatch?.present === true &&
runtimeDispatch.executor === "runtime-harness";
const runtimeRules = runtimeEnabled
? await discoverRuntimeRules(cwd)
: [];

if (staticRuleFiles.length === 0 && runtimeRules.length === 0) {
if (astGrepSources.length === 0 && runtimeRules.length === 0) {
if (args.json) {
console.log(
JSON.stringify(
Expand All @@ -352,12 +364,21 @@ export const checkCommand = defineCommand({
try {
const results: CheckResult[] = [];

// Static rules: always scan, no verification (inert data).
if (staticRuleFiles.length > 0) {
await generateSgConfig(cwd);
// Static rules: always scan, no verification (inert data). Each
// ast-grep source is scanned on its own — `sg/rules/` and, for an
// unmigrated checkout, the legacy `.taskless/rules/` — and identical
// findings from both are collapsed so a rule present in both layouts
// is reported once.
const staticResults: CheckResult[] = [];
for (const source of astGrepSources) {
await generateSgConfig(cwd, {
rulesDirectory: source.rulesDirectory,
testDirectory: source.ruleTestsDirectory,
});
const scan = await runAstGrepScan(cwd, existingPaths);
results.push(...scan.results);
staticResults.push(...scan.results);
}
results.push(...dedupeFindings(staticResults));

// Runtime rules: run only what the server validated (or forced).
const plan = await planRuntime(cwd, runtimeRules, {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const onboardCommand = defineCommand({
" --force re-runs the discovery recipe; --mark-complete records completion."
);
process.exitCode = 1;
throw new CLIError("conflicting flags");
throw new CLIError("conflicting flags", undefined, { reported: true });
}

await ensureTasklessDirectory(cwd);
Expand Down Expand Up @@ -101,7 +101,7 @@ export const onboardCommand = defineCommand({
// Should not happen — onboard.txt is embedded at build time.
console.error("Internal error: onboard recipe is not available.");
process.exitCode = 1;
throw new CLIError("recipe missing");
throw new CLIError("recipe missing", undefined, { reported: true });
}
console.log(recipe.trimEnd());
},
Expand Down
11 changes: 7 additions & 4 deletions packages/cli/src/commands/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
readRuleMetaFile,
deleteRuleFiles,
} from "../rules/files";
import { ENGINE_LAYOUTS, LEGACY_RULES_DIRECTORY } from "../rules/engines";
import {
inputSchema as createInputSchema,
outputSchema as createOutputSchema,
Expand Down Expand Up @@ -97,7 +98,7 @@ const createCommand = defineCommand({
console.error(`Error: ${message}`);
}
process.exitCode = 1;
throw new CLIError(message);
throw new CLIError(message, code, { reported: true });
}

if (args.anonymous) {
Expand Down Expand Up @@ -342,7 +343,7 @@ const improveCommand = defineCommand({
console.error(`Error: ${message}`);
}
process.exitCode = 1;
throw new CLIError(message);
throw new CLIError(message, code, { reported: true });
}

if (args.anonymous) {
Expand Down Expand Up @@ -582,7 +583,7 @@ const metaCommand = defineCommand({
console.error(`Error: ${message}`);
}
process.exitCode = 1;
throw new CLIError(message);
throw new CLIError(message, code, { reported: true });
}

const meta = await readRuleMetaFile(cwd, args.id);
Expand Down Expand Up @@ -658,7 +659,9 @@ const deleteCommand = defineCommand({
}
success = true;
} else {
const message = `Rule "${id}" not found in .taskless/rules/${id}.yml`;
const message =
`Rule "${id}" not found in .taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}/${id}.yml ` +
`or .taskless/${LEGACY_RULES_DIRECTORY}/${id}.yml`;
if (args.json) {
console.log(
JSON.stringify(makeErrorEnvelope("RULE_NOT_FOUND", message))
Expand Down
22 changes: 15 additions & 7 deletions packages/cli/src/detect/scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { readFile as readFileNode } from "node:fs/promises";
import { resolve } from "node:path";

import { parse as parseToml } from "smol-toml";
import { SG_RULES_DIRECTORY } from "../filesystem/layout";

import { ENGINE_LAYOUTS, LEGACY_RULES_DIRECTORY } from "../rules/engines";

export interface DetectedLinter {
name: string;
Expand Down Expand Up @@ -416,22 +417,29 @@ interface PythonManifest {

/**
* Surface the styles of the repo's own existing rules so the authoring recipe
* can match house conventions. `.taskless/rules` is the repo-root, polyglot
* Taskless convention, so it is read at the scan root; the custom-ESLint-rule
* tells (house rule directories and the local-rules plugin dependency) describe
* how this repo already authors lint rules.
* can match house conventions. ast-grep rules are the repo-root, polyglot
* Taskless convention, so they are read at the scan root — from the `sg` engine
* directory, or the pre-migration `.taskless/rules` when that is what the repo
* still has; the custom-ESLint-rule tells (house rule directories and the
* local-rules plugin dependency) describe how this repo already authors lint
* rules.
*/
function detectRuleStyles(
root: string,
nodeManifests: NodeManifest[]
): RuleStyle[] {
const ruleStyles: RuleStyle[] = [];
if (existsSync(resolve(root, ".taskless", SG_RULES_DIRECTORY))) {
for (const source of [
`.taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}`,
`.taskless/${LEGACY_RULES_DIRECTORY}`,
]) {
if (!existsSync(resolve(root, source))) continue;
ruleStyles.push({
source: `.taskless/${SG_RULES_DIRECTORY}`,
source,
description:
"Existing Taskless ast-grep rules — match their structure and conventions.",
});
break;
}
for (const directory of [
"eslint-rules",
Expand Down
25 changes: 19 additions & 6 deletions packages/cli/src/filesystem/sgconfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,30 @@ import { writeFile } from "node:fs/promises";
import { join } from "node:path";

import { ensureTasklessDirectory } from "./directory";
import { SG_RULES_DIRECTORY, SG_RULE_TESTS_DIRECTORY } from "./layout";
import { ENGINE_LAYOUTS } from "../rules/engines";

/** Build sgconfig contents pointing `ruleDirs` at the given directory. */
function sgConfigContent(rulesDirectory: string): string {
return `ruleDirs:\n - ${rulesDirectory}\ntestConfigs:\n - testDir: ${SG_RULE_TESTS_DIRECTORY}\n`;
function sgConfigContent(
rulesDirectory: string,
testDirectory: string
): string {
return `ruleDirs:\n - ${rulesDirectory}\ntestConfigs:\n - testDir: ${testDirectory}\n`;
}

export interface SgConfigOptions {
/**
* Directory (relative to `.taskless/`) that ast-grep should load rules from.
* Defaults to the `sg` engine directory. Reconciliation points this at the
* ephemeral run directory so only the server-blessed run set is evaluated.
* Defaults to `sg/rules`, the engine-partitioned location. Callers pass the
* legacy `rules` when scanning an unmigrated tree, and reconciliation points
* this at the ephemeral run directory so only the server-blessed run set is
* evaluated.
*/
rulesDirectory?: string;
/**
* Directory (relative to `.taskless/`) holding that rule set's tests.
* Defaults to `sg/rule-tests`. Only `sg test` reads it.
*/
testDirectory?: string;
}

/**
Expand All @@ -29,7 +39,10 @@ export async function generateSgConfig(
await ensureTasklessDirectory(cwd);
await writeFile(
join(cwd, ".taskless", "sgconfig.yml"),
sgConfigContent(options.rulesDirectory ?? SG_RULES_DIRECTORY),
sgConfigContent(
options.rulesDirectory ?? ENGINE_LAYOUTS.sg.rulesDirectory,
options.testDirectory ?? ENGINE_LAYOUTS.sg.ruleTestsDirectory
),
"utf8"
);
}
Loading
Loading