Skip to content

Commit 174b52e

Browse files
committed
test(ast-grep): make the ast-grep 0.51.0 upgrade land green
The pin bump alone left two contract pins red. ast-grep 0.50.0 added Zig, so AST_GREP_LANGUAGES gains it. 0.51.0 replaced serde_yaml with serde-saphyr, which drops the backticks from the severity error; the pin records the new wording. No Taskless user reads that text, since validate rejects the severity against the schema first. Two measured changes no test caught are now pinned. A rule file ending in a stray `---` loads on 0.51.0, where 0.45.3 exited 8 and took every sg rule in the config down with it. And tree-sitter-haskell 0.24.1 renamed the kind `type_synomym` to `type_synonym`, the only kind or field that changed across the two grammars' node-types.json; a rule on the old spelling now exits 8. That rename is fixed deterministically. `update --rules` now applies rule migrations keyed on the engine versions the rules were last reconciled against (`rules.engines`), not on the scaffold version, so a rewrite that touches a handful of projects never bumps every project's scaffold and breaks pinned CLIs in CI. Migrations run before the stamp, are idempotent, edit bytes in place, and must only rewrite shapes the service never issued; the service has issued no rule on the old Haskell kind. `update --json` lists pending migrations, and `update --rules` refuses with SCAFFOLD_MIGRATION_REQUIRED when the layout is behind. The ledger gains a 0.13.3 entry and the update topic moves to v18.
1 parent b510c3d commit 174b52e

10 files changed

Lines changed: 664 additions & 7 deletions

File tree

‎.changeset/ast-grep-0-51-0.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,11 @@
33
---
44

55
Update the bundled ast-grep to 0.51.0 (from 0.45.3).
6+
7+
What a rule author sees:
8+
9+
- **Haskell rules on `kind: type_synomym` are renamed to `kind: type_synonym`.** ast-grep's Haskell grammar fixed the typo in that kind name, and a rule still using the old spelling fails to load. Because `taskless check` loads every ast-grep rule together, `check` would report the failure and no ast-grep findings at all. `taskless update --rules` now applies the rename itself when it records a reconciliation, naming each file it rewrote. No other Haskell kind or field changed.
10+
- `taskless update --rules` applies mechanical rule rewrites for the engine versions a project's rules cross, keyed on the engine versions it last recorded rather than on the `.taskless/` layout version, so they never force a scaffold bump. `taskless update --json` lists them in advance. It now refuses with `SCAFFOLD_MIGRATION_REQUIRED` when the layout is behind; run `taskless init` first.
11+
- `sg` rules can target `Zig`.
12+
- A rule file ending in a stray `---` no longer stops `taskless check` from reporting any ast-grep findings for the project. ast-grep used to read the empty document after it as a broken rule and abort the whole scan. `taskless validate` still flags the file as multi-document YAML.
13+
- The `.taskless/` layout does not change, so a pinned CLI keeps reading the project.

‎packages/cli/src/agent/update.md‎

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Topic: update (CLI v%(CLI_VERSION)s / topic v17)
1+
# Topic: update (CLI v%(CLI_VERSION)s / topic v18)
22

33
%(ONE_COMMAND_PER_CALL)s
44

@@ -8,8 +8,10 @@ ALREADY IN THIS PROJECT, and what to do about them.
88

99
It is not about installing. Running `%(TASKLESS_CLI)s` migrates the
1010
`.taskless/` layout and refreshes the installed skills on its own. That
11-
handles the DIRECTORY. This recipe handles the RULES, which no migration
12-
can rewrite for you: a rewriter that now needs a `fix`, a rule whose
11+
handles the DIRECTORY. This recipe handles the RULES. A purely
12+
mechanical rewrite, such as a kind an engine renamed, is applied by
13+
`update --rules` when you record the walk. The rest needs judgment no
14+
migration can supply: a rewriter that now needs a `fix`, a rule whose
1315
matching semantics shifted under a new engine, a rule that could now be
1416
expressed better with a newly supported language.
1517

@@ -103,6 +105,16 @@ that you finished.
103105
This also records the engine versions the rules are now valid
104106
against, which is what lets a later upgrade say what moved.
105107

