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
5 changes: 5 additions & 0 deletions .changeset/fresh-install-owes-no-rules.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@taskless/cli": patch
---

Stop telling a new project its rules are out of date. Installing from the terminal never recorded that a new project had nothing to reconcile, so the first `agent ok` during onboarding asked for `update --rules` on a project with no rules. Installing now records it, and upgrading clears the message on any project that already has it and still has no rules.
20 changes: 3 additions & 17 deletions packages/cli/src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ import {
processLauncherContext,
} from "../util/package-manager";
import {
pathExists,
recordReconciliation,
stampNewProjectRules,
stampRulelessProject,
} from "../rules/reconcile-marker";
import { readManifest } from "../filesystem/manifest";
import {
Expand Down Expand Up @@ -455,15 +454,6 @@ async function runNonInteractive(
// stderr — visible to a person watching the terminal, invisible to a
// machine consumer parsing stdout.
const log = options.json ? console.error : console.log;
// Sampled BEFORE the directory is created, and that order is the whole
// point. `ensureTasklessDirectory` mkdir -p's, so afterwards a pre-existing
// project is indistinguishable from a fresh one.
//
// This path is `init` itself, whose documented job is
// refreshing an EXISTING project. Stamping there would mark a project that
// never walked the ledger as fully reconciled and skip every entry, which is
// the silent skip this feature exists to prevent.
const wasNewProject = !(await pathExists(join(cwd, TASKLESS_DIRECTORY)));
// `init` is now the ONLY command that migrates, so it is the only one that
// can report what a migration moved. `check`, `validate` and `verify` used to
// carry this on their own envelopes and refuse rather than migrate now, so
Expand All @@ -486,11 +476,7 @@ async function runNonInteractive(
if (!options.json) console.error(message);
},
});
if (wasNewProject) {
// A project this CLI just created has no entries to walk: everything the
// ledger describes is already true of the scaffold it wrote.
await stampNewProjectRules(cwd);
}
await stampRulelessProject(cwd);

const allSkills = getEmbeddedSkills();
const mandatoryNames = new Set(getMandatorySkillNames());
Expand Down Expand Up @@ -599,7 +585,7 @@ async function runNonInteractive(
log(warning);
}

// Read after `stampNewProjectRules`, so a project this run created reports
// Read after `stampRulelessProject`, so a project with no rules reports
// nothing due.
const { manifest } = await readManifest(join(cwd, TASKLESS_DIRECTORY));
const rules = await reconcileStatus(cwd, manifest.rules);
Expand Down
41 changes: 23 additions & 18 deletions packages/cli/src/rules/reconcile-marker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { access } from "node:fs/promises";
import { join } from "node:path";

