Skip to content

fix(cli): stop telling a new project its rules are out of date - #613

Merged
theCodeDrift merged 1 commit into
mainfrom
fix/fresh-install-owes-no-rules
Nov 5, 2026
Merged

theCodeDrift merged 1 commit into
mainfrom
fix/fresh-install-owes-no-rules

Conversation

@theCodeDrift

Copy link
Copy Markdown
Member

A wizard install on an empty repo never recorded rules.reconciledTo; only init did. No marker reads as "this project predates the ledger", so the first agent ok during onboarding reported entries "from 0.0.0 to 0.13.5" and asked for update --rules on a project with no rules. The wizard gap is old. agent ok started surfacing it with the environment check added on 2026-11-03.

check and verify never refused, since no engines were recorded. The cost was an agent stalling on the first step of onboarding over a false alarm.

Both install paths now call stampRulelessProject, which stamps any project with no rules in sg, vale or runtime. This replaces the wasNewProject check, which had to be sampled before mkdir and could not help projects the wizard had already installed. Those clear the next time init runs, provided they still have no rules.

  • A project with rules and no marker still owes its walk.
  • A marker written by a newer CLI is never rewound.
  • init now moves the marker forward on a project with no rules. Previously it never overwrote one.

Verified by driving the real wizard in a pty on a fresh repo (reconciledTo: 0.13.5, agent ok up to date), and by running init on a project the old wizard left unstamped. Typecheck, lint and the full suite pass.

Only init recorded rules.reconciledTo, so a wizard install left no marker. That reads as a project predating the ledger, and agent ok asked for update --rules on a repo with no rules. Both install paths now stamp any project with no rules, which also clears it for wizard installs already in the wild the next time init runs.
@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 1m 47s —— View job


Review mode: incremental — read 0 prior review thread(s) before reviewing.

No prior review threads, reviews, or top-level comments exist on this PR (the only comment is the /review trigger itself), so this is the first review — every finding below is [New].

  • Read prior review data (none found)
  • Fetch PR diff and metadata
  • Read changed files in full context (reconcile-marker.ts, init.ts, wizard/index.ts, layout.ts, engines.ts, both test files)
  • Review for correctness, security, performance, tests, clarity
  • Post findings

Summary

The fix is well-targeted: stampRulelessProject replaces the old wasNewProject sampling (which had to run before mkdir and could never help a project the wizard had already set up) with a check based on the actual rule tree, gated behind the existing reconciliationStart logic so it can't rewind a marker a newer CLI wrote or silently skip a real walk.

Verified against the code:

  • stampRulelessProject is called in both init.ts (packages/cli/src/commands/init.ts:479) and wizard/index.ts (packages/cli/src/wizard/index.ts:144), both right after ensureTasklessDirectory and before anything that could write skills/rules — applyInstallPlan never touches .taskless/rules/, so the ruleless check can't race against the install it's part of.
  • reconciliationStart(manifest.rules?.reconciledTo) === undefined correctly early-returns when the marker is already current or ahead of the installed CLI (covers "never rewind a newer marker"), and falls through to the listRuleIds scan otherwise, so the 3 readdir calls only happen when a walk would otherwise be owed — no added cost on the common "already reconciled" path.
  • listRuleIds/subdirectories swallow a missing rules/<engine> directory as [] (packages/cli/src/rules/engines.ts:119-128), so a project with no rules/ directory at all behaves the same as one with empty per-engine directories.
  • Test coverage is solid: it exercises "stamped when never recorded," "moves forward with the CLI," "keeps a marker a newer CLI wrote," and "leaves a project alone once any engine holds a rule" (packages/cli/test/reconcile-marker.test.ts:368-411), plus wizard-level checks that a fresh install is stamped reconciled while one with pre-existing rules still owes its walk (packages/cli/test/wizard-integration.test.ts:173-206).

Two minor points, neither blocking:

  1. [New] Broadened match in the read path vs. the write path's stricter validation — reconciliationStart (packages/cli/src/rules/reconcile-marker.ts:138-151) treats an unparseable reconciledTo (e.g. a stray string) the same as a missing one, since compareVersions coerces an unparseable segment to 0. recordReconciliation's write path explicitly guards against exactly this class of value before writing it (see the "Checked BEFORE either comparison" comment in packages/cli/src/rules/engines.ts:46-53), but stampRulelessProject now reuses reconciliationStart as its gate, so a garbage reconciledTo on a rule-less project would get silently overwritten with the current version. This is pre-existing behavior of reconciliationStart (unchanged by this PR) rather than something newly introduced, and the blast radius is narrow — it only fires on a project with zero rules in every engine — but worth flagging since this PR is the first caller to drive that function from unsanitized manifest data in a context where the old code used a stricter !== undefined check.

  2. [New] Broad try/catch still swallows real failures silently — stampRulelessProject's outer try/catch (packages/cli/src/rules/reconcile-marker.ts:181-206) discards any error from readManifest, listRuleIds, or writeManifest with no logging. This is carried over from the previous stampNewProjectRules implementation (same shape pre-PR) and the doc comment's rationale — "setup must not fail because a marker could not be written" — is reasonable for a best-effort stamp, so not a regression, but a malformed manifest or permissions error here now fails just as silently as before, with no way for a user to notice the project never got stamped.

No security, correctness, or test-adequacy issues found beyond the above. The changeset text and PR description accurately describe the behavior implemented.

@theCodeDrift
theCodeDrift merged commit 2614cf0 into main Nov 5, 2026
8 checks passed
@theCodeDrift
theCodeDrift deleted the fix/fresh-install-owes-no-rules branch November 5, 2026 00:51
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.

1 participant