development

Instruction file imported from stbenjam/skillsaw (.cursor/rules/development.mdc). Copyright stays with the author.

Follow these rules when developing skillsaw, a configurable, rule-based linter for agentic contextual building blocks.

Pre-push Checklist

CRITICAL: You MUST always run these steps before pushing changes.

1. Ensure branch is up to date with UPSTREAM (stbenjam/skillsaw) — check git remotes then merge upstream's main.
2. Run make test — run the full test suite and ensure all tests pass. Never assume a problem is pre-existing; follow the boyscout rule and fix it anyway.
3. Run make lint — check formatting (or run make format to fix it).
4. Run make update — regenerate all generated files (must come after version bump).
5. Test against openshift-eng/ai-helpers: clone it, run skillsaw, and ensure exit 0.

Pre-PR Checklist

Follow these checks before opening a PR:

• Check whether documentation is up to date for the changes on this branch.
  • Example: you add a new feature, flag, or lint type. Action: update README.md to include this.
  • Example: you encounter a problem during development you must keep for later. Action: update .apm/instructions and run make update.
• Verify test coverage on a new feature, flag, lint type or bug fix is complete.
  • Bug fixes must include regression protection.
  • New features, linters, and rules need integration test coverage WITH fixtures (see testing rules).

Authoring Agentic Context

When you create or edit a skill, slash command, agent, hook, or instruction file in this repo, load the skillsaw-lint skill and follow it: lint the file with .venv/bin/skillsaw lint <path>, apply fixes, and re-lint until clean before you finish. We ship the linter — our own context must pass it.

Post-PR Checklist

After opening a PR, keep monitoring for feedback from CodeRabbit, Gemini, and stbenjam. You may stop monitoring 20 minutes after you push a PR. Review and handle valid feedback that comes in.

Performance

Benchmark lint speed with the harness in benchmarks/ (make benchmark, make profile, make benchmark-save / make benchmark-compare for regression checks — review DEVELOPMENT.md). When touching content rules, the lint tree, or utils.py read paths, save a baseline on main and compare on your branch.

• Avoid scanning every line against every regex pattern. Run patterns_matching_anywhere(body, patterns) from content_analysis first to filter down to patterns that actually match the text, then scan line-by-line only with surviving patterns.
• lint_tree.find(NodeType) is memoized per node — keep it valid: anything that mutates tree structure outside a rebuild must call invalidate_find_cache() (see FrontmatteredBlock.write_frontmatter_text).
• Read top-level frontmatter key lines from frontmatter_key_line(), which uses a libyaml-backed line map with a ruamel fallback. Don't add per-key ruamel parses. Avoid ruamel round-trip parsing — it is ~30x slower and was the dominant cost of lint-tree construction.
• Keep per-blob work (whole-body .lower(), config-file stats) outside the per-block loop — run it once per check().
• Use fast string checks before running heavy regexes. Simple checks like str.isascii(), str.translate(), or "/" in text quickly filter out clean text so regexes only run when necessary.
• Use paths.relative_to_str() for repo-relative paths. On Python 3.12+, PurePath.relative_to and path.parents allocate new Path objects for every ancestor, which adds significant overhead in large repos.
• Load YAML with yaml.load(..., Loader=_SAFE_LOADER) from utils.py instead of bare yaml.safe_load — _SAFE_LOADER uses the fast C-based LibYAML loader when available (~10x faster) while maintaining safe loading behavior.
• Linter.run() / fix() pause Python's cyclic garbage collector during rule execution to avoid expensive GC pauses across millions of objects. Rules should avoid circular references so temporary data is freed immediately via reference counting.

Markdown: AST for reading, splice for writing

Read markdown structure (links, code spans, fences, HTML comments, headings) from the markdown-it-py AST — read it via block.markdown (a skillsaw.markdown_doc.MarkdownDoc). Never hand-roll per-line regexes for markdown structure.

• Detection: use the MarkdownDoc accessors — links(), code_spans(), text_segments(), fences(), headings(), html_comments(), prose_lines(). Scanning rules needing only prose read it automatically through read_body(strip_code_blocks=True).
• Fixes: splice at the token spans the check matched using markdown_doc.splice(content, edits) — never re-locate fix targets with line.find() / str.replace() — avoid them; they corrupt substrings of other tokens.
• Never render the AST back to markdown — round-trip rendering reformats whole files and violates the scope-the-fix autofix rule.
• Use markdown_doc.file_span() to translate body-relative columns to file columns before splicing (YAML-embedded bodies are indented in the file); skip the edit — return early — when file_span() yields None.