import { AST_GREP_VERSION, VALE_VERSION } from "./capabilities";
import { listRuleIds } from "./engines";
import { ENGINES } from "./layout";
import { applyRuleMigrations, type AppliedRuleMigration } from "./rule-migrate";
import { readManifest, writeManifest } from "../filesystem/manifest";
import { TASKLESS_DIRECTORY } from "./vale/formats";
Expand Down Expand Up @@ -138,8 +140,8 @@ export function reconciliationStart(
): { from: string; to: string } | undefined {
const installed = getCliVersion();
// A missing marker means the project PREDATES the marker, so every entry
// still applies and the walk starts from the beginning. `init` stamps the
// field on a genuinely new project, which is what makes the two
// still applies and the walk starts from the beginning. Installing stamps
// the field on a project with no rules, which is what makes the two
// distinguishable: absent is old, present is accounted for. Treating absent
// as "nothing to do" would have quietly excused every project that existed
// before this feature from the entries written for it.
Expand All @@ -159,29 +161,32 @@ export async function pathExists(path: string): Promise<boolean> {
}

/**
* Stamp the rules marker on a project being set up by this CLI.
* Stamp the rules marker on a project that has no rules.
*
* A new project has no ledger entries to walk: everything the entries describe
* is already true of a scaffold this CLI just wrote. Recording that up front
* is what gives an ABSENT marker its meaning, which is "this project predates
* the ledger and has had none of it applied".
* The ledger describes what to do about rules already in a project. With none
* there is nothing to apply, so owing a walk would be a false claim, and
* `agent ok` and `check` would repeat it to a person who just installed. That
* holds whether the project is new, set up before this ran on the wizard's
* path, or has moved to a newer CLI since.
*
* ONLY call this when the project was genuinely new, which the caller has to
* establish BEFORE creating the directory. This function cannot tell: by the
* time it runs, `ensureTasklessDirectory` has already `mkdir -p`'d, so a
* pre-existing project and a fresh one look identical. Stamping one that
* merely never recorded a marker would mark it fully reconciled and skip every
* entry, which is the exact silent-skip this whole feature exists to prevent,
* reachable by re-running setup on an ordinary older project.
* Both install paths call this after `ensureTasklessDirectory`, since asking
* about the rules, unlike asking whether `.taskless/` exists, still has its
* answer after the directory is created.
*
* Never overwrites either. Re-running setup on a project that HAS a marker
* must not reset one a real walk earned.
* It never touches a project that has rules: stamping one that merely never
* recorded a marker would skip every entry, which is the exact silent skip
* this feature exists to prevent, reachable by re-running setup on an ordinary
* older project. It also never rewinds a marker a newer CLI wrote.
*/
export async function stampNewProjectRules(cwd: string): Promise<void> {
export async function stampRulelessProject(cwd: string): Promise<void> {
const tasklessDirectory = join(cwd, TASKLESS_DIRECTORY);
try {
const { manifest, raw } = await readManifest(tasklessDirectory);
if (manifest.rules?.reconciledTo !== undefined) return;
if (reconciliationStart(manifest.rules?.reconciledTo) === undefined) return;
const rules = await Promise.all(
ENGINES.map((engine) => listRuleIds(cwd, engine))
);
if (rules.some((ids) => ids.length > 0)) return;
await writeManifest(
tasklessDirectory,
{
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/wizard/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
TELEMETRY_NOTICE,
TELEMETRY_NOTICE_TITLE,
} from "../telemetry";
import { stampRulelessProject } from "../rules/reconcile-marker";
import { CLIError } from "../util/cli-error";
import { buildInvocation } from "../util/invocation";
import { getCliPrefix } from "../util/package-manager";
Expand Down Expand Up @@ -140,6 +141,7 @@ export async function runWizard(
}
},
});
await stampRulelessProject(options.cwd);
const cliVersion = getCliVersion();
const { launcher, lockfile, retired } = await applyInstallPlan(
options.cwd,
Expand Down
65 changes: 61 additions & 4 deletions packages/cli/test/reconcile-marker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,16 @@ describe("recording a rules reconciliation", () => {
return (JSON.parse(raw) as { rules?: Record<string, unknown> }).rules;
}

async function setMarker(reconciledTo: string): Promise<void> {
const path = join(cwd, ".taskless", "taskless.json");
const manifest = JSON.parse(await readFile(path, "utf8")) as Record<
string,
unknown
>;
manifest["rules"] = { reconciledTo };
await writeFile(path, JSON.stringify(manifest, undefined, 2));
}

async function installedVersion(): Promise<string> {
const result = await runCli(["info", "--json", "-d", cwd]);
return (JSON.parse(result.stdout) as { version: string }).version;
Expand Down Expand Up @@ -325,10 +335,8 @@ describe("recording a rules reconciliation", () => {

it("does not stamp a pre-existing project that never reconciled", async () => {
// The silent skip this whole feature exists to prevent, reachable through
// setup rather than through the walk. `init` is the
// documented refresh path for an EXISTING project, and it runs after
// `ensureTasklessDirectory` has already created the directory, so
// "was this new" has to be sampled before that or it always reads new.
// setup rather than through the walk. `init` is the documented refresh
// path for an EXISTING project, and this fixture has rules.
//
// Stamping here would mark a project that has walked nothing as fully
// reconciled and skip every ledger entry.
Expand All @@ -349,6 +357,55 @@ describe("recording a rules reconciliation", () => {
expect(walk?.from).toBe("0.0.0");
});

describe("a project with no rules", () => {
beforeEach(async () => {
await rm(join(cwd, ".taskless", "rules"), {
recursive: true,
force: true,
});
});

it("is stamped by init when it never recorded a marker", async () => {
// The ledger is about rules already here, so there is nothing to owe.
// The wizard left projects in this state, and `agent ok` then told a
// person who had just installed that their rules were out of date.
await runCli(["init", "-d", cwd]);

expect(await readRules()).toEqual({
reconciledTo: await installedVersion(),
engines: { sg: AST_GREP_VERSION, vale: VALE_VERSION },
});
});

it("moves forward with the CLI", async () => {
await setMarker("0.11.0");

await runCli(["init", "-d", cwd]);

const rules = await readRules();
expect(rules?.["reconciledTo"]).toBe(await installedVersion());
});

it("keeps a marker a newer CLI wrote", async () => {
await setMarker("999.0.0");

await runCli(["init", "-d", cwd, "--allow-version-mismatches"]);

const rules = await readRules();
expect(rules?.["reconciledTo"]).toBe("999.0.0");
});

it("leaves a project alone once any engine holds a rule", async () => {
const rule = join(cwd, ".taskless", "rules", "runtime", "some-rule");
await mkdir(rule, { recursive: true });

await runCli(["init", "-d", cwd]);

const rules = await readRules();
expect(rules?.["reconciledTo"]).toBeUndefined();
});
});

it("reports the walk boundary on info as well, so callers agree", async () => {
// The recipe tells an agent to read `rules.walk` from `info --json`. It
// has to actually be there, or the guidance sends it looking for a field
Expand Down
34 changes: 34 additions & 0 deletions packages/cli/test/wizard-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,40 @@ describe("runWizard end-to-end", () => {
expect(captureSpy).toHaveBeenCalledWith("cli_installed");
});

it("records a new project's rules as reconciled, so nothing is owed", async () => {
clackResponses.locations = ["Claude Code"];
clackResponses.summary = true;

const { runWizard } = await import("../src/wizard");
await runWizard({ cwd, globalTrustEnvironment });

const manifest = JSON.parse(
await readFile(join(cwd, ".taskless", "taskless.json"), "utf8")
) as {
rules?: { reconciledTo?: string; engines?: unknown };
install: { cliVersion?: string };
};
expect(manifest.rules?.reconciledTo).toBeDefined();
expect(manifest.rules?.reconciledTo).toBe(manifest.install.cliVersion);
expect(manifest.rules?.engines).toBeDefined();
});

it("leaves a project with rules owing its walk", async () => {
clackResponses.locations = ["Claude Code"];
clackResponses.summary = true;
await mkdir(join(cwd, ".taskless", "rules", "sg", "some-rule"), {
recursive: true,
});

const { runWizard } = await import("../src/wizard");
await runWizard({ cwd, globalTrustEnvironment });

const manifest = JSON.parse(
await readFile(join(cwd, ".taskless", "taskless.json"), "utf8")
) as { rules?: { reconciledTo?: string } };
expect(manifest.rules?.reconciledTo).toBeUndefined();
});

it("completes without a token and never offers to log in", async () => {
vi.stubEnv("TASKLESS_TOKEN", "");
clackResponses.locations = ["Claude Code"];
Expand Down
Loading