Planned Taskless Rules - #168
Conversation
Seven rules mined from CLAUDE.md, the .conventions styleguides, and the posthog conventions — the places this repo already states its rules in prose. Each one is verified and tested locally; `check` reports two real findings, both READMEs telling external consumers to run `pnpm dlx`. sg: no-index-imports relative imports ending in /index no-pii-in-telemetry PII in capture/identify/groupIdentify prefer-do-rpc `as DurableObjectStub<T>` casts no-regex-over-build-output a test function that reads dist/ and regexes it no-pull-request-branches-filter a branches: filter under pull_request: vale: docs-npx-cli pnpm invocations in READMEs no-hedging simply / obviously / of course Also backfills tests for the pre-existing no-eval rule, which was failing `verify` for a missing test file, and puts every rule's `language:` on ast-grep's own spelling. The install churn — the rules/ partition, taskless.json, the embedded SKILL.md — was already in the tree and is carried along rather than authored here.
|
|
A sweep of merged-PR review threads and the OSS Linear backlog turned up OSS-49, which names two planned GitHub Actions rules and records that a prior onboarding pass stalled on whether ast-grep handles YAML. It does, so both are buildable locally rather than runtime-tier. Adds pr-workflow-ready-for-review, and renames the branches-filter rule to the pr-workflow-* name OSS-49 already uses for the pair. One finding on the real workflows: stack-breadcrumb.yml names types: [opened, reopened, edited, closed] with no ready_for_review. That is a question rather than a defect — it reacts to PR metadata, not to readiness — and is left for review to settle.
OSS-28 specifies the house writing style as Vale rules. This adds the two that can be expressed cleanly, scoped to READMEs rather than the whole repository: broadening is a separate decision with roughly 2200 findings attached. no-em-dashes needs `nonword: true`, which the snippet in OSS-28 does not carry. Measured: without it the rule parses, passes verify, and does not fire, because Vale wraps every token in word boundaries and an em dash is non-word on both sides. no-blocklist-phrases drops the `landed on` token OSS-28 proposed. Measured firing on "the plane landed on time", which is the literal sense the issue says to protect; `we landed` already covers "we landed on the second option", so the broader token earned nothing. Not included: the "not X, Y" antithesis rule. OSS-28 is undecided on how to express it and calls it a heuristic, so it wants a human judgement this pass cannot make. 24 findings in READMEs, all em dashes, left unfixed pending a call on whether to rewrite the prose or relax the level.
Brings the nine READMEs in line with no-em-dashes, which this branch scopes to `**/README.md`. Twenty-four instances, rewritten as a period, comma, colon, or parentheses per the rule's own message rather than mechanically swapped for one substitute.
docs-npx-cli says docs use `npx @taskless/cli`, the normalized invocation for external consumers. Two README blocks offered `pnpm dlx` alongside it, which is the same claim the rule exists to prevent, so the alternative is removed rather than annotated. Annotating was measured and is not available here: a rule scoped to `raw` reads the unparsed document, so Vale's `<!-- vale Rule = NO -->` directive is inert for it. That scope is what lets the rule see fenced blocks at all, and every command in a README lives in one.
The most repeated finding in the merged-PR review threads: a comment that cites a version or a claim the code moved out from under it. On #161 alone, five threads — "we pin 3.17.1" left behind after VALE_VERSION went to 3.18.0, a comment naming .rmd as plaintext after the diff moved it to markup, a block still framing 3.18.0 as "the known incoming bump" after it shipped. Same shape on #155 and #157. The rule flags the construction rather than the stale number, which is the half a static check can see. Vale reads .ts in its comments-only tier, so it lints the comment text and never the code. Zero findings on packages/cli/src today, because #161 fixed them all. Reach verified separately by planting a forecast comment in a real source file: fixtures run under an isolating config, so a passing test does not prove the matcher glob reaches anything. Also ignores .taskless/ in eslint and prettier. A rule about source comments needs .ts fixtures, Vale decides its comments-only tier by extension, and those files belong to no tsconfig — so lint-staged failed the commit on "not found by the project service". Rule fixtures are inputs written to be flagged; verify and test are what keep them honest.
pr-workflow-ready-for-review flagged this as the only finding on the repo, and it is a real gap rather than the intended exception. `ready_for_review` is not in the default event set, so a draft marked ready fired nothing here: the breadcrumb went on describing the PR as a draft until some later event happened to run. The existing comment says tree SHAPE only, which is still true — `synchronize` stays out because a head push never changes membership. A draft becoming ready does change the stack, so it is named explicitly and the comment now says why. `taskless check` is clean at 12 rules.
prefer-do-rpc targets `$EXPR as DurableObjectStub<$T>`, a Cloudflare Durable Objects pattern. The Taskless services need it; this repository has no Cloudflare code for it to reach, so the rule and its synthetic fixtures are removed rather than carried pre-emptively. Nothing else in the tree referenced it. docs-npx-cli stays as written, and the root README now records that `package.json` carries a `cli` script pointing at `./packages/cli/dist/index.js` for running the local build. The note names the script instead of reproducing its shell invocation, because docs-npx-cli has no per-case escape hatch: its `raw` scope is what reaches a fenced code block, and Vale evaluates a raw scope against the unparsed markup, so in-file directives are ignored. Measured on Vale 3.18.0, both the rule-specific comment and a blanket `vale off` fail to suppress it, and dropping `raw` makes them work at the cost of every fenced-block finding. The rule keeps its full reach; the finding is recorded in its .vale.ini.
All three settled in 09cb566. 1. 2. 3. On the self-flagging problem, which is real and turned out to be a property of the rule's scope: The rule fires on the literal Rather than weaken it, the note names the script instead of reproducing its shell invocation. That keeps the rule at full reach with no exception, no per-file matcher, and no broadened scope. I deliberately did not reach for Checks, all local: — AI Coding Agent |
09cb566 to
e855198
Compare
First slice of the broadening in #169. The three prose rules were scoped to READMEs only, because landing them repo-wide meant roughly 2300 findings at level: error. This adds the two smallest surfaces and fixes what they catch, so the slice is green on its own. 41 findings: 39 em dashes rewritten as a period, comma, colon, or parentheses per the rule's own message rather than swapped mechanically for one substitute, and two uses of "simply" where the sentence was describing a real distinction ("merely lives further down", "just out of date") rather than hedging. Fenced code blocks are untouched. These rules carry Vale's default scope, which does not read them, so the em dashes in the shell comments at CLAUDE.md:155 and STYLEGUIDE-CODE.md:222 are out of scope and stay. Remaining surfaces, in the order #169 proposes: TypeScript comments (Vale's comments-only tier), then the agent-facing recipe text, which is the largest. openspec/changes/archive/ gets a permanent exclusion rather than a slice. Refs #169
Stack (root → tip):
Rules for our own repository, mined from the places we already state our conventions in prose:
CLAUDE.md, the two.conventionsstyleguides, and the PostHog notes. Nothing here is settled — the point of landing it is to have somewhere concrete to argue about what we actually want enforced.docs-npx-clipnpm dlx @taskless/cli/pnpm cliin any READMEno-em-dashesno-blocklist-phrasesload-bearing)no-hedgingno-index-imports/indexno-pii-in-telemetryemail/displayName/etc. incapture(),identify(),groupIdentify()no-regex-over-build-outputdist/and then regexes itpr-workflow-no-branches-filterbranches:filter under apull_request:triggerpr-workflow-ready-for-reviewpull_request:types:list that omitsready_for_reviewThe two
docs-npx-clifindings —README.md:37andpackages/cli/README.md:17— are left unfixed on purpose, so the PR demonstrates a rule catching something rather than only asserting it would.The two Vale house-style rules come from the dogfooding issue (#104 / OSS-28), scoped to
**/README.md. Widening them is #169, because repo-wide they are roughly 2300 findings. Two corrections to that issue's spec, both measured and both in the rule files:no-em-dashesneedsnonword: trueor it silently never fires, and the proposedlanded ontoken fires on "the plane landed on time" and was dropped.The 24 em-dash findings in READMEs are unfixed. Rewriting them is an editorial pass on user-facing prose, so it wants a decision rather than a default: rewrite the prose, or drop the rule to
warninguntil someone does.The two
pr-workflow-*rules come from OSS-49, which names both and records that an earlier onboarding pass stalled on whether ast-grep handles YAML. It does —Yamlis one of its 26 languages — so both build locally instead of needing a login.pr-workflow-ready-for-reviewreports one finding, and it is a question rather than a defect:stack-breadcrumb.ymlnamestypes: [opened, reopened, edited, closed]with noready_for_review. It reacts to PR metadata rather than to readiness, so omitting it may well be correct. Settling that is part of what this PR is for.Where the rules are narrower than the conventions
Three are worth arguing about specifically, because each is a deliberate retreat from what the styleguide actually says:
no-index-importsis not "no barrel imports." Tellingfrom "../wizard"(a directory barrel) fromfrom "./ask"(a file) needs filesystem resolution, which is a cross-file question and therefore runtime-tier. The rule catches only the explicit/indexform, which meanspackages/cli/src/commands/init.ts:16is a real violation that nothing in this PR flags.no-regex-over-build-outputscopes to the enclosing function, not the file. Scoped to the file it fired three times onpackages/cli/test/prompts.test.ts, where a helper deliberately regexes hand-written source and the same file separately loads a built artifact. That helper is correct and the styleguide says so.prefer-do-rpcwas proposed here and has since been removed. It had no instances in this repository, and the CLI has no Cloudflare code for it to reach, so it belongs with the Taskless services rather than being carried pre-emptively. The rule and its synthetic fixtures are gone as of the review pass.Two conventions could not be expressed at all while logged out, both for the same reason — they need the cross-file graph: "a type reachable from an exported signature must itself be exported" (the
declaration: truetrap) and "a hand-written type must not duplicate a library type."Where to start
The two Vale
.vale.inifiles. Scoping is the decision most likely to be wrong and the least visible in a diff: both rules are restricted to READMEs, both exclude**/test/fixtures/**(they were firing on CLI test inputs), anddocs-npx-clideliberately does not coverCLAUDE.md, which documents the localpnpm clipath on purpose.The root README now records that
package.jsoncarries acliscript for running the local build. It names the script rather than reproducing the shell line, because therawscope below is what reaches a fenced code block and Vale evaluates a raw scope against the unparsed markup, so the rule has no in-file escape hatch: measured on Vale 3.18.0, both<!-- vale docs-npx-cli.docs-npx-cli = NO -->and a blanket<!-- vale off -->are ignored, and droppingrawmakes them work at the cost of every fenced-block finding.docs-npx-clialso carriesscope: [raw, code, text]. Without it the rule passes every fixture and catches nothing real, because every command in a README is in a code span or a fenced block.Carried along
The
rules/engine partition,taskless.json, the embeddedSKILL.md, and theno-evalmove were already in the working tree from the install. Also backfilled: tests forno-eval, which had been failingverifyfor a missing test file, and every rule'slanguage:is now on ast-grep's own spelling rather than lowercase.Fixes #104
Refs #165
Refs #166
Refs #167
Contains #176
First slice of the broadening in #169, stacked on #168 because the rules it widens do not exist anywhere else yet.
The three prose rules currently match
**/README.mdonly. That was not a judgement about where the house style applies — it was that landing them repo-wide meant roughly 2300 findings atlevel: error, which is a remediation decision rather than a technical one. This slice takes the two smallest surfaces and fixes what they catch, so it is green on its own.What it catches, and what changed
41 findings across
CLAUDE.mdand.conventions/STYLEGUIDE-CODE.md:Nothing else in either file changed.
Scope decisions a reviewer should check
Fenced code blocks are untouched, deliberately. These rules carry Vale's default scope, which does not read them, so the em dashes in the shell comments at
CLAUDE.md:155andSTYLEGUIDE-CODE.md:222are genuinely out of scope. Widening torawto catch them would also mean the rules could no longer be suppressed per-case (see #167), and code comments in a fenced example are not the prose this rule is about.no-hedginggainsCLAUDE.md, which it did not have. It already covered.conventions/*.md, so this makes the pair consistent rather than adding a surface arbitrarily.Where the stack goes next
Per #169's ordering, and one PR per surface so each carries its own remediation and stays independently green:
CLAUDE.md+.conventions/(~47 findings).tsin its comments-only tier, so this touches comment prose and never codepackages/cli/src/agent/*.txt— the largest surface, and the most product-facing, since it is agent-facing recipe textopenspec/changes/archive/— a permanent exclusion rather than a slice. Rewriting an archived proposal would edit the record of what was proposed at the time, which feat: resolve the CLI invocation as a recipe variable #143's review thread already settledNo changeset, here or anywhere in this stack. Taskless rules live in
.taskless/and are not part of the published package, so none of this ships a release note.changeset.ymlwill warn; that is expected.One thing worth knowing
Measuring this slice on a machine with agent worktrees present produced 122 findings, of which 83 came from
worktrees/. That is #166, and its fix (#172) is not on this branch. The 41 above are the real in-scope count.Refs #169