New Linter Rules

• Make rules configurable when there are tuneable settings.
• Read declared options with self.setting() so overrides, schema defaults, and explicit nulls follow one contract. Declare every literal config key the rule reads; tests/test_rule_registry.py enforces this for builtin rule class bodies.
• Never break existing rules for users of skillsaw.
• Rules register themselves — never hand-maintain import lists or config dicts. skillsaw auto-discovers any concrete Rule subclass under src/skillsaw/rules/builtin/. Keep defaults on the class: default_enabled and default_severity() define the single source of truth, and LinterConfig.default() is generated from them.
• New rules default to enabled: auto or enabled: false. Set default_enabled = False for opt-in; the base class default is "auto". Never force-enable a new rule that could break existing users.
• Use the lint tree for discovery — call context.lint_tree.find(NodeType).

Making rules helpful by default

Setting enabled: auto means a rule provides clear, actionable value on typical repositories without extra setup. Before shipping a rule with auto enabled, test it against real-world repositories rather than just unit test fixtures.

Evaluate every new rule against these three practical questions:

1. Is it actionable on common setups? Findings should point to things developers genuinely want to fix. If a check only applies to niche or rare configurations, make it opt-in (default_enabled = False). Be especially mindful of JSON files, which lack inline disable comments and require configuration tweaks to silence.
2. Is it accurate? Test the rule against at least 10 real repositories. Validate edge cases directly against the official schema, loader, or CLI rather than memory. Aim for a false-positive rate under 10% for auto rules, and zero known false positives for ERROR rules.
3. Is it high signal? A rule that fires dozens of times across healthy, working projects quickly becomes noise. When a pattern appears frequently across a repository, aggregate related issues or provide sensible thresholds so the output stays helpful and focused.

Match severity to impact:

• ERROR: The tool will fail to load or run the file, or the issue presents a real security risk.
• WARNING: The file functions, but quality or maintainability is noticeably degraded.
• INFO: Gentle suggestions, style guidance, or advisory review prompts.

Always respect the configured severity: never hardcode severity= on the primary violation, so user overrides in .skillsaw.yaml work seamlessly.

Keep findings concise and consolidated:

• One defect, one finding: Group related issues together. For example, report invalid list items or a folder of unreferenced files as a single consolidated finding with examples.
• Recognize common placeholders: Understand standard templates and sample strings (such as ${VERSION}, dummy auth tokens, or example endpoints) so helpful documentation isn't flagged as invalid.
• Clear, actionable messages: State what happened and how to fix it. Suggest rule configuration options (like exclusions or custom thresholds) before suggesting a disable comment. Focus checks on what files actually do rather than just where they are placed.

Line numbers and the parse tree

• Always report line numbers on every violation traceable to a specific line, except whole-file violations.
• Use read_yaml_commented() (from utils.py) for YAML — never use yaml.safe_load() or read_yaml(). Keep line numbers via the ruamel.yaml objects read_yaml_commented() returns.
• Use commented_key_line(node, key) / commented_item_line(node, index) for 1-based line numbers from ruamel structures.
• Use read_toml() (from utils.py) for TOML. It returns (data, error).
• Never fabricate line numbers — if a field is missing, omit the line.
• Declare repo_types to control when enabled: auto fires.
• Declare config_schema when the rule accepts parameters.
• Always keep EVERYTHING part of the parse tree.
• Keep the prose/config split in the block hierarchy — ContentBlock subclasses are prose for an agent's context window and get every content-quality rule automatically, so never put a config file under one: content rules would lint it as instruction text. Structured JSON (settings, hooks, MCP) subclasses JsonConfigBlock; structured YAML or TOML gets a direct LintTarget subclass, since that hierarchy is JSON-specific — OpenAIMetadataBlock and GrokConfigBlock are the patterns. One exception, CodexConfigHooksBlock: a hooks file in another syntax stays a HooksBlock — the hooks rules iterate that hierarchy and no hooks role mixin exists — and declares its syntax on the block (syntax_name, mapping_noun) so no rule branches on it.

Never require line numbers for JSON or TOML: neither parser gives a position, so both report at file level.

Autofix invariants

