fix(configure): use ~/.claude/rules/ for claude-cli, not CLAUDE.md#35
Open
t-dub wants to merge 6 commits into
Open
fix(configure): use ~/.claude/rules/ for claude-cli, not CLAUDE.md#35t-dub wants to merge 6 commits into
t-dub wants to merge 6 commits into
Conversation
Claude Code auto-loads any *.md file under ~/.claude/rules/ on every session (see https://code.claude.com/docs/en/memory#user-level-rules), so the Snyk rules belong in their own dedicated file rather than injected as a delimited block into the user's global ~/.claude/CLAUDE.md. Why this matters: - ~/.claude/CLAUDE.md is the user's personal preferences file. Tool-managed content there is unwelcome pollution and survives uninstall as a leftover delimited block unless removeGlobalRules runs successfully. - A dedicated file uninstalls cleanly with a single os.Remove instead of a delimiter-based search-and-replace inside a shared file. - Mirrors the pattern already used for Cursor (globalSkillsPath → ~/.cursor/skills/snyk-rules/SKILL.md) — claude-cli was the inconsistent one in the existing codebase. Migration safety: a new legacyGlobalRulesPath field on hostConfig points at the prior ~/.claude/CLAUDE.md location. Both addConfiguration and removeConfiguration now invoke removeGlobalRules against it (idempotent — no-op when the delimited block is absent), so users upgrading from a prior install have the old block automatically stripped from their CLAUDE.md without losing any of their own personal content around it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Adds a new globalDedicatedRulesPath field on hostConfig distinct from globalSkillsPath, and embeds Claude-Code-specific rules content under internal/configure/rules/claude/ that omits the Cursor SKILL.md frontmatter (name:/description:) it doesn't recognize. Before this commit the claude-cli host wrote Cursor-style SKILL frontmatter into ~/.claude/rules/snyk-security.md. Six tribunal agents flagged this independently as a content/format mismatch: Claude Code's user-level-rules schema does not recognize the SKILL keys, and relying on the loader silently ignoring unknown frontmatter is fragile. The split also resolves the leaky-abstraction concern that "globalSkillsPath" was lying for claude-cli — the path pointed at a rules directory, not a skills one. Cursor continues to use globalSkillsPath unchanged. claude-cli now uses globalDedicatedRulesPath, which is wired into the same writeGlobalSkills machinery (it's just "write a whole file, no delimiters" — the function name remains accurate to its semantics, even if it predated this split). Addresses Tribunal finding #F1 (content/format mismatch, score 82) from review tribunal-2026-05-05T20-11-37Z-pr-35-f6cf7a69.json. Addresses Tribunal finding #F7 (naming / leaky abstraction, score 75) from review tribunal-2026-05-05T20-11-37Z-pr-35-f6cf7a69.json. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> -- ᴛ-ᴅᴜʙ's Review Tribunal ⚖️
…s gate
Previously the migration cleanup that strips the old delimited Snyk block
from ~/.claude/CLAUDE.md sat inside `if configureRules { ... }` in both
addConfiguration and removeConfiguration. A user invoking
`studio-mcp configure --tool claude-cli --no-rules` (MCP only) — a
perfectly reasonable invocation — would never have their old CLAUDE.md
Snyk block cleaned up, even though Snyk has abandoned that location.
Migration housekeeping is conceptually independent of rules configuration:
once we've decided the legacy block is stale (the prior PR did), it should
be cleaned up regardless of whether the user is configuring rules this run.
This commit moves only the legacyGlobalRulesPath cleanup out of the gate;
legacyLocalRulesPath stays inside (its semantics — "you're migrating from
local to global rules" — are inherently tied to rules being configured).
Addresses Tribunal finding #F12 (migration completeness — gating, score 60)
from review tribunal-2026-05-05T20-11-37Z-pr-35-f6cf7a69.json.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
-- ᴛ-ᴅᴜʙ's Review Tribunal ⚖️
…on test
Adds TestConfigure_ClaudeCliEndToEnd, which drives Configure() — not the
write/remove helpers — for the claude-cli host against a temp HOME and
asserts the full migration story:
1. add: writes ~/.claude/rules/snyk-security.md with the new
Claude-Code-shaped content (no Cursor SKILL frontmatter), strips
the delimited Snyk block from a pre-existing CLAUDE.md, and
preserves user content above and below the block.
2. remove: deletes the dedicated rules file and strips the legacy
block; user content survives.
3. MCP-only configure (configureRules=false): legacy CLAUDE.md cleanup
still runs (pinning the F12 behavior contract — migration
housekeeping is independent of whether rules are configured).
Pre-existing tests pinned the helper-level behavior of removeGlobalRules
but never exercised the wiring through addConfiguration /
removeConfiguration — so a future refactor that dropped the migration
call sites would slip past every other test in this package. This test
catches that.
Implementation notes: a small noopUserInterface stub satisfies
ui.UserInterface without polluting test output. The test is skipped on
Windows because t.Setenv("HOME",...) does not redirect
os.UserHomeDir() there (it reads USERPROFILE) — the migration logic
itself is platform-agnostic; we just rely on Linux/macOS to drive it.
Addresses Tribunal finding #F3 (test gap / integration-untested behavior, score 80) from review
tribunal-2026-05-05T20-11-37Z-pr-35-f6cf7a69.json.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
-- ᴛ-ᴅᴜʙ's Review Tribunal ⚖️
… content removeDelimitedBlock and upsertDelimitedBlock previously located markers via strings.Index (first occurrence). A user's CLAUDE.md that quoted the literal RuleStart token in a sentence above the real Snyk block — e.g. a how-to doc explaining the migration — would have everything between the user's quote and Snyk's real RuleEnd silently deleted by the migration cleanup. The new migration runs unconditionally on every claude-cli install, so the failure mode would compound over time. This commit replaces the index-based search with line-anchored matching: both markers must appear on their own line (no surrounding text), which is exactly what writeGlobalRules / writeGlobalSkills emit. User content that quotes the marker text inline within a sentence no longer matches and is preserved. When multiple complete delimited pairs exist (e.g. user pasted the marker on its own line in a fenced code block AFTER the real block), the new findDelimitedBlockLines helper scans from the end and returns the last START that has a matching END after it — biased toward the real block rather than orphan user-quoted markers. A new TestRemoveDelimitedBlock_LineAnchored_PreservesUserQuotedMarkers pins both pathological cases. The existing TestRemoveDelimitedBlock and TestUpsertDelimitedBlock cases pass unchanged — the line-anchored algorithm produces identical output for any input the prior implementation got right; it differs only in the user-quoted-marker case it was getting wrong. Addresses Tribunal finding #F2 (data destruction / migration safety, score 80) from review tribunal-2026-05-05T20-11-37Z-pr-35-f6cf7a69.json. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> -- ᴛ-ᴅᴜʙ's Review Tribunal ⚖️
…leanup in UI Extracts the four near-identical legacy-cleanup blocks (legacy_local + legacy_global, in both addConfiguration and removeConfiguration) into two small helpers: cleanupLegacyLocalRules (workspace-relative, quiet) and cleanupLegacyGlobalRules (announces in the CLI, surfaces failures with a concrete file path so the user can inspect manually). The two-helper split (rather than one bundled cleanupLegacyPaths) reflects the different gating: cleanupLegacyLocalRules is still called from inside the configureRules block (its local→global migration semantics are tied to rules being configured this run), while cleanupLegacyGlobalRules is called from outside the gate (migration housekeeping is unconditional — see prior commit ac19d72). The user-facing breadcrumb on legacy global cleanup matters because the whole point of moving claude-cli rules out of CLAUDE.md is to stop silently mutating a user-owned file. Announcing the cleanup before it runs and surfacing failures keeps that contract honest: a user who's now told their CLAUDE.md will be edited can see when it actually happens. Addresses Tribunal finding #F4 (duplication / DRY, score 80) from review tribunal-2026-05-05T20-11-37Z-pr-35-f6cf7a69.json. Addresses Tribunal finding #F10 (silent failure / user-visibility, score 65) from review tribunal-2026-05-05T20-11-37Z-pr-35-f6cf7a69.json. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> -- ᴛ-ᴅᴜʙ's Review Tribunal ⚖️
Author
|
I heard from the team that the new direction is actually going to be hooks, via [this](https://github.com/snyk/studio-recipes/tree/main/installer] or something similar, so presumably the logic I'm touching here is going to be retired wholesale. I'll leave the PR open for now, but I'm assuming it's mostly irrelevant with that direction in mind. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
mcp configure --tool=claude-cliruns today, it injectsrules formated block into the user's global
~/.claude/CLAUDE.md.User-level rules is the correct location for the way this rule is formatted. I agree that this is best suited to a discrete rule as it makes clear what the source is, but injecting it into the user's global
CLAUDE.mdisn't the right spot.This PR moves the
claude-clihost to the dedicated-file pattern that thecodebase already uses for Cursor (
globalSkillsPath), and mirrors theexisting
legacyLocalRulesPathmigration mechanism with a newlegacyGlobalRulesPathfield so existing installs are upgraded cleanly.Behavior change
~/.claude/CLAUDE.md(delimited inject)~/.claude/rules/snyk-security.md(dedicated file)os.Removeof the dedicated file~/.claude/CLAUDE.md(idempotent — surrounding user content preserved)Migration safety
The cleanup is intentionally non-destructive:
removeGlobalRulesonly removes content between<!--# BEGIN SNYK GLOBAL RULE -->and<!--# END SNYK GLOBAL RULE -->.~/.claude/CLAUDE.mdaround the Snykblock, those edits are preserved.
A new test case (
TestRemoveGlobalRulesIsClaudeCliMigrationSafe) verifiesthis round-trip on a CLAUDE.md that mixes user content and a delimited
Snyk block.
Test plan
go test ./...— all existing tests still passgo vet ./...— cleanmake lint— 0 issues (GOTOOLCHAIN=go1.25.7to match the pinnedgolangci-lint v2.6.1 build environment on machines running newer Go)
TestGetHostConfig_ClaudeCliPathspins the exact pathsTestRemoveGlobalRulesIsClaudeCliMigrationSafeexercises themigration cleanup against a representative pre-existing CLAUDE.md
TestGetHostConfigupdated forclaude-cli— now expectsglobalSkillsPathandlegacyGlobalRulesPath🤖 Generated with Claude Code