108+
Before it records them, it applies every mechanical rewrite between
109+
the engine versions the rules were last reconciled against and the
110+
bundled ones, and names each file it changed (`migrated` with
111+
`--json`). `ruleMigrations` in `update --json` lists them in advance.
112+
A section below that says `update --rules` does something needs
113+
nothing from you beyond this step. Read the rewritten files in the
114+
diff, run `%(TASKLESS_CLI)s validate`, and commit them with the
115+
marker. If the project's `.taskless/` layout is behind, this refuses
116+
with `SCAFFOLD_MIGRATION_REQUIRED`; run `%(TASKLESS_CLI)s init` first.
117+
106118
## The ledger
107119

108120
### Migrating to 0.11.x
@@ -483,13 +495,45 @@ until now and starts reporting.
483495
by `check`, and `validate` rejected it before this release while Vale
484496
loaded it without complaint.
485497

498+
### Migrating to 0.13.3
499+
500+
The vendored ast-grep moves from 0.45.3 to 0.51.0. Nothing here needs
501+
work by hand.
502+
503+
**A Haskell type synonym is `type_synonym` now, not `type_synomym`, and
504+
`update --rules` renames it for you.** ast-grep's Haskell grammar moved
505+
from tree-sitter-haskell 0.23.1 to 0.24.1, which fixed the typo in that
506+
kind's name. Diffed across both grammars, it is the only kind or field
507+
that changed. A rule still naming the old kind is a HARD CONFIG ERROR:
508+
exit 8, ``Kind `type_synomym` is invalid``, and since `check` loads
509+
every sg rule into one config it reports no sg findings at all until
510+
the rule is fixed.
511+
512+
Do not rename it by hand. Step 5 rewrites every `kind: type_synomym` in
513+
a rule whose `language` is Haskell, leaves everything else in the file
514+
as it was, and names each file it changed. No rule Taskless issued uses
515+
the old kind, so the rewrite never touches a signed rule.
516+
517+
**A rule file ending in a stray `---` no longer breaks `check`.** On
518+
0.45.3 ast-grep read the empty document after it as a second rule with
519+
no `language`, exited 8, and `check` reported no sg findings at all.
520+
It now loads. If `check` was failing on that, findings come back and
521+
are real. `validate` still rejects the file as multi-document YAML, so
522+
delete the trailing `---` when you next touch the rule.
523+
524+
**`Zig` is a language.** `language: Zig` parses, so a rule over `.zig`
525+
files that was routed to the runtime tier, or not written because
526+
ast-grep could not parse Zig, can now be an sg rule. No existing rule
527+
changes.
528+
486529
## Errors
487530

488531
With `--json`, `--rules` failures emit `{ ok: false, code, message }`:
489532

490533
| code | meaning | fix |
491534
|-----------------|------------------------------------------------|--------------------------------------------|
492535
| `INVALID_INPUT` | no `.taskless/`, or an older CLI would rewind the marker | set the project up, or upgrade first |
536+
| `SCAFFOLD_MIGRATION_REQUIRED` | the `.taskless/` layout is behind this CLI, so rule rewrites have nowhere safe to write | run `%(TASKLESS_CLI)s init`, then `update --rules` again |
493537

494538
## See Also
495539

