From fa7d165eb21a4a211c69374c0b6fc08b2c9cbc7a Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Tue, 19 May 2026 13:44:37 +0200 Subject: [PATCH 1/5] feat(autofixer): per-suggestion suppression via `svelte-mcp-ignore` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The custom-visitor pass (`add_autofixers_issues`, the third source after the Svelte compiler and ESLint passes) had no ignore protocol. Some visitors are deliberately heuristic — they nudge toward Svelte 5 idioms but can fire on intentional code (an `$effect` whose side effect into a third-party library cannot become a `$derived`, a `bind:this` on a host element where an attachment would be a wider refactor than worth, etc.). This commit adds a directive comment that silences a single suggestion on the following line, modelled after `svelte-ignore` and `eslint-disable-next-line`: // svelte-mcp-ignore effect_calls_function fetch_data(); Each visitor suggestion now carries a stable lowercase snake_case code (`effect_calls_function`, `effect_assigns_state`, `bind_this_attachment`, `use_action_attachment`, `derived_with_function`, `imported_runes`, `runes_instead_of_store`, `wrong_property_access_state`). Multiple codes can be listed on one directive (space-separated). Stale codes (no matching suggestion fired) and typos (unknown code) surface as follow-up "unused directive" suggestions so the comments don't quietly rot. Implementation lives in a single new module (`autofixers/ignore-directives.ts`) that collects directives by target line once per file. Visitors call `push_suggestion(state.output, state.ignore_registry, code, node.loc?.start?.line, message)` instead of `state.output.suggestions.push(message)`. `read-state-with-dollar.ts` remains un-guarded because its diagnostics are `issues` (hard errors), not `suggestions`. Scope matches `svelte-ignore`: a directive on line N silences suggestions whose triggering node starts on line N+1. This means two statements that share a target line can be silenced by one directive, but two statements on different lines need two directives — same shape the Svelte compiler warning suppression already uses. Tests: - 14 new test cases in `ignore-directives.test.ts` covering every code, the `//` / `/* */` / `` comment shapes, the line-N-only scope, multiple-codes-per-directive, the unused-directive diagnostic, and the unknown-code diagnostic. - All 251 pre-existing tests still pass (the refactor is non-breaking; `add_autofixers_issues` builds its own registry when invoked at the top level). Docs: - `documentation/docs/30-mcp/40-tools.md`: user-facing "Suppressing suggestions" section + code table. - `tools/instructions/AGENTS.md`: instruction for the LLM that drives the autofixer. - `CLAUDE.md`: developer note pointing at the new module. - Tool description on `svelte-autofixer` itself updated so any LLM introspecting the tool registration sees the directive shape. Co-authored-by: Cursor --- .changeset/svelte-mcp-ignore-directive.md | 37 +++ CLAUDE.md | 1 + documentation/docs/30-mcp/40-tools.md | 31 +++ .../mcp/autofixers/add-autofixers-issues.ts | 11 +- .../mcp/autofixers/ignore-directives.test.ts | 242 +++++++++++++++++ .../src/mcp/autofixers/ignore-directives.ts | 253 ++++++++++++++++++ .../autofixers/visitors/assign-in-effect.ts | 13 +- .../visitors/derived-with-function.ts | 7 +- .../mcp/autofixers/visitors/imported-runes.ts | 13 +- .../src/mcp/autofixers/visitors/index.ts | 10 + .../visitors/suggest-attachments.ts | 19 +- .../visitors/use-runes-instead-of-store.ts | 7 +- .../visitors/wrong-property-access-state.ts | 9 +- .../mcp/handlers/tools/svelte-autofixer.ts | 2 +- tools/instructions/AGENTS.md | 2 + 15 files changed, 646 insertions(+), 11 deletions(-) create mode 100644 .changeset/svelte-mcp-ignore-directive.md create mode 100644 packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts create mode 100644 packages/mcp-server/src/mcp/autofixers/ignore-directives.ts diff --git a/.changeset/svelte-mcp-ignore-directive.md b/.changeset/svelte-mcp-ignore-directive.md new file mode 100644 index 00000000..92b54c10 --- /dev/null +++ b/.changeset/svelte-mcp-ignore-directive.md @@ -0,0 +1,37 @@ +--- +'@sveltejs/mcp': minor +--- + +feat(autofixer): per-suggestion suppression via `svelte-mcp-ignore` directives + +The custom-visitor suggestions emitted by `add_autofixers_issues` (the third +diagnostic source, separate from the Svelte compiler and ESLint passes) can now +be silenced individually with a comment directive on the line above the +triggering node: + +```svelte + + + + +``` + +Multiple codes can be listed on one directive (space-separated). Stale codes +(no matching suggestion fired) and typos (unknown code) surface as follow-up +"unused directive" suggestions so the comments don't quietly rot. + +Available codes: + +- `effect_calls_function` +- `effect_assigns_state` +- `bind_this_attachment` +- `use_action_attachment` +- `derived_with_function` +- `imported_runes` +- `runes_instead_of_store` +- `wrong_property_access_state` diff --git a/CLAUDE.md b/CLAUDE.md index 12a2cc88..8232842e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,6 +57,7 @@ Located in `src/lib/server/analyze/`: - **Autofixers** (`src/lib/mcp/autofixers.ts`): Visitor pattern implementations for code analysis - **Walker Utility** (`src/lib/index.ts`): Enhanced AST walking with visitor mixing capabilities - **Current Autofixer**: `assign_in_effect` - detects assignments to `$state` variables inside `$effect` blocks +- **Suggestion suppression**: each custom-visitor suggestion carries a stable code (`effect_calls_function`, `bind_this_attachment`, …). Users can silence one with a `// svelte-mcp-ignore ` (script) or `` (markup) comment on the line above the triggering node. Stale or typo'd codes surface as follow-up "unused directive" suggestions. See `packages/mcp-server/src/mcp/autofixers/ignore-directives.ts`. ### Database Layer diff --git a/documentation/docs/30-mcp/40-tools.md b/documentation/docs/30-mcp/40-tools.md index 23b8b8bf..01f7ecff 100644 --- a/documentation/docs/30-mcp/40-tools.md +++ b/documentation/docs/30-mcp/40-tools.md @@ -16,6 +16,37 @@ Allows the model to get the full (and up-to-date) documentation for the requeste Uses static analysis to provide suggestions for code that your LLM generates. It can be invoked in an agentic loop by your model until all issues and suggestions are resolved. +### Suppressing suggestions + +Some custom-visitor suggestions are heuristic — they're written to nudge a model toward a Svelte 5 idiom, but they can fire on perfectly reasonable code (e.g. an `$effect` that imperatively pushes state into a third-party library, where `$derived` genuinely doesn't apply). You can silence a single suggestion on the next line with a `svelte-mcp-ignore` comment, modelled after `svelte-ignore` and `eslint-disable-next-line`: + +```svelte + + + + +``` + +The directive scopes to **the immediately following line** — the same shape `svelte-ignore` uses for compiler warnings. Multiple codes can be listed on one directive (`// svelte-mcp-ignore effect_calls_function effect_assigns_state`). Stale or typo'd codes surface as a follow-up suggestion so the comments don't quietly rot. + +Available codes: + +| Code | What it silences | +| ----------------------------- | -------------------------------------------------------------------------------- | +| `effect_calls_function` | "You are calling a function inside an `$effect`." | +| `effect_assigns_state` | "The stateful variable X is assigned inside an `$effect`." | +| `bind_this_attachment` | "`bind:this` can often be replaced with an `attachment`." | +| `use_action_attachment` | "Consider using an `attachment` instead of an `action`." | +| `derived_with_function` | "You are passing a function to `$derived` … use `$derived.by` instead." | +| `imported_runes` | "You are importing `state` / `effect` / … from `svelte`. Runes are global." | +| `runes_instead_of_store` | "You are importing `derived` / `writable` / `readable` from `svelte/store`." | +| `wrong_property_access_state` | "You are trying to update the stateful variable X using `set` / `update` / `$`." | + ## playground-link Generates an ephemeral playground link with the generated code. It's useful when the generated code is not written to a file in your project and you want to quickly test the generated solution. The code is not stored anywhere except the URL itself (which will often, as a consequence, be quite large). diff --git a/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.ts b/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.ts index 085f071b..37c80900 100644 --- a/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.ts +++ b/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.ts @@ -2,6 +2,7 @@ import { parse } from '../../parse/parse.js'; import { walk } from '../../mcp/autofixers/ast/walk.js'; import type { Node } from 'estree'; import * as autofixers from './visitors/index.js'; +import { gather_ignore_directives, report_unused_directives } from './ignore-directives.js'; export function add_autofixers_issues( content: { issues: string[]; suggestions: string[] }, @@ -11,13 +12,21 @@ export function add_autofixers_issues( async = false, ) { const parsed = parse(code, filename); + const ignore_registry = gather_ignore_directives(parsed); // Run each autofixer separately to avoid interrupting logic flow for (const autofixer of Object.values(autofixers)) { walk( parsed.ast as unknown as Node, - { output: content, parsed, desired_svelte_version, async }, + { output: content, parsed, desired_svelte_version, async, ignore_registry }, autofixer, ); } + + // Surface any `svelte-mcp-ignore` directive that listed a + // code which never matched — same shape as ESLint's + // `no-unused-svelte-ignore` rule. Diagnoses both stale + // comments left over from a refactor and typos in the + // code name itself. + report_unused_directives(content, ignore_registry); } diff --git a/packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts b/packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts new file mode 100644 index 00000000..7e49779b --- /dev/null +++ b/packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts @@ -0,0 +1,242 @@ +import { describe, expect, it } from 'vitest'; +import { add_autofixers_issues } from './add-autofixers-issues.js'; + +function run(code: string, desired_svelte_version = 5) { + const content: { issues: string[]; suggestions: string[] } = { issues: [], suggestions: [] }; + add_autofixers_issues(content, code, desired_svelte_version); + return content; +} + +describe('svelte-mcp-ignore', () => { + describe('effect_calls_function', () => { + it('suppresses the suggestion when the directive sits on the previous line (`//` form)', () => { + const { suggestions } = run(` + `); + expect(suggestions).not.toContain( + `You are calling the function \`fetch_data\` inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, + ); + }); + + it('also accepts the `/* … */` block form', () => { + const { suggestions } = run(` + `); + expect(suggestions).not.toContain( + `You are calling the function \`fetch_data\` inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, + ); + }); + + it('does NOT suppress when the directive sits two lines above the call', () => { + const { suggestions } = run(` + `); + expect(suggestions).toContain( + `You are calling the function \`fetch_data\` inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, + ); + }); + + it('does NOT suppress unrelated suggestions on the same line', () => { + // `bind:this` and the function-in-effect are different + // codes — silencing one must not silence the other. + const { suggestions } = run(` + + +
`); + expect(suggestions).toContain( + 'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.', + ); + }); + }); + + describe('effect_assigns_state', () => { + it('suppresses the state-assigned-in-effect suggestion', () => { + const { suggestions } = run(` + `); + expect(suggestions).not.toContain( + 'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.', + ); + }); + + it('can list multiple codes on a single directive to silence two suggestions on the same target line', () => { + // Directive scope is **line N+1** (matching + // `svelte-ignore`). Both call + assignment have to + // share the target line for one directive to cover + // both. Splitting them across multiple lines would + // need two directives, one per target line. + const { suggestions } = run(` + `); + expect(suggestions).not.toContain( + `You are calling the function \`fetch_data\` inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, + ); + expect(suggestions).not.toContain( + 'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.', + ); + }); + }); + + describe('bind_this_attachment', () => { + it('suppresses the suggestion via a markup HTML comment directly above the element', () => { + const { suggestions } = run(` + + + +
`); + expect(suggestions).not.toContain( + 'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.', + ); + }); + + it('does NOT suppress when the next element on the line below has no matching directive', () => { + const { suggestions } = run(` + + + +
+ `); + // One element silenced, the next still produces the + // suggestion. The autofixer's `bind:this` message is + // identical for every element it fires on, so we only + // assert it appears (i.e. at least once for ``). + expect(suggestions).toContain( + 'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.', + ); + }); + }); + + describe('imported_runes', () => { + it('suppresses the suggestion for an import line', () => { + const { suggestions } = run(` + `); + expect(suggestions).not.toContain( + `You are importing "state" from "svelte". This is not necessary, all runes are globally available. Please remove this import and use "$state" directly.`, + ); + }); + }); + + describe('derived_with_function', () => { + it('suppresses the suggestion on the next line', () => { + const { suggestions } = run(` + `); + expect(suggestions).not.toContain( + 'You are passing a function to $derived when declaring "v" but $derived expects an expression. You can use $derived.by instead.', + ); + }); + }); + + describe('unused-directive diagnostics', () => { + it('reports a directive whose code never matched', () => { + const { suggestions } = run(` + `); + expect(suggestions.some((s) => s.startsWith('Unused `svelte-mcp-ignore` directive'))).toBe( + true, + ); + }); + + it('reports an unknown code with a hint about valid codes', () => { + const { suggestions } = run(` + `); + const unknown = suggestions.find((s) => + s.startsWith('Unknown `svelte-mcp-ignore` code "not_a_real_code"'), + ); + expect(unknown).toBeTruthy(); + expect(unknown).toContain('Known codes:'); + expect(unknown).toContain('effect_calls_function'); + }); + + it('does NOT report when the directive actually matched something', () => { + const { suggestions } = run(` + `); + expect(suggestions.some((s) => s.startsWith('Unused `svelte-mcp-ignore` directive'))).toBe( + false, + ); + }); + + it('partially-used directives still report the unused codes', () => { + // `effect_calls_function` fires; `bind_this_attachment` + // has nothing to silence on the same line — report + // only the second one as unused. + const { suggestions } = run(` + `); + expect( + suggestions.some( + (s) => + s.startsWith('Unused `svelte-mcp-ignore` directive') && + s.includes('bind_this_attachment'), + ), + ).toBe(true); + expect( + suggestions.some( + (s) => + s.startsWith('Unused `svelte-mcp-ignore` directive') && + s.includes('effect_calls_function'), + ), + ).toBe(false); + }); + }); +}); diff --git a/packages/mcp-server/src/mcp/autofixers/ignore-directives.ts b/packages/mcp-server/src/mcp/autofixers/ignore-directives.ts new file mode 100644 index 00000000..38eca527 --- /dev/null +++ b/packages/mcp-server/src/mcp/autofixers/ignore-directives.ts @@ -0,0 +1,253 @@ +/** + * Per-suggestion suppression for the MCP autofixer. + * + * The compiler and ESLint passes already have first-class + * ignore mechanisms (`` and + * `// eslint-disable-next-line ` respectively). The + * custom-visitor pass that this module guards is the third + * source — `add-autofixers-issues.ts` plus everything under + * `visitors/` — and it had no ignore protocol at all. This + * module adds one. + * + * Comment syntax (mirrors `svelte-ignore`): + * + * + * // svelte-mcp-ignore ... + * /* svelte-mcp-ignore ... *\/ + * + * Scope: a directive on line N suppresses the listed codes + * for every suggestion whose triggering AST node *starts* on + * line N+1. That mirrors what `svelte-ignore` does for + * compiler warnings and keeps the implementation tractable — + * the parser hands us `loc.start.line` on every node, so a + * per-line lookup is `O(1)`. + * + * Each suggestion that this module guards is identified by a + * **stable code** — the keys are documented in + * `IGNORE_CODES`. Code names are lowercase snake_case so they + * read naturally in a comment. + * + * Unused-directive reporting: if a directive lists a code but + * no matching suggestion fires on the next line, we surface + * that as a suggestion of its own (same shape as ESLint's + * `no-unused-svelte-ignore`). Discourages stale comments + * lingering after a refactor. + */ + +import type { ParseResult } from '../../parse/parse.js'; + +/** Stable codes every guarded visitor pushes through. */ +export const IGNORE_CODES = { + /** `assign-in-effect.ts` — function call inside an `$effect` body. */ + EFFECT_CALLS_FUNCTION: 'effect_calls_function', + /** `assign-in-effect.ts` — stateful variable assigned inside an `$effect` body. */ + EFFECT_ASSIGNS_STATE: 'effect_assigns_state', + /** `suggest-attachments.ts` — `bind:this` on a host element could be an attachment. */ + BIND_THIS_ATTACHMENT: 'bind_this_attachment', + /** `suggest-attachments.ts` — `use:` action could be an attachment. */ + USE_ACTION_ATTACHMENT: 'use_action_attachment', + /** `derived-with-function.ts` — `$derived(() => …)` should be `$derived.by`. */ + DERIVED_WITH_FUNCTION: 'derived_with_function', + /** `imported-runes.ts` — runes imported from the Svelte package. */ + IMPORTED_RUNES: 'imported_runes', + /** `use-runes-instead-of-store.ts` — `derived` / `writable` / `readable` from `svelte/store`. */ + RUNES_INSTEAD_OF_STORE: 'runes_instead_of_store', + /** `wrong-property-access-state.ts` — `state.set()` / `state.$` access on stateful variables. */ + WRONG_PROPERTY_ACCESS_STATE: 'wrong_property_access_state', +} as const; + +export type IgnoreCode = (typeof IGNORE_CODES)[keyof typeof IGNORE_CODES]; + +const ALL_CODES: ReadonlySet = new Set(Object.values(IGNORE_CODES)); + +const DIRECTIVE_KEYWORD = 'svelte-mcp-ignore'; + +/** + * One directive collected from a single comment. `codes` is + * intentionally a `Set` (not `Set`) so + * typos by the user surface as unused-directive diagnostics + * rather than being silently dropped at parse time. + */ +type DirectiveEntry = { + codes: Set; + used: Set; + source_line: number; +}; + +export type IgnoreRegistry = ReturnType; + +function build_registry(parsed: ParseResult): { + /** Suppress `code` for any suggestion whose triggering node starts on `line`. */ + is_ignored(line: number, code: string): boolean; + /** Collect every directive code that was declared but never matched. */ + unused_directives(): Array<{ line: number; codes: string[] }>; +} { + /** + * `directives_by_target_line[N] = entry` means "an + * `svelte-mcp-ignore` comment on line N-1 suppresses + * `entry.codes` for any node on line N". `entry.used` + * tracks the subset that actually matched, so we can + * report the unused remainder. + */ + const directives_by_target_line = new Map(); + + function consider(comment_value: string, source_line: number) { + const codes = parse_directive(comment_value); + if (!codes) { + return; + } + const target_line = source_line + 1; + const existing = directives_by_target_line.get(target_line); + if (existing) { + for (const code of codes) { + existing.codes.add(code); + } + return; + } + directives_by_target_line.set(target_line, { + codes: new Set(codes), + used: new Set(), + source_line, + }); + } + + // Script comments — both `// …` and `/* … */` land here + // because `svelte-eslint-parser` runs the underlying JS + // parser with `comment: true`. + for (const comment of parsed.ast.comments ?? []) { + const value = comment.value ?? ''; + const start_line = comment.loc?.start?.line; + if (typeof start_line !== 'number') { + continue; + } + consider(value, start_line); + } + + // Markup comments are top-level AST nodes (`SvelteHTMLComment`), + // not entries in `ast.comments`. We walk the body once and + // pull the directive shape from each one's text. + for (const node of parsed.ast.body ?? []) { + if ((node as { type?: string }).type !== 'SvelteHTMLComment') { + continue; + } + const value = (node as { value?: string }).value ?? ''; + const start_line = (node as { loc?: { start?: { line?: number } } }).loc?.start?.line; + if (typeof start_line !== 'number') { + continue; + } + consider(value, start_line); + } + + return { + is_ignored(line, code) { + const entry = directives_by_target_line.get(line); + if (!entry) { + return false; + } + if (entry.codes.has(code)) { + entry.used.add(code); + return true; + } + return false; + }, + unused_directives() { + const out: Array<{ line: number; codes: string[] }> = []; + for (const entry of directives_by_target_line.values()) { + const unused = [...entry.codes].filter((c) => !entry.used.has(c)); + if (unused.length > 0) { + out.push({ line: entry.source_line, codes: unused }); + } + } + return out; + }, + }; +} + +export function gather_ignore_directives(parsed: ParseResult): IgnoreRegistry { + return build_registry(parsed); +} + +/** + * Pull `code1 code2 …` out of a directive comment body. Returns + * `null` for non-directive comments (the common case) so the + * scan stays cheap. + * + * Whitespace handling matches `svelte-ignore` — any amount of + * leading whitespace before the keyword, and any whitespace + * separator between codes. We deliberately don't accept commas + * as separators because the upstream `svelte-ignore` parser + * doesn't either and we want the two surfaces to feel the same. + */ +function parse_directive(raw: string): string[] | null { + const trimmed = raw.trim(); + if (!trimmed.startsWith(DIRECTIVE_KEYWORD)) { + return null; + } + const rest = trimmed.slice(DIRECTIVE_KEYWORD.length).trim(); + if (rest.length === 0) { + // Bare `svelte-mcp-ignore` with no codes — accept it but + // produce zero entries. The user typed it presumably to + // silence everything; we surface that as "you must list + // codes" via a synthetic unused-directive (code list is + // the empty set, so no code is ever matched against it). + return []; + } + return rest.split(/\s+/); +} + +/** + * Push `message` onto the autofixer's suggestion list unless a + * `svelte-mcp-ignore ` directive on the previous line + * suppresses it. Visitors call this instead of + * `state.output.suggestions.push(...)` directly so the + * suppression check happens in one place. + * + * `target_line` is the line the suggestion's triggering AST + * node lives on. Visitors pass `node.loc?.start?.line` from + * whatever node tripped the check — typically the offending + * call/import/directive itself. + */ +export function push_suggestion( + output: { suggestions: string[] }, + registry: IgnoreRegistry, + code: IgnoreCode, + target_line: number | null | undefined, + message: string, +): void { + if (typeof target_line === 'number' && registry.is_ignored(target_line, code)) { + return; + } + output.suggestions.push(message); +} + +/** + * After every visitor has run, surface any directive that + * declared a code which never matched. The diagnostic is a + * suggestion (not an issue) because the underlying intent is + * "your comment is stale" — same severity as the suggestion + * the comment was trying to silence. + * + * Unknown codes (typos, codes referring to other tools) are + * reported with a hint so the user knows the difference + * between "you wrote `effect_call_function`" and "you wrote + * `effect_calls_function` but the next line doesn't trigger + * that check anymore". + */ +export function report_unused_directives( + output: { suggestions: string[] }, + registry: IgnoreRegistry, +): void { + for (const { line, codes } of registry.unused_directives()) { + for (const code of codes) { + if (!ALL_CODES.has(code)) { + output.suggestions.push( + `Unknown \`svelte-mcp-ignore\` code "${code}" at line ${line}. Known codes: ${[...ALL_CODES].sort().join(', ')}.`, + ); + continue; + } + output.suggestions.push( + `Unused \`svelte-mcp-ignore\` directive for code "${code}" at line ${line} — no matching suggestion was emitted for that location.`, + ); + } + } +} diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts b/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts index 3a58db98..fc40123b 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts @@ -9,6 +9,7 @@ import type { Autofixer, AutofixerState } from './index.js'; import { left_most_id } from '../ast/utils.js'; import type { AST } from 'svelte-eslint-parser'; import type { Context } from 'zimmerframe'; +import { IGNORE_CODES, push_suggestion } from '../ignore-directives.js'; function run_if_in_effect( path: (Node | AST.SvelteNode)[], @@ -39,7 +40,11 @@ function assign_or_update_visitor( init?.type === 'CallExpression' && state.parsed.is_rune(init, ['$state', '$state.raw', '$derived', '$derived.by']) ) { - state.output.suggestions.push( + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.EFFECT_ASSIGNS_STATE, + node.loc?.start?.line, `The stateful variable "${id.name}" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.`, ); } @@ -66,7 +71,11 @@ function call_expression_visitor( run_if_in_effect(path, state, () => { const function_name = node.callee.type === 'Identifier' ? `the function \`${node.callee.name}\`` : 'a function'; - state.output.suggestions.push( + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.EFFECT_CALLS_FUNCTION, + node.loc?.start?.line, `You are calling ${function_name} inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, ); }); diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/derived-with-function.ts b/packages/mcp-server/src/mcp/autofixers/visitors/derived-with-function.ts index c5f9a05b..da6de68c 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/derived-with-function.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/derived-with-function.ts @@ -1,5 +1,6 @@ import type { Identifier, PrivateIdentifier } from 'estree'; import type { Autofixer } from './index.js'; +import { IGNORE_CODES, push_suggestion } from '../ignore-directives.js'; export const derived_with_function: Autofixer = { CallExpression(node, { state, path }) { @@ -33,7 +34,11 @@ export const derived_with_function: Autofixer = { : undefined; } - state.output.suggestions.push( + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.DERIVED_WITH_FUNCTION, + node.loc?.start?.line, `You are passing a function to $derived ${variable_id ? `when declaring "${variable_id.name}" ` : ''}but $derived expects an expression. You can use $derived.by instead.`, ); } diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/imported-runes.ts b/packages/mcp-server/src/mcp/autofixers/visitors/imported-runes.ts index 65b1cd87..54afb6ea 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/imported-runes.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/imported-runes.ts @@ -1,5 +1,6 @@ import { base_runes } from '../../../constants.js'; import type { Autofixer } from './index.js'; +import { IGNORE_CODES, push_suggestion } from '../ignore-directives.js'; const dollarless_runes = base_runes.map((r) => r.replace('$', '')); @@ -37,11 +38,19 @@ export const imported_runes: Autofixer = { source.startsWith('svelte/') || source.startsWith('@sveltejs') ) { - state.output.suggestions.push( + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.IMPORTED_RUNES, + node.loc?.start?.line, `You are importing "${id.name}" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$${id.name}" directly.`, ); } else { - state.output.suggestions.push( + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.IMPORTED_RUNES, + node.loc?.start?.line, `You are importing "${id.name}" from "${source}". If you are trying to import runes to use them this is not necessary, all runes are globally available. Please remove this import and use "$${id.name}" directly. If you are importing the function from a separate library ignore this suggestion.`, ); } diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/index.ts b/packages/mcp-server/src/mcp/autofixers/visitors/index.ts index 263fd1ee..1f3d4c0b 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/index.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/index.ts @@ -2,12 +2,22 @@ import type { Node } from 'estree'; import type { AST } from 'svelte-eslint-parser'; import type { Visitors } from 'zimmerframe'; import type { ParseResult } from '../../../parse/parse.js'; +import type { IgnoreRegistry } from '../ignore-directives.js'; export type AutofixerState = { output: { issues: string[]; suggestions: string[] }; parsed: ParseResult; desired_svelte_version: number; async?: boolean; + /** + * Per-file map of `svelte-mcp-ignore` directives that + * suppress suggestions on the line they target. Visitors + * should route their `state.output.suggestions.push(...)` + * calls through `push_suggestion` from + * `../ignore-directives.js` so the suppression check + * happens in one place. + */ + ignore_registry: IgnoreRegistry; }; export type Autofixer = Visitors; diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/suggest-attachments.ts b/packages/mcp-server/src/mcp/autofixers/visitors/suggest-attachments.ts index 1d72ac0f..90a14a6b 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/suggest-attachments.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/suggest-attachments.ts @@ -1,6 +1,7 @@ import type { Identifier } from 'estree'; import type { Autofixer } from './index.js'; import { left_most_id } from '../ast/utils.js'; +import { IGNORE_CODES, push_suggestion } from '../ignore-directives.js'; export const suggest_attachments: Autofixer = { SvelteDirective(node, { state, next, path }) { @@ -11,7 +12,16 @@ export const suggest_attachments: Autofixer = { if (state.desired_svelte_version === 4) { better_an_attachment = ``; } - state.output.suggestions.push( + // Anchor the directive lookup on the **element** + // line rather than the bind: attribute line — the + // user's `` sits one + // line above the element, and that's the place + // where the suggestion is conceptually pointing. + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.BIND_THIS_ATTACHMENT, + parent_element.loc?.start?.line, `The usage of \`bind:this\` can often be replaced with an easier to read \`action\`${better_an_attachment}. Consider using the latter if possible.`, ); } @@ -35,7 +45,12 @@ export const suggest_attachments: Autofixer = { state.parsed.is_rune(definition.node.init, ['$props']) ) ) { - state.output.suggestions.push( + const parent_element = path.findLast((p) => p.type === 'SvelteElement'); + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.USE_ACTION_ATTACHMENT, + parent_element?.loc?.start?.line, `Consider using an \`attachment\` instead of an \`action\` for "${id.name}".`, ); } diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/use-runes-instead-of-store.ts b/packages/mcp-server/src/mcp/autofixers/visitors/use-runes-instead-of-store.ts index 30d97068..90c1d508 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/use-runes-instead-of-store.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/use-runes-instead-of-store.ts @@ -1,4 +1,5 @@ import type { Autofixer } from './index.js'; +import { IGNORE_CODES, push_suggestion } from '../ignore-directives.js'; export const use_runes_instead_of_store: Autofixer = { ImportDeclaration(node, { state, next }) { @@ -10,7 +11,11 @@ export const use_runes_instead_of_store: Autofixer = { specifier.imported.type === 'Identifier' && ['derived', 'writable', 'readable'].includes(specifier.imported.name) ) { - state.output.suggestions.push( + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.RUNES_INSTEAD_OF_STORE, + node.loc?.start?.line, `You are importing "${specifier.imported.name}" from "svelte/store". Unless the user specifically asked for stores or it's required because some library/component requires a store as input consider using runes like \`$state\` or \`$derived\` instead, all runes are globally available.`, ); } diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/wrong-property-access-state.ts b/packages/mcp-server/src/mcp/autofixers/visitors/wrong-property-access-state.ts index 74057bfe..cdd20a04 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/wrong-property-access-state.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/wrong-property-access-state.ts @@ -1,5 +1,6 @@ import type { Autofixer } from './index.js'; import { left_most_id } from '../ast/utils.js'; +import { IGNORE_CODES, push_suggestion } from '../ignore-directives.js'; const UPDATE_PROPERTIES = new Set(['set', 'update', '$']); const METHODS = new Set(['set', 'update']); @@ -31,7 +32,13 @@ export const wrong_property_access_state: Autofixer = { if (!argument || (argument.type !== 'Literal' && argument.type !== 'ArrayExpression')) { suggestion += ` However I can't verify if "${id.name}" is a state variable of an object or a class with a "${node.property.name}" ${is_property ? 'property' : 'method'} on it. Please verify that before updating the code to use a normal ${is_property ? 'access' : 'assignment'}`; } - state.output.suggestions.push(suggestion); + push_suggestion( + state.output, + state.ignore_registry, + IGNORE_CODES.WRONG_PROPERTY_ACCESS_STATE, + node.loc?.start?.line, + suggestion, + ); } } } diff --git a/packages/mcp-server/src/mcp/handlers/tools/svelte-autofixer.ts b/packages/mcp-server/src/mcp/handlers/tools/svelte-autofixer.ts index f3bb1206..77e3db84 100644 --- a/packages/mcp-server/src/mcp/handlers/tools/svelte-autofixer.ts +++ b/packages/mcp-server/src/mcp/handlers/tools/svelte-autofixer.ts @@ -123,7 +123,7 @@ export function svelte_autofixer(server: SvelteMcp) { name: 'svelte-autofixer', title: 'Svelte Autofixer', description: - 'Given a svelte component or module returns a list of suggestions to fix any issues it has. This tool MUST be used whenever the user is asking to write svelte code before sending the code back to the user', + 'Given a svelte component or module returns a list of suggestions to fix any issues it has. This tool MUST be used whenever the user is asking to write svelte code before sending the code back to the user. When a custom-visitor suggestion is a confirmed false positive (e.g. a deliberate `$effect` whose side effect cannot be a `$derived`), silence it with a `// svelte-mcp-ignore ` script comment or `` markup comment on the line above the triggering node. Available codes: `effect_calls_function`, `effect_assigns_state`, `bind_this_attachment`, `use_action_attachment`, `derived_with_function`, `imported_runes`, `runes_instead_of_store`, `wrong_property_access_state`.', get schema() { return ( cached_schema ?? (cached_schema = get_autofixer_schema(server.ctx.custom?.stdio ?? false)) diff --git a/tools/instructions/AGENTS.md b/tools/instructions/AGENTS.md index 4711cd04..c63ddf28 100644 --- a/tools/instructions/AGENTS.md +++ b/tools/instructions/AGENTS.md @@ -17,6 +17,8 @@ After calling the list-sections tool, you MUST analyze the returned documentatio Analyzes Svelte code and returns issues and suggestions. You MUST use this tool whenever writing Svelte code before sending it to the user. Keep calling it until no issues or suggestions are returned. +Some custom-visitor suggestions are heuristic and can fire on intentional code (e.g. an `$effect` that imperatively pushes state into a third-party library, where `$derived` genuinely doesn't apply). When you've confirmed a suggestion is a false positive, silence it with a `svelte-mcp-ignore` directive on the line above the triggering node — `// svelte-mcp-ignore ` in ` @@ -32,8 +32,6 @@ Some custom-visitor suggestions are heuristic — they're written to nudge a mod ``` -The directive scopes to **the immediately following line** — the same shape `svelte-ignore` uses for compiler warnings. Multiple codes can be listed on one directive (`// svelte-mcp-ignore effect_calls_function effect_assigns_state`). Stale or typo'd codes surface as a follow-up suggestion so the comments don't quietly rot. - Available codes: | Code | What it silences | diff --git a/documentation/docs/40-skills/.generated/skills.md b/documentation/docs/40-skills/.generated/skills.md index a4cd8839..7db9981d 100644 --- a/documentation/docs/40-skills/.generated/skills.md +++ b/documentation/docs/40-skills/.generated/skills.md @@ -70,7 +70,7 @@ npx @sveltejs/mcp svelte-autofixer ./Component.svelte --svelte-version 4 1. **Uncertain about syntax?** Run `list-sections` then `get-documentation` for relevant topics 2. **Reviewing/debugging?** Run `svelte-autofixer` on the code to detect issues 3. **Always validate** - Run `svelte-autofixer` before finalizing any Svelte component -4. **Confirmed false positive?** Some custom-visitor suggestions are heuristic (an `$effect` that imperatively pushes state into a third-party library, a `bind:this` whose attachment refactor is wider than the change at hand). Silence one with a `// svelte-mcp-ignore ` script comment or `` markup comment on the line above the triggering node, then re-run the autofixer. Codes: `effect_calls_function`, `effect_assigns_state`, `bind_this_attachment`, `use_action_attachment`, `derived_with_function`, `imported_runes`, `runes_instead_of_store`, `wrong_property_access_state`. Multiple codes on one directive are space-separated. +4. **Confirmed false positive?** Silence one suggestion with a `// svelte-mcp-ignore ` (script) or `` (markup) comment on the line above, then re-run the autofixer. Codes are listed in the autofixer's reply if you write an unknown one. ```` diff --git a/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts b/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts index a6c21928..824fb409 100644 --- a/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts +++ b/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts @@ -5,8 +5,14 @@ import { base_runes } from '../../constants.js'; const dollarless_runes = base_runes.map((r) => ({ rune: r.replace('$', '') })); function run_autofixers_on_code(code: string, desired_svelte_version = 5) { - const content = { issues: [], suggestions: [] }; + const content: { issues: string[]; suggestions: string[] } = { issues: [], suggestions: [] }; add_autofixers_issues(content, code, desired_svelte_version); + // Suggestion messages carry a trailing ` []` marker + // since the `svelte-mcp-ignore` work — strip it here so + // pre-existing assertions that match the human-readable + // message verbatim keep working. The dedicated ignore- + // directive tests assert on the marker explicitly. + content.suggestions = content.suggestions.map((s) => s.replace(/ \[[a-z_]+\]$/, '')); return content; } diff --git a/packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts b/packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts index 7e49779b..c09ff1dd 100644 --- a/packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts +++ b/packages/mcp-server/src/mcp/autofixers/ignore-directives.test.ts @@ -1,13 +1,57 @@ import { describe, expect, it } from 'vitest'; import { add_autofixers_issues } from './add-autofixers-issues.js'; +/** Run the autofixer and strip the trailing ` []` marker + * from each suggestion so verbatim message assertions stay + * readable. The marker itself is exercised by the dedicated + * `code marker` tests below via `run_raw`. */ function run(code: string, desired_svelte_version = 5) { + const content = run_raw(code, desired_svelte_version); + content.suggestions = content.suggestions.map((s) => s.replace(/ \[[a-z_]+\]$/, '')); + return content; +} + +function run_raw(code: string, desired_svelte_version = 5) { const content: { issues: string[]; suggestions: string[] } = { issues: [], suggestions: [] }; add_autofixers_issues(content, code, desired_svelte_version); return content; } describe('svelte-mcp-ignore', () => { + describe('code marker', () => { + it('emits a trailing `[effect_calls_function]` marker on the call-in-effect suggestion', () => { + const { suggestions } = run_raw(` + `); + expect(suggestions).toEqual([expect.stringMatching(/ \[effect_calls_function\]$/)]); + }); + + it('emits a trailing `[effect_assigns_state]` marker on the state-assigned-in-effect suggestion', () => { + const { suggestions } = run_raw(` + `); + expect(suggestions).toEqual([expect.stringMatching(/ \[effect_assigns_state\]$/)]); + }); + + it('emits a trailing `[bind_this_attachment]` marker on the bind:this suggestion', () => { + const { suggestions } = run_raw(` + + +
`); + expect(suggestions).toEqual([expect.stringMatching(/ \[bind_this_attachment\]$/)]); + }); + }); + describe('effect_calls_function', () => { it('suppresses the suggestion when the directive sits on the previous line (`//` form)', () => { const { suggestions } = run(` diff --git a/packages/mcp-server/src/mcp/autofixers/ignore-directives.ts b/packages/mcp-server/src/mcp/autofixers/ignore-directives.ts index 38eca527..b6dbcd6c 100644 --- a/packages/mcp-server/src/mcp/autofixers/ignore-directives.ts +++ b/packages/mcp-server/src/mcp/autofixers/ignore-directives.ts @@ -206,6 +206,11 @@ function parse_directive(raw: string): string[] | null { * node lives on. Visitors pass `node.loc?.start?.line` from * whatever node tripped the check — typically the offending * call/import/directive itself. + * + * The emitted message carries the code as a trailing + * `[]` marker so an LLM consumer can read the + * suggestion and write the matching `svelte-mcp-ignore` + * directive without consulting any external docs surface. */ export function push_suggestion( output: { suggestions: string[] }, @@ -217,7 +222,7 @@ export function push_suggestion( if (typeof target_line === 'number' && registry.is_ignored(target_line, code)) { return; } - output.suggestions.push(message); + output.suggestions.push(`${message} [${code}]`); } /** diff --git a/packages/mcp-server/src/mcp/handlers/prompts/svelte-task.ts b/packages/mcp-server/src/mcp/handlers/prompts/svelte-task.ts index 8435a6f7..2a10073c 100644 --- a/packages/mcp-server/src/mcp/handlers/prompts/svelte-task.ts +++ b/packages/mcp-server/src/mcp/handlers/prompts/svelte-task.ts @@ -18,7 +18,7 @@ ${available_docs} These are the available documentation sections that \`list-sections\` will return, you do not need to call it again. -Every time you write a Svelte component or a Svelte module you MUST invoke the \`svelte-autofixer\` tool providing the code. The tool will return a list of issues or suggestions. If there are any issues or suggestions you MUST fix them and call the tool again with the updated code. You MUST keep doing this until the tool returns no issues or suggestions. Only then you can return the code to the user. Some custom-visitor suggestions are heuristic and can be confirmed false positives for intentional code; in that case silence the specific suggestion on the line above the triggering node with \`// svelte-mcp-ignore \` in \`