diff --git a/CHANGELOG.md b/CHANGELOG.md index 114ff5439..77e6c1abe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING: Windsurf skills now deploy to `.agents/skills//SKILL.md` + instead of `.windsurf/skills//SKILL.md` (skill routing convergence).** + Cascade + [natively discovers `.agents/skills/`](https://docs.windsurf.com/windsurf/cascade/skills#skill-scopes) + at both workspace and user scope, so Windsurf joins the existing convergence + with Copilot, Cursor, Codex, Gemini, and OpenCode (6 clients total) and APM + no longer writes a separate `.windsurf/skills/` copy. Claude remains on its + native `.claude/skills/` routing. **Migration:** the first `apm install` + after upgrading auto-migrates any `.windsurf/skills//` recorded in + `apm.lock.yaml` to `.agents/skills/` and deletes the stale copy (idempotent; + foreign/hand-authored skills are untouched; aborts with a clear error on a + content collision). If you never re-run `apm install`, delete the stale + directories by hand: `rm -rf .windsurf/skills//`. Pass + `--legacy-skill-paths` (or set `APM_LEGACY_SKILL_PATHS=1`) to keep the + pre-convergence `.windsurf/skills/` layout. (closes #1520) + ### Removed - `apm marketplace publish` command and consumer-repo fan-out workflow; consumers should run `apm install --update` instead. (#1134) diff --git a/docs/src/content/docs/getting-started/migration.md b/docs/src/content/docs/getting-started/migration.md index 225d1a98f..086bbfca5 100644 --- a/docs/src/content/docs/getting-started/migration.md +++ b/docs/src/content/docs/getting-started/migration.md @@ -104,7 +104,9 @@ skill collection layout reference. ## Skill routing convergence :::caution[Behavior change] -Skills for **Copilot, Cursor, OpenCode, Codex, and Gemini** now deploy to `.agents/skills/` by default instead of per-client directories (`.github/skills/`, `.cursor/skills/`, `.gemini/skills/`, etc.). This matches the `.agents/` discovery path documented by all five clients and eliminates redundant copies when targeting multiple clients. +Skills for **Copilot, Cursor, OpenCode, Codex, Gemini, and Windsurf** now deploy to `.agents/skills/` by default instead of per-client directories (`.github/skills/`, `.cursor/skills/`, `.gemini/skills/`, `.windsurf/skills/`, etc.). This matches the `.agents/` discovery path documented by all six clients and eliminates redundant copies when targeting multiple clients. + +**Windsurf joined the convergence in [#1520](https://github.com/microsoft/apm/issues/1520).** Cascade [natively discovers `.agents/skills/`](https://docs.windsurf.com/windsurf/cascade/skills#skill-scopes) at both workspace and user scope, so APM no longer writes a separate `.windsurf/skills/` copy. The next `apm install` after upgrading auto-migrates any `.windsurf/skills//` recorded in `apm.lock.yaml` to `.agents/skills/` and deletes the stale copy (see below). If you never re-run `apm install`, remove the stale directories by hand with `rm -rf .windsurf/skills//`. **Claude is unchanged** - its skills continue to deploy to `.claude/skills/`. @@ -113,7 +115,7 @@ To restore the previous per-client layout, pass `--legacy-skill-paths` to any co ### Auto-migration of legacy lockfile state -When you upgrade APM and run `apm install`, the tool automatically detects legacy per-client skill paths (`.github/skills/`, `.cursor/skills/`, `.opencode/skills/`, `.gemini/skills/`) recorded in your `apm.lock.yaml` and migrates them to `.agents/skills/`. +When you upgrade APM and run `apm install`, the tool automatically detects legacy per-client skill paths (`.github/skills/`, `.cursor/skills/`, `.opencode/skills/`, `.gemini/skills/`, `.windsurf/skills/`) recorded in your `apm.lock.yaml` and migrates them to `.agents/skills/`. **What happens:** - Old per-client skill files are deleted after the new `.agents/skills/` files are written @@ -135,7 +137,7 @@ per-client skill paths to `.agents/skills/` and update `apm.lock.yaml`. In CI pipelines, this means the working tree will show: - Deletions under `.github/skills/`, `.cursor/skills/`, `.opencode/skills/`, - and/or `.gemini/skills/` + `.gemini/skills/`, and/or `.windsurf/skills/` - Additions under `.agents/skills/` - An updated `apm.lock.yaml` diff --git a/docs/src/content/docs/producer/author-primitives/skills.md b/docs/src/content/docs/producer/author-primitives/skills.md index d159e94c1..886cfe283 100644 --- a/docs/src/content/docs/producer/author-primitives/skills.md +++ b/docs/src/content/docs/producer/author-primitives/skills.md @@ -99,22 +99,23 @@ in `references/`; keep `SKILL.md` to the always-relevant flow. | Target | Deploy directory | |-------------------|----------------------------------------------| | `claude` | `.claude/skills//SKILL.md` | -| `windsurf` | `.windsurf/skills//SKILL.md` | | `kiro` | `.kiro/skills//SKILL.md` | | `copilot` | `.agents/skills//SKILL.md` | | `cursor` | `.agents/skills//SKILL.md` | | `codex` | `.agents/skills//SKILL.md` | | `gemini` | `.agents/skills//SKILL.md` | | `opencode` | `.agents/skills//SKILL.md` | +| `windsurf` | `.agents/skills//SKILL.md` | | `agent-skills` | `.agents/skills//SKILL.md` (explicit) | -Five harnesses converge on the cross-tool `.agents/skills/` +Six harnesses converge on the cross-tool `.agents/skills/` directory. Claude keeps its harness-native path because Claude Code's -default scan is `.claude/skills/`; Windsurf and Kiro currently use -`.windsurf/skills/` and `.kiro/skills/` for the same reason. Windsurf's Cascade also -[discovers `.agents/skills/`](https://docs.windsurf.com/windsurf/cascade/skills#skill-scopes) -natively for cross-agent compatibility (convergence tracked in -[#1520](https://github.com/microsoft/apm/issues/1520)). The whole +default scan is `.claude/skills/`; Kiro likewise keeps `.kiro/skills/`. +Windsurf joined the convergence in +[#1520](https://github.com/microsoft/apm/issues/1520): Cascade +[natively discovers `.agents/skills/`](https://docs.windsurf.com/windsurf/cascade/skills#skill-scopes) +at both workspace and user scope, so APM no longer ships a separate +`.windsurf/skills/` copy. The whole skill folder is copied (`shutil.copytree`), so `scripts/`, `references/`, `assets/`, and `examples/` ride along. Symlinks and the `.apm-pin` cache marker are filtered out diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 5a9b43d2a..f8f516f76 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -157,7 +157,7 @@ A plural alias `targets:` (YAML list only) is also accepted and takes precedence | `opencode` | Emits to `.opencode/agents/`, `.opencode/commands/`, `.opencode/skills/`. | | `codex` | Emits `AGENTS.md` and deploys skills to `.agents/skills/`, agents to `.codex/agents/`. | | `gemini` | Emits `GEMINI.md` and deploys to `.gemini/commands/`, `.gemini/skills/`, `.gemini/settings.json`. | -| `windsurf` | Emits `AGENTS.md` and deploys to `.windsurf/rules/`, `.windsurf/skills/`, `.windsurf/workflows/`, `.windsurf/hooks.json`. | +| `windsurf` | Emits `AGENTS.md` and deploys skills to `.agents/skills/`, plus `.windsurf/rules/`, `.windsurf/workflows/`, `.windsurf/hooks.json`. | | `kiro` | Emits `AGENTS.md` and deploys to `.kiro/steering/`, `.kiro/skills/`, `.kiro/hooks/`, `.kiro/settings/mcp.json`. | | `all` | All targets. Cannot be combined with other values in a list. | | `minimal` | `AGENTS.md` only at project root. **Auto-detected only**: this value MUST NOT be set explicitly in manifests; it is an internal fallback when no target folder is detected. | diff --git a/docs/src/content/docs/reference/targets-matrix.md b/docs/src/content/docs/reference/targets-matrix.md index 87207a1dc..966faa0c8 100644 --- a/docs/src/content/docs/reference/targets-matrix.md +++ b/docs/src/content/docs/reference/targets-matrix.md @@ -25,13 +25,13 @@ see [Primitive types](./primitive-types/). | codex | `.codex/` + `.agents/` | [ ] | [ ] | [x] | [x] | [ ] | [x] | [x] | | gemini | `.gemini/` | [ ] | [ ] | [ ] | [x] | [x] | [x] | [x] | | opencode | `.opencode/` | [ ] | [ ] | [x] | [x] | [x] | [ ] | [x] | -| windsurf | `.windsurf/` | [x] | [ ] | [ ] | [x] | [x] | [x] | [x] | +| windsurf | `.windsurf/` + `.agents/` | [x] | [ ] | [ ] | [x] | [x] | [x] | [x] | | kiro | `.kiro/` | [x] | [ ] | [ ] | [x] | [ ] | [x] | [x] | | agent-skills | `.agents/` | [ ] | [ ] | [ ] | [x] | [ ] | [ ] | [ ] | Skills deploy to `.agents/skills/` for Copilot, Cursor, OpenCode, -Gemini, and Codex by default (see [Skills convergence](#skills-convergence) -below). Claude, Windsurf, and Kiro keep target-native skill directories. +Gemini, Codex, and Windsurf by default (see [Skills convergence](#skills-convergence) +below). Claude and Kiro keep target-native skill directories. `copilot-cowork` (Microsoft 365 Copilot), `copilot-app` (GitHub Copilot desktop App), and `openclaw` (OpenClaw agent runtime) are @@ -164,11 +164,11 @@ OpenCode. Windsurf / Cascade. - **Detection.** `.windsurf/` directory. -- **Deploy directory.** `.windsurf/` at project scope; `~/.codeium/windsurf/` at user scope. +- **Deploy directory.** `.windsurf/` at project scope (`~/.codeium/windsurf/` at user scope), plus `.agents/` for skills. - **Supported primitives.** instructions, skills, commands, hooks, mcp. - **File conventions.** - instructions: `.windsurf/rules/.md` - - skills: `.windsurf/skills//SKILL.md` + - skills: `.agents/skills//SKILL.md` (Cascade natively discovers `.agents/skills/`; [#1520](https://github.com/microsoft/apm/issues/1520)) - commands: `.windsurf/workflows/.md` - hooks: `.windsurf/hooks.json` - **Agents.** Not deployed. Cascade auto-invokes any `SKILL.md` by its `description:` frontmatter, so a separate agents primitive would collide with skills on the same path. Ship personas as skills under `.apm/skills//SKILL.md` instead. diff --git a/src/apm_cli/bundle/lockfile_enrichment.py b/src/apm_cli/bundle/lockfile_enrichment.py index ff4a4cc4f..2db7f4f9b 100644 --- a/src/apm_cli/bundle/lockfile_enrichment.py +++ b/src/apm_cli/bundle/lockfile_enrichment.py @@ -16,9 +16,10 @@ # for Copilot. Cursor/opencode sources are niche; if someone publishes # skills exclusively under .cursor/, they must pack with --target cursor. # -# Windsurf converts agents -> skills (lossy: AGENTS.md format is collapsed -# into the windsurf skill envelope), so .github/agents/ maps to -# .windsurf/skills/. +# Windsurf converges skills onto the cross-tool .agents/skills/ directory +# (apm#1520), so .github/skills/ maps there. It also converts agents -> +# skills (lossy: AGENTS.md format is collapsed into the windsurf skill +# envelope), so .github/agents/ maps to .agents/skills/ as well. _CROSS_TARGET_MAPS: dict[str, dict[str, str]] = { "claude": { ".github/skills/": ".claude/skills/", @@ -45,8 +46,8 @@ ".github/agents/": ".codex/agents/", }, "windsurf": { - ".github/skills/": ".windsurf/skills/", - ".github/agents/": ".windsurf/skills/", + ".github/skills/": ".agents/skills/", + ".github/agents/": ".agents/skills/", }, "agent-skills": { ".github/skills/": ".agents/skills/", diff --git a/src/apm_cli/install/skill_path_migration.py b/src/apm_cli/install/skill_path_migration.py index c46e4a0d9..36edcf5da 100644 --- a/src/apm_cli/install/skill_path_migration.py +++ b/src/apm_cli/install/skill_path_migration.py @@ -54,7 +54,11 @@ # Legacy per-client skill prefixes that have been converged into .agents/skills/. # .claude/skills/ is excluded (Claude is not in the convergence set). # .codex/skills/ was never a legacy path (Codex always used .agents/). -_LEGACY_SKILL_PATTERN = re.compile(r"^\.(github|cursor|opencode|gemini)/skills/([^/]+)/.+$") +# .windsurf/skills/ joined the convergence in apm#1520; prior APM versions +# deployed windsurf skills there, so it is migrated like the other clients. +_LEGACY_SKILL_PATTERN = re.compile( + r"^\.(github|cursor|opencode|gemini|windsurf)/skills/([^/]+)/.+$" +) # ------------------------------------------------------------------ # Shared message templates (single source of truth — H6) diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index 3027c5c48..d7f5f8444 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -664,8 +664,16 @@ def for_scope(self, user_scope: bool = False) -> TargetProfile | None: ), # Windsurf/Cascade -- .windsurf/ is the workspace config directory. # Rules are markdown files with trigger/globs frontmatter under .windsurf/rules/. - # Skills use the standard SKILL.md format under .windsurf/skills/. - # Cascade auto-invokes them when the description frontmatter matches the + # Skills converge on the cross-tool ``.agents/skills//SKILL.md`` path + # (deploy_root=".agents"): Cascade natively discovers ``.agents/skills/`` at + # both workspace and user scope, so windsurf joins the same convergence as + # copilot/cursor/codex/gemini/opencode instead of keeping a windsurf-native + # ``.windsurf/skills/`` copy (apm#1520). Pass ``--legacy-skill-paths`` (or + # ``APM_LEGACY_SKILL_PATHS=1``) to restore the pre-convergence + # ``.windsurf/skills/`` layout; ``apply_legacy_skill_paths`` resets + # ``deploy_root`` to ``None``. + # Ref: https://docs.windsurf.com/windsurf/cascade/skills#skill-scopes + # Cascade auto-invokes a skill when its description frontmatter matches the # task -- this is the universal invocation mechanism, so windsurf does # NOT expose a separate ``agents`` primitive. Package authors who want # their content to deploy to windsurf must declare it under @@ -688,7 +696,12 @@ def for_scope(self, user_scope: bool = False) -> TargetProfile | None: "windsurf_rules", output_compare=True, ), - "skills": PrimitiveMapping("skills", "/SKILL.md", "skill_standard"), + "skills": PrimitiveMapping( + "skills", + "/SKILL.md", + "skill_standard", + deploy_root=".agents", + ), "commands": PrimitiveMapping("workflows", ".md", "windsurf_workflow"), "hooks": PrimitiveMapping("", "hooks.json", "windsurf_hooks"), }, @@ -697,6 +710,9 @@ def for_scope(self, user_scope: bool = False) -> TargetProfile | None: user_supported="partial", user_root_dir=".codeium/windsurf", unsupported_user_primitives=("instructions",), + # Skills deploy to .agents/ while rules/workflows/hooks stay under + # .windsurf/, so pack must include both roots (mirrors codex). + pack_prefixes=(".windsurf/", ".agents/"), compile_family="agents", hooks_config_display=".windsurf/hooks.json", ), diff --git a/tests/integration/test_agent_skills_target.py b/tests/integration/test_agent_skills_target.py index 4589db1fd..f55c4d007 100644 --- a/tests/integration/test_agent_skills_target.py +++ b/tests/integration/test_agent_skills_target.py @@ -811,3 +811,91 @@ def test_auto_migration_collision_skips_gracefully( out = r2.output or "" assert "Skill path migration skipped" in out or "already exist" in out assert "--legacy-skill-paths" in out + + +# --------------------------------------------------------------------------- +# Windsurf convergence (apm#1520) +# --------------------------------------------------------------------------- + + +def _make_windsurf_project(tmp_path: Path) -> Path: + """Project with ``.windsurf/`` present so the windsurf target deploys. + + windsurf has ``auto_create=False`` and ``detect_by_dir=True`` keyed off + ``.windsurf/`` (NOT ``.agents/``), so skills only deploy when the + workspace already has a ``.windsurf/`` directory -- even for the + converged ``.agents/skills/`` path. + """ + project = _make_project(tmp_path / "dst", with_github=False) + (project / ".windsurf").mkdir() + return project + + +def test_windsurf_install_deploys_to_agents_skills( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """apm#1520: ``apm install --target windsurf`` deploys skills to the + converged ``.agents/skills//SKILL.md`` path, NOT ``.windsurf/skills/``.""" + bundle = _make_plugin_bundle(tmp_path / "src", pack_target="windsurf") + project = _make_windsurf_project(tmp_path) + + monkeypatch.delenv("APM_LEGACY_SKILL_PATHS", raising=False) + result = _invoke(project, ["install", str(bundle), "--target", "windsurf"], monkeypatch) + assert result.exit_code == 0, f"output={result.output!r}" + + assert (project / ".agents/skills" / SKILL_NAME / "SKILL.md").is_file(), ( + "windsurf skills must converge on .agents/skills/" + ) + assert not (project / ".windsurf/skills" / SKILL_NAME / "SKILL.md").exists(), ( + "windsurf must no longer write the per-client .windsurf/skills/ copy" + ) + + +def test_windsurf_legacy_flag_produces_windsurf_skills( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``--legacy-skill-paths`` restores the pre-convergence ``.windsurf/skills/`` + layout for the windsurf target.""" + bundle = _make_plugin_bundle(tmp_path / "src", pack_target="windsurf") + project = _make_windsurf_project(tmp_path) + + result = _invoke( + project, + ["install", str(bundle), "--target", "windsurf", "--legacy-skill-paths"], + monkeypatch, + ) + assert result.exit_code == 0, f"output={result.output!r}" + + assert (project / ".windsurf/skills" / SKILL_NAME / "SKILL.md").is_file(), ( + "--legacy-skill-paths must restore .windsurf/skills/ for windsurf" + ) + assert not (project / ".agents/skills" / SKILL_NAME / "SKILL.md").exists(), ( + "legacy mode must not also write the converged .agents/skills/ copy" + ) + + +def test_windsurf_auto_migration_from_windsurf_skills( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A legacy ``.windsurf/skills/`` install auto-migrates to ``.agents/skills/`` + on the next converged ``apm install`` (apm#1520).""" + bundle = _make_plugin_bundle(tmp_path / "src", pack_target="windsurf") + project = _make_windsurf_project(tmp_path) + + # --- First install: legacy mode -> .windsurf/skills/ --- + monkeypatch.setenv("APM_LEGACY_SKILL_PATHS", "1") + r1 = _invoke(project, ["install", str(bundle), "--target", "windsurf"], monkeypatch) + assert r1.exit_code == 0, f"legacy install failed: {r1.output!r}" + assert (project / ".windsurf/skills" / SKILL_NAME / "SKILL.md").is_file() + + # --- Second install: converged mode -> migrate --- + monkeypatch.delenv("APM_LEGACY_SKILL_PATHS", raising=False) + r2 = _invoke(project, ["install", str(bundle), "--target", "windsurf"], monkeypatch) + assert r2.exit_code == 0, f"migration install failed: {r2.output!r}" + + assert (project / ".agents/skills" / SKILL_NAME / "SKILL.md").is_file(), ( + "converged path must exist after migration" + ) + assert not (project / ".windsurf/skills" / SKILL_NAME / "SKILL.md").exists(), ( + "legacy .windsurf/skills/ copy must be removed by the migration" + ) diff --git a/tests/unit/install/test_skill_path_migration.py b/tests/unit/install/test_skill_path_migration.py index 38f2aaf8e..79afef059 100644 --- a/tests/unit/install/test_skill_path_migration.py +++ b/tests/unit/install/test_skill_path_migration.py @@ -58,6 +58,7 @@ class TestLegacySkillPattern: ".cursor/skills/review/SKILL.md", ".opencode/skills/deep/nested/file.md", ".gemini/skills/lint/SKILL.md", + ".windsurf/skills/my-skill/SKILL.md", ], ) def test_matches_legacy_clients(self, path: str) -> None: @@ -132,15 +133,35 @@ def test_detects_multiple_clients(self, tmp_path: Path) -> None: ".cursor/skills/s1/SKILL.md", ".opencode/skills/s1/SKILL.md", ".gemini/skills/s1/SKILL.md", + ".windsurf/skills/s1/SKILL.md", ] ), } ) plans = detect_legacy_skill_deployments(lf, tmp_path) - assert len(plans) == 4 + assert len(plans) == 5 for plan in plans: assert plan.dst_path == ".agents/skills/s1/SKILL.md" + def test_detects_windsurf_legacy(self, tmp_path: Path) -> None: + """apm#1520: windsurf joined the convergence, so prior-version + .windsurf/skills/ deployments must be migrated to .agents/skills/.""" + lf = _StubLockFile( + dependencies={ + "pkg-a": _StubDep( + deployed_files=[ + ".windsurf/skills/my-skill/SKILL.md", + ".agents/skills/my-skill/SKILL.md", # new path -- ignored + ] + ), + } + ) + plans = detect_legacy_skill_deployments(lf, tmp_path) + assert len(plans) == 1 + assert plans[0].src_path == ".windsurf/skills/my-skill/SKILL.md" + assert plans[0].dst_path == ".agents/skills/my-skill/SKILL.md" + assert plans[0].dep_name == "pkg-a" + def test_ignores_claude_and_codex(self, tmp_path: Path) -> None: lf = _StubLockFile( dependencies={ diff --git a/tests/unit/integration/test_data_driven_dispatch.py b/tests/unit/integration/test_data_driven_dispatch.py index bc25c0d75..0789c4e97 100644 --- a/tests/unit/integration/test_data_driven_dispatch.py +++ b/tests/unit/integration/test_data_driven_dispatch.py @@ -310,7 +310,8 @@ def test_partition_parity_with_old_buckets(self): "agents_opencode", "agents_codex", # NOTE: windsurf no longer exposes an 'agents' primitive - # (its content deploys as skills under .windsurf/skills/). + # (its content deploys as skills under the converged + # .agents/skills/ directory; apm#1520). "commands", # was commands_claude, aliased "commands_cursor", "commands_gemini", diff --git a/tests/unit/integration/test_scope_integration.py b/tests/unit/integration/test_scope_integration.py index fbe4fb911..83d31029a 100644 --- a/tests/unit/integration/test_scope_integration.py +++ b/tests/unit/integration/test_scope_integration.py @@ -357,7 +357,8 @@ def test_project_scope_uses_windsurf_root(self): assert "instructions" in resolved.primitives assert "skills" in resolved.primitives # windsurf intentionally does not expose an 'agents' primitive: - # Cascade discovers SKILL.md uniformly under .windsurf/skills/. + # Cascade discovers SKILL.md uniformly under the converged + # .agents/skills/ directory (apm#1520). assert "agents" not in resolved.primitives def test_user_scope_uses_codeium_windsurf_root(self): diff --git a/tests/unit/integration/test_targets.py b/tests/unit/integration/test_targets.py index 34a53847c..a96573c64 100644 --- a/tests/unit/integration/test_targets.py +++ b/tests/unit/integration/test_targets.py @@ -219,6 +219,16 @@ def test_windsurf_and_github_returns_both(self): names = {t.name for t in targets} assert names == {"windsurf", "copilot"} + def test_windsurf_not_detected_when_only_agents_dir_exists(self): + """apm#1520: windsurf skills converge on .agents/, but auto-detection + must still key off .windsurf/ (NOT the shared .agents/ dir). A workspace + with only .agents/ must not auto-select windsurf -- otherwise any tool + that creates .agents/ would silently pull in windsurf deploys.""" + (self.root / ".agents").mkdir() + targets = active_targets(self.root) + # .agents/ alone matches no target root_dir -> copilot fallback only. + assert [t.name for t in targets] == ["copilot"] + # -- explicit list of targets -- def test_explicit_list_single_target(self): @@ -310,16 +320,18 @@ def test_copilot_profile_lists_root_generated_file(self): class TestDefaultSkillRouting: - """Assert that the 4 documented clients route skills to .agents/ by default.""" + """Assert that the documented clients route skills to .agents/ by default.""" def test_default_skill_routing_uses_agents_dir_for_documented_clients(self): - """copilot, cursor, opencode, codex, gemini all have deploy_root='.agents' on skills.""" + """copilot, cursor, opencode, codex, gemini, windsurf all have + deploy_root='.agents' on skills.""" expected = { "copilot": ".agents", "cursor": ".agents", "opencode": ".agents", "codex": ".agents", "gemini": ".agents", + "windsurf": ".agents", "claude": None, # not documented as .agents/-aware } for name, want_root in expected.items(): @@ -335,11 +347,12 @@ def test_legacy_skill_paths_flag_restores_per_client_routing(self): from apm_cli.integration.targets import apply_legacy_skill_paths profiles = [ - KNOWN_TARGETS[n] for n in ("copilot", "cursor", "opencode", "codex", "claude", "gemini") + KNOWN_TARGETS[n] + for n in ("copilot", "cursor", "opencode", "codex", "claude", "gemini", "windsurf") ] restored = apply_legacy_skill_paths(profiles) - # All 6 should have deploy_root=None after legacy restore + # All 7 should have deploy_root=None after legacy restore for profile in restored: skills_pm = profile.primitives.get("skills") assert skills_pm is not None, f"{profile.name} should have skills" @@ -374,6 +387,33 @@ def test_gemini_legacy_skill_paths_restores_per_client_routing(self): f"gemini: expected deploy_root=None (legacy), got {skills_pm.deploy_root!r}" ) + def test_windsurf_skill_routing_uses_agents_dir_by_default(self): + """Cascade natively discovers .agents/skills/; windsurf converges there + (apm#1520) instead of keeping a .windsurf/skills/ copy.""" + profile = KNOWN_TARGETS["windsurf"] + skills_pm = profile.primitives["skills"] + assert skills_pm.deploy_root == ".agents", ( + f"windsurf: expected deploy_root='.agents', got {skills_pm.deploy_root!r}" + ) + + def test_windsurf_legacy_skill_paths_restores_per_client_routing(self): + """With apply_legacy_skill_paths(), windsurf deploy_root is reset to None + so skills fall back to the pre-convergence .windsurf/skills/ layout.""" + from apm_cli.integration.targets import apply_legacy_skill_paths + + profiles = [KNOWN_TARGETS["windsurf"]] + restored = apply_legacy_skill_paths(profiles) + skills_pm = restored[0].primitives["skills"] + assert skills_pm.deploy_root is None, ( + f"windsurf: expected deploy_root=None (legacy), got {skills_pm.deploy_root!r}" + ) + + def test_windsurf_pack_prefixes_cover_both_roots(self): + """windsurf deploys skills to .agents/ and rules/workflows/hooks to + .windsurf/, so pack must include both roots (mirrors codex).""" + profile = KNOWN_TARGETS["windsurf"] + assert set(profile.effective_pack_prefixes) == {".windsurf/", ".agents/"} + def test_apply_legacy_does_not_mutate_known_targets(self): """apply_legacy_skill_paths must not mutate the global KNOWN_TARGETS.""" from apm_cli.integration.targets import apply_legacy_skill_paths diff --git a/tests/unit/integration/test_windsurf_uninstall_skills.py b/tests/unit/integration/test_windsurf_uninstall_skills.py index 9c235ac85..24be1a3c4 100644 --- a/tests/unit/integration/test_windsurf_uninstall_skills.py +++ b/tests/unit/integration/test_windsurf_uninstall_skills.py @@ -1,14 +1,20 @@ """Regression tests for the windsurf uninstall-cleanup bug (#1481): ``apm uninstall`` silently failed to remove deployed skill directories -under ``.windsurf/skills/``. - -The fix dropped the ``agents`` primitive from the windsurf -``TargetProfile`` so that the deploy path ``.windsurf/skills//`` -is owned exclusively by the ``skills`` primitive. These tests pin the -post-fix shape of the windsurf profile and the directory-aware cleanup -path so a future regression -- e.g. re-introducing an ``agents`` -primitive that aliases the same deploy path -- is caught here instead -of silently corrupting an end-user workspace. +that windsurf owned. + +The original fix (#1486) dropped the ``agents`` primitive from the +windsurf ``TargetProfile`` so the skills deploy path is owned +exclusively by the ``skills`` primitive -- preventing a shadowing +``agents`` bucket from skipping directory cleanup. + +apm#1520 then converged windsurf skills onto the cross-tool +``.agents/skills//SKILL.md`` path (Cascade discovers +``.agents/skills/`` natively), so these tests pin the post-convergence +shape of the windsurf profile and the directory-aware cleanup path. +A future regression -- e.g. re-introducing an ``agents`` primitive that +aliases the same deploy path, or reverting the ``.agents/`` convergence +without updating cleanup routing -- is caught here instead of silently +corrupting an end-user workspace. """ from pathlib import Path @@ -24,41 +30,47 @@ class TestWindsurfTargetProfileShape: def test_windsurf_does_not_expose_agents_primitive(self): """windsurf intentionally has no 'agents' primitive: Cascade reads SKILL.md uniformly, so a separate agents primitive would re-create - the .windsurf/skills/ path collision.""" + the skills-path collision that caused the silent uninstall bug.""" windsurf = KNOWN_TARGETS["windsurf"] assert "agents" not in windsurf.primitives, ( "windsurf must not declare an 'agents' primitive: it shares the " - "deploy path '.windsurf/skills/' with the 'skills' primitive and " - "would re-introduce the silent uninstall-cleanup bug." + "skills deploy path with the 'skills' primitive and would " + "re-introduce the silent uninstall-cleanup bug." ) def test_windsurf_skills_primitive_uses_standard_format(self): """windsurf 'skills' primitive uses the standard skill_standard - format (deployed as SKILL.md under .windsurf/skills/).""" + format, converged onto the cross-tool ``.agents/skills/`` directory + (apm#1520).""" windsurf = KNOWN_TARGETS["windsurf"] skills = windsurf.primitives["skills"] assert skills.subdir == "skills" assert skills.extension == "/SKILL.md" assert skills.format_id == "skill_standard" + assert skills.deploy_root == ".agents", ( + "windsurf skills converge on .agents/skills/ (apm#1520); " + f"got deploy_root={skills.deploy_root!r}" + ) class TestWindsurfPartitionRouting: - """partition_managed_files must route .windsurf/skills/ paths to the - cross-target 'skills' bucket -- not to a windsurf-specific agents bucket.""" + """partition_managed_files must route windsurf's converged + ``.agents/skills/`` paths to the cross-target 'skills' bucket -- not to a + windsurf-specific agents bucket.""" def test_windsurf_skill_path_routes_to_skills_bucket(self): - """The lockfile path '.windsurf/skills/' must land in the + """The converged lockfile path '.agents/skills/' must land in the 'skills' bucket so SkillIntegrator (directory-aware) handles it.""" managed = { - ".windsurf/skills/code-review", - ".windsurf/skills/grill-me", + ".agents/skills/code-review", + ".agents/skills/grill-me", } buckets = BaseIntegrator.partition_managed_files(managed) - assert ".windsurf/skills/code-review" in buckets["skills"], ( - "windsurf skill path must be in the cross-target 'skills' bucket" + assert ".agents/skills/code-review" in buckets["skills"], ( + "windsurf's converged skill path must be in the cross-target 'skills' bucket" ) - assert ".windsurf/skills/grill-me" in buckets["skills"] + assert ".agents/skills/grill-me" in buckets["skills"] def test_no_agents_windsurf_bucket_is_created(self): """The 'agents_windsurf' bucket must not exist: windsurf no longer @@ -67,28 +79,28 @@ def test_no_agents_windsurf_bucket_is_created(self): assert "agents_windsurf" not in buckets def test_windsurf_skill_path_not_routed_to_other_buckets(self): - """A windsurf skill path must NOT leak into instructions/commands/ + """A converged skill path must NOT leak into instructions/commands/ hooks buckets (which would mean the prefix trie matched the wrong primitive).""" - managed = {".windsurf/skills/my-skill"} + managed = {".agents/skills/my-skill"} buckets = BaseIntegrator.partition_managed_files(managed) for bucket_name, paths in buckets.items(): if bucket_name == "skills": continue - assert ".windsurf/skills/my-skill" not in paths, ( - f"windsurf skill path leaked into bucket '{bucket_name}'" + assert ".agents/skills/my-skill" not in paths, ( + f"converged windsurf skill path leaked into bucket '{bucket_name}'" ) class TestWindsurfSkillUninstallCleanup: - """End-to-end: SkillIntegrator.sync_integration must remove the - .windsurf/skills// directories that install deployed.""" + """End-to-end: SkillIntegrator.sync_integration must remove the converged + .agents/skills// directories that install deployed for windsurf.""" - def test_sync_removes_windsurf_skill_directories(self, tmp_path: Path): - """Regression: skill dirs under .windsurf/skills/ created at install - time must be removed when listed in managed_files.""" - skills_root = tmp_path / ".windsurf" / "skills" + def test_sync_removes_converged_skill_directories(self, tmp_path: Path): + """Regression: skill dirs under the converged .agents/skills/ created + at install time must be removed when listed in managed_files.""" + skills_root = tmp_path / ".agents" / "skills" skills_root.mkdir(parents=True) managed_a = skills_root / "code-review" @@ -99,14 +111,14 @@ def test_sync_removes_windsurf_skill_directories(self, tmp_path: Path): managed_b.mkdir() (managed_b / "SKILL.md").write_text("managed by APM\n") - # User-authored skill in the same directory must not be touched. + # User-authored / foreign skill in the same directory must not be touched. user_skill = skills_root / "my-custom" user_skill.mkdir() (user_skill / "SKILL.md").write_text("authored by user\n") managed = { - ".windsurf/skills/code-review", - ".windsurf/skills/grill-me", + ".agents/skills/code-review", + ".agents/skills/grill-me", } stats = SkillIntegrator().sync_integration(None, tmp_path, managed_files=managed) @@ -117,7 +129,7 @@ def test_sync_removes_windsurf_skill_directories(self, tmp_path: Path): # do not couple the test to its semantics. assert not managed_a.exists(), "managed skill dir 'code-review' must be removed" assert not managed_b.exists(), "managed skill dir 'grill-me' must be removed" - assert user_skill.exists(), "user-authored skill dir must be preserved" + assert user_skill.exists(), "foreign/user-authored skill dir must be preserved" assert (user_skill / "SKILL.md").read_text() == "authored by user\n" assert stats["errors"] == 0 assert stats["files_removed"] == 2 @@ -125,13 +137,13 @@ def test_sync_removes_windsurf_skill_directories(self, tmp_path: Path): def test_sync_handles_trailing_slash_in_managed_path(self, tmp_path: Path): """Lockfile entries may carry a trailing slash on directory paths; cleanup must work either way.""" - skills_root = tmp_path / ".windsurf" / "skills" + skills_root = tmp_path / ".agents" / "skills" skills_root.mkdir(parents=True) skill = skills_root / "code-review" skill.mkdir() (skill / "SKILL.md").write_text("managed") - managed = {".windsurf/skills/code-review/"} + managed = {".agents/skills/code-review/"} stats = SkillIntegrator().sync_integration(None, tmp_path, managed_files=managed) # Primary assertion: the directory is gone regardless of how the diff --git a/tests/unit/test_lockfile_enrichment.py b/tests/unit/test_lockfile_enrichment.py index 95c47e9cc..afb82f93b 100644 --- a/tests/unit/test_lockfile_enrichment.py +++ b/tests/unit/test_lockfile_enrichment.py @@ -443,32 +443,50 @@ def _lockfile_with(self, files: list[str]) -> LockFile: return lf def test_windsurf_target_includes_windsurf_prefix(self): + """``.windsurf/`` (rules/workflows/hooks, plus legacy skills) is one of + windsurf's two pack roots and must survive filtering.""" lf = self._lockfile_with( [ - ".windsurf/skills/x/SKILL.md", + ".windsurf/rules/r.md", ".unrelated/foo", ] ) result = enrich_lockfile_for_pack(lf, fmt="apm", target="windsurf") deployed = yaml.safe_load(result)["dependencies"][0]["deployed_files"] - assert ".windsurf/skills/x/SKILL.md" in deployed + assert ".windsurf/rules/r.md" in deployed + assert ".unrelated/foo" not in deployed + + def test_windsurf_target_includes_agents_skills_prefix(self): + """apm#1520: windsurf skills converge on ``.agents/skills/``, so that + prefix must be one of windsurf's pack roots (was ``.windsurf/skills/``).""" + lf = self._lockfile_with( + [ + ".agents/skills/x/SKILL.md", + ".unrelated/foo", + ] + ) + result = enrich_lockfile_for_pack(lf, fmt="apm", target="windsurf") + deployed = yaml.safe_load(result)["dependencies"][0]["deployed_files"] + assert ".agents/skills/x/SKILL.md" in deployed assert ".unrelated/foo" not in deployed def test_windsurf_cross_map_skills_from_github(self): - """``.github/skills/`` files are remapped under ``.windsurf/skills/``.""" + """``.github/skills/`` files are remapped under ``.agents/skills/`` + (apm#1520 convergence).""" lf = self._lockfile_with([".github/skills/x/SKILL.md"]) result = enrich_lockfile_for_pack(lf, fmt="apm", target="windsurf") deployed = yaml.safe_load(result)["dependencies"][0]["deployed_files"] - assert ".windsurf/skills/x/SKILL.md" in deployed + assert ".agents/skills/x/SKILL.md" in deployed def test_windsurf_cross_map_agents_collapse_to_skills(self): - """``.github/agents/`` is intentionally remapped to ``.windsurf/skills/`` - because windsurf has no native agent surface (lossy conversion). + """``.github/agents/`` is intentionally remapped to ``.agents/skills/`` + because windsurf has no native agent surface (lossy conversion), and + windsurf skills now live under ``.agents/skills/`` (apm#1520). """ lf = self._lockfile_with([".github/agents/a.md"]) result = enrich_lockfile_for_pack(lf, fmt="apm", target="windsurf") deployed = yaml.safe_load(result)["dependencies"][0]["deployed_files"] - assert ".windsurf/skills/a.md" in deployed + assert ".agents/skills/a.md" in deployed def test_target_all_includes_windsurf_files(self): """``--target all`` must include ``.windsurf/`` files (was missing pre-refactor)."""