‎packages/cli/src/commands/init.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ import {
5353
stampNewProjectRules,
5454
} from "../rules/reconcile-marker";
5555
import { readManifest } from "../filesystem/manifest";
56-
import type { MigrationReport } from "../filesystem/migrate";
56+
import {
57+
requireCurrentSchema,
58+
type MigrationReport,
59+
} from "../filesystem/migrate";
60+
import { pendingRuleMigrations } from "../rules/rule-migrate";
5761
import { TASKLESS_DIRECTORY } from "../rules/vale/formats";
5862
import { CLIError } from "../util/cli-error";
5963
import { buildInvocation } from "../util/invocation";
@@ -291,6 +295,19 @@ export const updateCommand = defineCommand({
291295
// never recorded a reconciliation, which is not the same as being
292296
// behind, or it is already current.
293297
walk: walk ?? null,
298+
// The rewrites `update --rules` will apply before it stamps, so
299+
// the walk can tell its reader what it does not need to do by
300+
// hand. Empty without a project, which has nothing to rewrite.
301+
ruleMigrations:
302+
manifest === undefined
303+
? []
304+
: pendingRuleMigrations(manifest.rules?.engines).map(
305+
({ engine, engineVersion, summary }) => ({
306+
engine,
307+
engineVersion,
308+
summary,
309+
})
310+
),
294311
recipe,
295312
})
296313
);
@@ -303,6 +320,10 @@ export const updateCommand = defineCommand({
303320

304321
const telemetry = await getTelemetry(cwd);
305322
try {
323+
// Rule migrations rewrite files where the CURRENT layout puts them, so
324+
// a scaffold that is behind has to be migrated first. Checked here and
325+
// not in `recordReconciliation`, because `migrate` imports the marker.
326+
await requireCurrentSchema(cwd);
306327
const result = await recordReconciliation(cwd);
307328
if (args.json) {
308329
console.log(JSON.stringify({ ok: true, ...result }));
@@ -312,6 +333,14 @@ export const updateCommand = defineCommand({
312333
? `Rules reconciled to ${result.reconciledTo} (ast-grep ${result.engines.sg}, Vale ${result.engines.vale}).`
313334
: `Rules reconciled to ${result.reconciledTo}, was ${result.previous} (ast-grep ${result.engines.sg}, Vale ${result.engines.vale}).`
314335
);
336+
for (const migration of result.migrated) {
337+
console.log(
338+
`${migration.summary} (${migration.engine} ${migration.engineVersion}): ` +
339+
(migration.files.length === 0
340+
? "no rule needed it."
341+
: `rewrote ${migration.files.join(", ")}.`)
342+
);
343+
}
315344
}
316345
telemetry.capture("cli_rules_reconciled");
317346
} catch (error) {

‎packages/cli/src/rules/capabilities.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ export const AST_GREP_LANGUAGES = [
9191
"Tsx",
9292
"TypeScript",
9393
"Yaml",
94+
"Zig",
9495
] as const;
9596

9697
/** One of the spellings {@link AST_GREP_LANGUAGES} lists, canonically cased. */

‎packages/cli/src/rules/reconcile-marker.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { access } from "node:fs/promises";
22
import { join } from "node:path";
33

44
import { AST_GREP_VERSION, VALE_VERSION } from "./capabilities";
5+
import { applyRuleMigrations, type AppliedRuleMigration } from "./rule-migrate";
56
import { readManifest, writeManifest } from "../filesystem/manifest";
67
import { TASKLESS_DIRECTORY } from "./vale/formats";
78
import { CLIError } from "../util/cli-error";
@@ -27,6 +28,8 @@ export interface ReconcileResult {
2728
engines: { sg: string; vale: string };
2829
/** The value replaced, or `undefined` on a project that had never recorded one. */
2930
previous: string | undefined;
31+
/** Rule rewrites applied for the engine versions this stamp crosses. */
32+
migrated: AppliedRuleMigration[];
3033
}
3134

3235
/**
@@ -98,6 +101,15 @@ export async function recordReconciliation(
98101
);
99102
}
100103

104+
// Before the stamp, never after: the stamp records these engines, and a
105+
// marker naming an engine whose rewrites did not run would excuse the next
106+
// walk from them. A failure part-way leaves the marker where it was, and
107+
// every migration is idempotent, so the rerun finishes the job.
108+
const migrated = await applyRuleMigrations(
109+
tasklessDirectory,
110+
manifest.rules?.engines
111+
);
112+
101113
const rules = {
102114
...manifest.rules,
103115
reconciledTo,
@@ -108,7 +120,7 @@ export async function recordReconciliation(
108120

109121
await writeManifest(tasklessDirectory, { ...manifest, rules }, raw);
110122

111-
return { reconciledTo, engines: rules.engines, previous };
123+
return { reconciledTo, engines: rules.engines, previous, migrated };
112124
}
113125

114126
/**
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { join } from "node:path";
2+
3+
import { AST_GREP_VERSION, VALE_VERSION } from "./capabilities";
4+
import haskellTypeSynonym from "./rule-migrations/sg-0-50-0-haskell-type-synonym";
5+
import { compareVersions } from "../util/version-compare";
6+
7+
/**
8+
* Mechanical rewrites of rule CONTENT, applied when a project's rules cross an
9+
* engine version.
10+
*
11+
* Not scaffold migrations, and deliberately keyed on a different version.
12+
* The scaffold `version` records the LAYOUT, and every bump of it makes a
13+
* pinned older CLI refuse the project with `SCAFFOLD_VERSION_MISMATCH`. An
14+
* engine upgrade that renames a kind or retires a key touches the few projects
15+
* with a rule in that shape, so spending a scaffold bump on it would break
16+
* every pinned CI for a change almost none of them need. These are keyed on
17+
* `rules.engines` instead: the engine versions the rules were last reconciled
18+
* against, recorded by `update --rules` and nowhere else.
19+
*
20+
* `update --rules` applies them, after the ledger walk and before it stamps
21+
* the marker, so the marker never records an engine whose rewrites did not
22+
* run. Each migration runs when the recorded engine version is below its
23+
* `engineVersion` and the bundled engine is at or above it.
24+
*
25+
* A migration MUST:
26+
*
27+
* - be idempotent, and leave a file it has nothing to do in untouched, so a
28+
* rerun after a partial failure (nothing is stamped then) changes nothing
29+
* already done and `git status` stays clean on a project it does not apply
30+
* to;
31+
* - edit bytes in place rather than parse and re-serialize, so every comment
32+
* and blank line stays where the author put it;
33+
* - rewrite only shapes the Taskless service has not issued, or has already
34+
* migrated in its own corpus. An issued rule is checked against the bytes
35+
* the service recorded, and accepting any other bytes client-side would be a
36+
* signature bypass. A rewritten issued rule reads as "edited since Taskless
37+
* issued it", and `rule restore` then writes the old shape straight back.
38+
*/
39+
export type RuleEngine = "sg" | "vale";
40+
41+
export interface RuleMigration {
42+
engine: RuleEngine;
43+
/** The first engine version on which the rewritten shape is required. */
44+
engineVersion: string;
45+
/** One line saying what changes, as `update --rules` reports it. */
46+
summary: string;
47+
/**
48+
* Rewrite the rules under `engineDirectory` (`.taskless/rules/<engine>`),
49+
* returning each file written, relative to that directory.
50+
*/
51+
apply: (engineDirectory: string) => Promise<string[]>;
52+
}
53+
54+
/** What one applied migration reports: its identity and the files it wrote. */
55+
export interface AppliedRuleMigration {
56+
engine: RuleEngine;
57+
engineVersion: string;
58+
summary: string;
59+
/** Written files, relative to the project root. */
60+
files: string[];
61+
}
62+
63+
const RULE_MIGRATIONS: readonly RuleMigration[] = [haskellTypeSynonym];
64+
65+
const BUNDLED: Record<RuleEngine, string> = {
66+
sg: AST_GREP_VERSION,
67+
vale: VALE_VERSION,
68+
};
69+
70+
/**
71+
* A rules block that never recorded an engine predates the marker, so it has
72+
* had none of these applied. The same reading `BASELINE_VERSION` gives an
73+
* absent `reconciledTo`; restated rather than imported, because
74+
* `reconcile-marker` imports this module.
75+
*/
76+
const UNRECORDED = "0.0.0";
77+
78+
/** The migrations a project with `recorded` engines still needs, in order. */
79+
export function pendingRuleMigrations(
80+
recorded: Partial<Record<RuleEngine, string>> | undefined,
81+
migrations: readonly RuleMigration[] = RULE_MIGRATIONS
82+
): RuleMigration[] {
83+
return migrations
84+
.filter(
85+
(migration) =>
86+
compareVersions(
87+
recorded?.[migration.engine] ?? UNRECORDED,
88+
migration.engineVersion
89+
) < 0 &&
90+
compareVersions(migration.engineVersion, BUNDLED[migration.engine]) <= 0
91+
)
92+
.toSorted(
93+
(a, b) =>
94+
a.engine.localeCompare(b.engine) ||
95+
compareVersions(a.engineVersion, b.engineVersion)
96+
);
97+
}
98+
99+
/**
100+
* Apply every pending migration under `tasklessDirectory`, in order.
101+
*
102+
* Every migration is reported, including one that wrote nothing, so the
103+
* output says what was checked and not only what changed.
104+
*/
105+
export async function applyRuleMigrations(
106+
tasklessDirectory: string,
107+
recorded: Partial<Record<RuleEngine, string>> | undefined,
108+
migrations: readonly RuleMigration[] = RULE_MIGRATIONS
109+
): Promise<AppliedRuleMigration[]> {
110+
const applied: AppliedRuleMigration[] = [];
111+
for (const migration of pendingRuleMigrations(recorded, migrations)) {
112+
const engineDirectory = join(tasklessDirectory, "rules", migration.engine);
113+
const written = await migration.apply(engineDirectory);
114+
applied.push({
115+
engine: migration.engine,
116+
engineVersion: migration.engineVersion,
117+
summary: migration.summary,
118+
files: written.map((file) =>
119+
join(".taskless", "rules", migration.engine, file)
120+
),
121+
});
122+
}
123+
return applied;
124+
}

0 commit comments

Comments
 (0)