diff --git a/.gitignore b/.gitignore index 03954e0..671845f 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,6 @@ usage_rules-*.tar # Temporary files, for example, from tests. /tmp/ + +# Scratch test file +test/scratch_test.exs diff --git a/lib/mix/tasks/usage_rules.sync.ex b/lib/mix/tasks/usage_rules.sync.ex index 4f1e312..34ccd29 100644 --- a/lib/mix/tasks/usage_rules.sync.ex +++ b/lib/mix/tasks/usage_rules.sync.ex @@ -853,7 +853,7 @@ if Code.ensure_loaded?(Igniter) do acc, ref_path, content, - fn source -> Rewrite.Source.update(source, :content, content) end + fn source -> update_source_content(source, content) end ) end @@ -868,7 +868,7 @@ if Code.ensure_loaded?(Igniter) do inner_acc, ref_path, content, - fn source -> Rewrite.Source.update(source, :content, content) end + fn source -> update_source_content(source, content) end ) end) end) @@ -1038,7 +1038,7 @@ if Code.ensure_loaded?(Igniter) do acc2, dst_path, content, - fn source -> Rewrite.Source.update(source, :content, content) end + fn source -> update_source_content(source, content) end ) end) end) @@ -1346,20 +1346,50 @@ if Code.ensure_loaded?(Igniter) do if String.contains?(current_content, "") do custom = extract_skill_custom_content(current_content) - if custom != "" do - [new_frontmatter, new_managed] = - String.split(new_skill_md, "\n\n", parts: 2) + new_content = + if custom != "" do + [frontmatter, managed_body] = + String.split(new_skill_md, "\n\n", parts: 2) - new_frontmatter <> - "\n\n" <> custom <> "\n\n" <> new_managed + frontmatter <> + "\n\n" <> custom <> "\n\n" <> managed_body + else + new_skill_md + end + + # Only return new content if the managed section actually changed. + # Rewrite.Source.write/2 applies eof_newline/1 which appends "\n" + # to disk content (see rewrite/lib/rewrite/source.ex, write/3 and + # eof_newline/1). The generated content may not end with "\n", so + # subsequent reads see a trailing-whitespace diff that isn't a real + # change. Comparing just the managed section avoids this. + if managed_section_changed?(current_content, new_content) do + new_content else - new_skill_md + current_content end else new_skill_md end end + defp managed_section_changed?(current, new) do + extract_managed_section(current) != extract_managed_section(new) + end + + defp extract_managed_section(content) do + case String.split(content, "", parts: 2) do + [_, rest] -> + case String.split(rest, "", parts: 2) do + [managed, _] -> String.trim(managed) + _ -> String.trim(rest) + end + + _ -> + nil + end + end + defp strip_managed_skill_content(content) do # Remove managed-by metadata from frontmatter content = String.replace(content, ~r/metadata:\n\s+managed-by: usage-rules\n/, "") @@ -1385,6 +1415,21 @@ if Code.ensure_loaded?(Igniter) do String.replace(content, ~r/\A\s*\s*\n*/s, "") end + # Updates a reference file's content, ignoring trailing whitespace differences. + # Reference files have no managed-section markers — the entire file is synced + # content. Rewrite.Source.write/2 applies eof_newline/1 (see + # rewrite/lib/rewrite/source.ex) which appends "\n" when writing to disk, + # but dep content may not end with one, causing false diffs on subsequent syncs. + defp update_source_content(source, new_content) do + current = Rewrite.Source.get(source, :content) + + if String.trim_trailing(current) == String.trim_trailing(new_content) do + source + else + Rewrite.Source.update(source, :content, new_content) + end + end + defp format_yaml_string(str) do str = String.trim(str) diff --git a/test/mix/tasks/usage_rules.sync_test.exs b/test/mix/tasks/usage_rules.sync_test.exs index d670dcf..bf66cb8 100644 --- a/test/mix/tasks/usage_rules.sync_test.exs +++ b/test/mix/tasks/usage_rules.sync_test.exs @@ -1455,4 +1455,189 @@ defmodule Mix.Tasks.UsageRules.SyncTest do |> assert_creates(".claude/skills/foo-built/SKILL.md") end end + + describe "idempotency (--check)" do + # Rewrite.Source.write/2 calls eof_newline/1 before File.write/2 + # (see rewrite/lib/rewrite/source.ex line ~297 and ~970): + # + # defp write(%Source{path: path, content: content}, ...) do + # file_write(path, eof_newline(content)) + # end + # defp eof_newline(string), do: String.trim_trailing(string) <> "\n" + # + # In test mode, simulate_write stores raw content without this + # normalization, so tests don't reproduce the trailing-newline mismatch + # that causes --check failures in real usage. This helper applies the + # same eof_newline transform to simulate a real disk round-trip. + defp simulate_disk_roundtrip(igniter) do + test_files = + Enum.reduce(igniter.assigns[:test_files], igniter.assigns[:test_files], fn + {"deps/" <> _, _content}, acc -> + acc + + {_path, ""}, acc -> + acc + + {path, content}, acc -> + Map.put(acc, path, String.trim_trailing(content) <> "\n") + end) + + igniter + |> Map.put(:rewrite, Rewrite.new()) + |> Map.put(:assigns, %{ + test_mode?: true, + test_files: test_files, + igniter_exs: igniter.assigns[:igniter_exs] + }) + |> Igniter.include_glob("**/*.*") + end + + test "second sync reports no changes for skills.build" do + config = [ + skills: [ + location: ".claude/skills", + build: [ + "use-foo": [ + description: "Foo skill", + usage_rules: [:foo, :bar] + ] + ] + ] + ] + + igniter = + project_with_deps(%{ + "deps/foo/usage-rules.md" => "# Foo Rules\n\nUse foo wisely.", + "deps/bar/usage-rules.md" => "# Bar Rules\n\nUse bar wisely." + }) + |> sync(config) + |> assert_creates(".claude/skills/use-foo/SKILL.md") + |> assert_creates(".claude/skills/use-foo/references/foo.md") + |> assert_creates(".claude/skills/use-foo/references/bar.md") + |> apply_igniter!() + |> simulate_disk_roundtrip() + + igniter + |> sync(config) + |> assert_unchanged() + end + + test "second sync reports no changes for skills.deps" do + config = [ + skills: [ + location: ".claude/skills", + deps: [:foo] + ] + ] + + igniter = + project_with_deps(%{ + "deps/foo/usage-rules.md" => "# Foo Rules\n\nUse foo wisely." + }) + |> sync(config) + |> assert_creates(".claude/skills/use-foo/SKILL.md") + |> apply_igniter!() + |> simulate_disk_roundtrip() + + igniter + |> sync(config) + |> assert_unchanged() + end + + test "second sync reports no changes for AGENTS.md" do + config = [ + file: "AGENTS.md", + usage_rules: [:foo] + ] + + igniter = + project_with_deps(%{ + "deps/foo/usage-rules.md" => "# Foo Rules\n\nUse foo wisely." + }) + |> sync(config) + |> assert_creates("AGENTS.md") + |> apply_igniter!() + |> simulate_disk_roundtrip() + + igniter + |> sync(config) + |> assert_unchanged() + end + + test "second sync reports no changes with sub-rules" do + config = [ + skills: [ + location: ".claude/skills", + build: [ + "use-foo": [usage_rules: [:foo]] + ] + ] + ] + + igniter = + project_with_deps(%{ + "deps/foo/usage-rules.md" => "# Foo Rules", + "deps/foo/usage-rules/testing.md" => "# Testing Guide" + }) + |> sync(config) + |> assert_creates(".claude/skills/use-foo/SKILL.md") + |> assert_creates(".claude/skills/use-foo/references/foo.md") + |> assert_creates(".claude/skills/use-foo/references/testing.md") + |> apply_igniter!() + |> simulate_disk_roundtrip() + + igniter + |> sync(config) + |> assert_unchanged() + end + + test "second sync reports no changes with custom content in SKILL.md" do + config = [ + skills: [ + location: ".claude/skills", + build: [ + "use-foo": [ + description: "Foo skill", + usage_rules: [:foo] + ] + ] + ] + ] + + # First sync creates the skill + igniter = + project_with_deps(%{ + "deps/foo/usage-rules.md" => "# Foo Rules\n\nUse foo wisely." + }) + |> sync(config) + |> assert_creates(".claude/skills/use-foo/SKILL.md") + |> assert_creates(".claude/skills/use-foo/references/foo.md") + |> apply_igniter!() + + # Inject custom content between frontmatter and managed section + skill_content = igniter.assigns[:test_files][".claude/skills/use-foo/SKILL.md"] + + [frontmatter, managed] = + String.split(skill_content, "\n\n", parts: 2) + + custom_skill = + frontmatter <> + "\n\nMy custom instructions go here.\n\n" <> + managed + + test_files = + Map.put(igniter.assigns[:test_files], ".claude/skills/use-foo/SKILL.md", custom_skill) + + igniter = put_in(igniter.assigns[:test_files], test_files) + + igniter = + igniter + |> simulate_disk_roundtrip() + + # Second sync should preserve custom content and report no changes + igniter + |> sync(config) + |> assert_unchanged() + end + end end