• Avoid negative lookarounds around backtrackable quantifiers in patterns that feed autofixes. A trailing (?!\)) after \.\w{1,10} never rejects the match — the engine backtracks into a truncated match and the fix splices a corrupted span (issue #321). Match greedily, then check the characters adjacent to the full match in code and reject there.
• Fixes that add a frontmatter field must guard against the key already existing — check() may report "missing" for an empty/null value while the key: line is still present. Use replace_frontmatter_field() first and fall back to prepend_frontmatter_fields(), or the fix prepends a duplicate key on every run and never converges (issue #321).
• Add new SAFE-autofix edge cases to the tests/fixtures/autofix/safe-idempotency fixture so TestSafeAutofixIdempotency guards them; update EXPECTED_SAFE_VIOLATIONS when the fixture grows.
• Autofix stands down entirely for plugins installed under .codex/plugins/ — vendor-managed content is diagnostic-only, so every rule's fix() silently no-ops there, by design. A new rule's fix needs no guard of its own for that case, and a "fix didn't apply" report from such a path is expected behavior, not a bug.

Ecosystem provenance

"Which ecosystem owns this plugin directory" is decided in exactly one place: RepositoryContext.provenance(), which returns a cached PluginProvenance record per directory. It lives on RepositoryProvenanceMixin in repository_provenance.py, alongside in_format_scope, is_codex_only_plugin, and the containment helpers; RepositoryContext mixes that in, so every caller still reaches it as RepositoryContext.provenance(). Never answer an ownership question with a fresh filesystem probe in a rule or in the tree builder — two call sites probing independently is how a directory falls between per-ecosystem attach paths and loses its content silently.

• Discovery is state-free and lives in src/skillsaw/discovery/ (claude.py, codex.py, agent_plugins.py, detect.py, and excludes.py): functions take a root path and callbacks and return data, holding no caches and importing nothing from context. RepositoryContext is the stateful orchestrator — its methods wrap the discovery functions with the per-context caches and render the provenance verdicts over the evidence they gather.
• Evidence is filesystem-first and --type-invariant. An override changes what discovery walks, not what the author declared, so provenance reads markers, contained manifests, and catalog files directly (_codex_catalog_files()), never discovery output.
• The lint tree builds plugins in ONE pass over the union of claimed directories. Each directory gets one container; prose (commands/, agents/, rules/, README) attaches once with containment; config files attach per claiming ecosystem through the contained helpers. Never add a second per-ecosystem plugin loop.
• Format rules gate on provenance declaratively, never with inline guards. A rule that enforces one ecosystem's conventions declares provenance_scope = "claude" on its class and iterates targets through self.scoped_find(context, NodeType); the filtering happens in exactly one place (RepositoryContext.in_format_scope, reading each node's provenance_dir()), so a Codex-only directory is exempt while dual-manifest and unclaimed directories keep every check. TestProvenanceScopeMechanism pins which rules declare the scope. The mechanism fails open by node type: LintTarget.provenance_dir() returns None unless the node type overrides it. Grep def provenance_dir for the current set — today PluginNode, AgentPluginNode, and AgentPluginConfigNode in lint_target.py, plus CommandBlock and AgentBlock in the frontmatter blocks module. A scope-declaring rule iterating a node type without the override silently gets no filtering, so add the override first.
• provenance_scope is for rules iterating SHARED node types. A rule reading CommandBlock or PluginNode — types every ecosystem populates — needs the declaration to stay out of another ecosystem's directories. A rule iterating an ecosystem-exclusive node type (AgentPluginConfigNode, CodexPluginConfigNode) is already scoped by its target, and declaring provenance_scope there is a bug: under a forced --type with no filesystem claim the scope filter skips the node, so the rule never reports a missing or malformed manifest. That is why agent-plugin-json-valid and agent-plugin-mcp-valid use plain find(AgentPluginConfigNode).
• Conditional strictness is not a skip. The ecosystem-tightened MCP shape checks stay provenance_scope = None and gate the tightened checks alone on context.in_codex_only_plugin(path) — the one spelling of "does a Codex-exclusive plugin own this file" — so dual-manifest plugins keep their established Claude results (TestDualManifestBackwardCompat pins this).
• Hooks get one subclass per host instead of conditional strictness. Each host's hooks file is its own HooksBlock subclass — Claude, Codex, Muse, Cursor, Grok, Antigravity — chosen from provenance in build_lint_tree, with a <host>-hooks-valid rule iterating it. hooks-dangerous and hooks-prohibited read the shared HooksBlock base, so a new host needs no changes there. Exception: GrokPluginHooksBlock has no shape rule — another adapter loads a Grok plugin's hooks file, and 1.0.13 shows no observable.

Ecosystems and editor tools are different problems. An ecosystem packages and installs content (Claude plugins, Codex, Grok Build, Agent Plugins, Antigravity), so it needs provenance: two can claim one directory, and the format rules must stay out of each other's trees. An editor tool (Cursor, Copilot, Cline, Qwen) reads its own configuration locations, which no other tool claims, so it needs no provenance machinery at all. Both are RepositoryType members — detection produces one set, and that enum is the only vocabulary. Pick the recipe that matches; following the ecosystem one for an editor tool builds machinery that design does not need.

Adding an editor tool (Cursor is the worked example): add its directory name to AGENT_TOOL_DIR_NAMES in the detect.py discovery module if it reads a directory rather than a single root file, so one walk finds it anywhere in the tree; add its skill directory to CONVENTIONAL_SKILL_DIRS in the discovery package, plus NESTED_TOOL_SKILL_DIRS in detect.py when a package can carry its own <dir>/skills/ — CONVENTIONAL_SKILL_DIRS is root-anchored and reaches only the repository root's copy; add a RepositoryType member in repository_types.py, list it in TOOL_REPO_TYPES, give it a slot in RepositoryContext._TYPE_PRIORITY (context.py) or the JSON and SARIF reports' repo_type falls through to unknown; key its evidence by that member's value in _TOOL_EVIDENCE (or a marker() check) in tool_types() — detection must agree with attachment, or the lint tree grows blocks no gated rule ever looks at; add block classes whose category encodes the budget role (command for on-demand prompts, instruction for always-on prose, agent for subagents); attach them in the editor-directory loop in build_lint_tree. Structural rules for the tool declare repo_types = frozenset({RepositoryType.<TOOL>}) and iterate their own block type — never provenance_scope, which is for shared node types only. A tool that routes agents or commands by description also needs its type in content-description-routing's repo_types and those block types in that rule's traversal list — it walks an explicit list, so attaching is not enough. Prefer subclassing a shared block (VsCodeMcpBlock(McpBlock)) over editing existing rule files, so the security rules pick the tool up without a visit.

Four more, from OpenCode (formats/opencode.py, rules/builtin/opencode/): check APM_COMPILED_DIR_TARGETS for a tool whose directory is also a compile target (.opencode is, resolved on APM's evidence alone); set jsonc: ClassVar[bool] = True on a JSONC host's block, never a Claude-family one; keep configuration vocabulary — key sets, alias tables — in formats/<tool>.py, not restated in rules; and when a dialect diverges enough that a shared rule's every check misfires, subclassing isn't enough — defer to the tool's own, as mcp-valid-json does for Agent Plugins, OpenCode and Grok. Declare that deferral on the block, never as a branch in the rule: McpShapeDeferral carries the gating repository type, whether the dialect-neutral checks survive, and which rule owns the syntax failure.

Adding an ecosystem (Codex and Agent Plugins are the worked examples): put its discovery leg — the state-free plugin/manifest walks, catalog enumeration, local-source resolution and install-location helpers — in a new src/skillsaw/discovery/<ecosystem>.py; add its evidence probe to provenance() in repository_provenance.py, and its context wrappers (caching, --type gating) in context.py, or a repository_<ecosystem>.py mixin when its line cap bites (Grok's); a RepositoryType member per packaging claim, in SKILL_REPO_TYPES when its plugins carry skills — which turns the agentskill-* rules on but discovers none until discover_skills and context._discover_skills gain a leg — and in _TYPE_PRIORITY, or repo_type reads unknown; its marker in PLUGIN_MARKER_DIR_NAMES (discovery/detect.py), so one walk finds a package's plugin or catalog, never a second traversal; its config-file cluster in the single plugin pass in build_lint_tree (through a contained helper); teach in_format_scope nothing — it already reads the provenance claim set, so every provenance_scope-declaring rule honors the new claim the moment the evidence probe lands; give the new ecosystem's format rules provenance_scope = "<ecosystem>" only where they iterate a shared node type — rules that iterate the ecosystem's own node type gate by node type instead (see the scope rule above); extend the union in merge_plugin_dirs callers if it discovers plugin directories of its own. Prose needs no work — every claimed directory already gets it, and no existing rule needs a scope visit: scope is declared per rule class. A rule keying on an explicit repo_types list is the exception.