feat(integration): converge windsurf skills onto .agents/skills/#1803
feat(integration): converge windsurf skills onto .agents/skills/#1803Aaryan-Dadu wants to merge 3 commits into
Conversation
Switch the windsurf target's `skills` deploy path from
`.windsurf/skills/<name>/SKILL.md` to the cross-tool
`.agents/skills/<name>/SKILL.md`, so windsurf joins the existing
convergence with copilot, cursor, codex, gemini, and opencode (6 clients).
Cascade natively discovers `.agents/skills/` at both workspace and user
scope, so the windsurf-native copy is no longer needed.
Uses the same `deploy_root=".agents"` extension point as the 5 already-
converged targets; `--legacy-skill-paths` / `APM_LEGACY_SKILL_PATHS=1`
still restores the pre-convergence `.windsurf/skills/` layout.
- targets.py: windsurf skills `deploy_root=".agents"`; add
`pack_prefixes=(".windsurf/", ".agents/")` so `apm pack --target windsurf`
ships both roots (mirrors codex).
- skill_path_migration.py: add `windsurf` to `_LEGACY_SKILL_PATTERN` so the
first `apm install` after upgrade migrates stale `.windsurf/skills/<pkg>/`
to `.agents/skills/` and deletes the old copy (idempotent, collision-safe).
- lockfile_enrichment.py: remap windsurf pack paths to `.agents/skills/`.
- docs: skills.md, targets-matrix.md, manifest-schema.md, migration guide,
CHANGELOG (breaking-change entry + migration note).
- tests: registry routing, migration detection, pack parity, converged-path
uninstall, plus integration coverage for the new path, the legacy flag,
auto-migration, and the .windsurf/-only detection guard.
Detection is unchanged: windsurf still auto-detects off `.windsurf/`, never
the shared `.agents/` directory.
BREAKING CHANGE: windsurf skills now deploy to `.agents/skills/<name>/SKILL.md`
instead of `.windsurf/skills/<name>/SKILL.md`. The first `apm install` after
upgrading auto-migrates lockfile-tracked `.windsurf/skills/<pkg>/` directories
to `.agents/skills/` and removes the stale copies; if you never re-run
`apm install`, delete them manually with `rm -rf .windsurf/skills/<pkg>/`.
Pass `--legacy-skill-paths` (or set `APM_LEGACY_SKILL_PATHS=1`) to keep the
old layout.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the Windsurf target to participate in skills-path convergence by deploying skills to the cross-tool .agents/skills/ directory (with legacy opt-out), and aligns packaging, migration, tests, and docs with that behavior.
Changes:
- Update the Windsurf
TargetProfileto deployskillsunder.agents/while keeping other primitives under.windsurf/, and include both roots in pack filtering. - Extend legacy skill-path auto-migration to include prior
.windsurf/skills/deployments. - Refresh unit/integration tests and documentation to reflect the new routing, detection, and migration behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/integration/targets.py |
Switch Windsurf skills to deploy_root=".agents" and include .agents/ in pack_prefixes. |
src/apm_cli/install/skill_path_migration.py |
Recognize .windsurf/skills/ as a legacy path that migrates to .agents/skills/. |
src/apm_cli/bundle/lockfile_enrichment.py |
Remap Windsurf cross-target .github/{skills,agents}/ to .agents/skills/. |
tests/unit/test_lockfile_enrichment.py |
Update pack filtering/mapping expectations for Windsurf convergence. |
tests/unit/integration/test_windsurf_uninstall_skills.py |
Validate uninstall cleanup and routing for converged .agents/skills/ paths. |
tests/unit/integration/test_targets.py |
Add detection regression and assert Windsurf default/legacy skill routing. |
tests/unit/integration/test_scope_integration.py |
Update Windsurf scope comments to reflect converged skills directory. |
tests/unit/integration/test_data_driven_dispatch.py |
Update notes to reflect converged .agents/skills/ deployment for Windsurf. |
tests/unit/install/test_skill_path_migration.py |
Add coverage for detecting Windsurf legacy skill deployments. |
tests/integration/test_agent_skills_target.py |
Add end-to-end install/legacy/migration tests for Windsurf convergence. |
docs/src/content/docs/reference/targets-matrix.md |
Document Windsurf using both .windsurf/ and .agents/ (skills) and update convergence list. |
docs/src/content/docs/reference/manifest-schema.md |
Update Windsurf deploy locations to reflect .agents/skills/. |
docs/src/content/docs/producer/author-primitives/skills.md |
Update skill deploy path table and convergence narrative to include Windsurf. |
docs/src/content/docs/getting-started/migration.md |
Document Windsurf joining convergence and auto-migration from .windsurf/skills/. |
CHANGELOG.md |
Add breaking-change entry describing Windsurf skill routing convergence and migration. |
| 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.""" |
sergio-sisternes-epam
added
the
panel-review
The converged windsurf uninstall test rewritten in the microsoft#1520 commit had a multi-line assertion string that ruff format would collapse, tripping the CI `ruff format --check` gate. No behavior change.
sergio-sisternes-epam
added
panel-review
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 1 | Clean Codex-pattern extension via deploy_root override; apply_legacy_skill_paths should use dataclasses.replace for idiomatic field preservation; no blocking issues. |
| CLI Logging Expert | 0 | 2 | 2 | Migration templates unchanged and well-formed; windsurf correctly reuses existing auto-migration flow. Two pre-existing output gaps now reachable for windsurf users; new migration test omits output assertion for the success-path message. |
| DevX UX Expert | 0 | 2 | 1 | Migration guardrails are solid; two wording gaps reduce self-service for Windsurf users hitting the in-band collision error or reading --help. |
| Supply Chain Security Expert | 0 | 2 | 1 | Path containment for deletion is correctly guarded; check_collisions reads bytes without ensure_path_within, creating a symlink info-disclosure gap; security.md not updated for 6-harness shared-namespace blast radius. |
| OSS Growth Hacker | 0 | 2 | 2 | Convergence story is technically solid; CHANGELOG buries the 6-client milestone as a parenthetical and leads with BREAKING -- swap frame so release notes celebrate the win first, break second. |
| Doc Writer | 0 | 2 | 1 | Windsurf convergence documentation is accurate and internally consistent; two pre-existing path errors (claude row in targets-matrix.md, opencode/cursor/gemini rows in manifest-schema.md) are exposed by the windsurf fix. |
| Test Coverage Expert | 0 | 1 | 1 | 3 new windsurf integration tests land correctly for deploy/legacy/migration; windsurf-specific collision regression trap is missing; multi-client legacy-flag smoke tests omit windsurf. |
| Performance Expert | 0 | 0 | 2 | One-time migration I/O is bounded and self-canceling; warm-path regex scan is sub-ms noise; no cache, transport, or parallelism regressions; ship. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Test Coverage Expert] Add test_windsurf_auto_migration_collision_skips_gracefully in tests/integration/test_agent_skills_target.py, modelled on the existing copilot collision test with pack_target='windsurf' and _make_windsurf_project. -- Evidence-backed missing test (outcome: missing, principles: portability-by-manifest). The CHANGELOG's collision-abort promise is integration-proven for copilot but not windsurf; the next refactor could silently break this user contract with no regression trap to catch it.
- [Supply Chain Security Expert] Add ensure_path_within(src_abs, project_root) and ensure_path_within(dst_abs, project_root) before the read_bytes() calls in check_collisions (skill_path_migration.py ~line 180), mirroring the execute_migration guard at line 223. -- validate_path_segments rejects '..' but not symlinks. A lockfile entry resolving to a symlink outside the project root passes the segment check and causes check_collisions to read an external file. Two-line fix; defense-in-depth parity with execute_migration is required by the security-default bias.
- [DevX UX Expert] Add APM_LEGACY_SKILL_PATHS=1 to COLLISION_HINT in skill_path_migration.py line 74; broaden the --legacy-skill-paths help text example in install.py line 1091 beyond '.cursor/skills/' to name Windsurf (and ideally all converged clients). -- CI users cannot pass CLI flags; they need the env var surfaced in the error message at the moment apm install fails mid-pipeline. A Windsurf user reading --help today sees only a Cursor-centric example. Both are one-line fixes with outsized self-service impact.
- [Doc Writer] Fix two pre-existing doc errors exposed by this PR: change '.agents/skills/(name)/SKILL.md' to '.claude/skills/(name)/SKILL.md' in the claude section of targets-matrix.md line 103; update cursor, opencode, and gemini rows in manifest-schema.md line 156 to '.agents/skills/'. -- These errors were latent before this PR; the windsurf edit to the same files makes them immediately visible and directly contradictory to the convergence story the PR is telling. In-PR or day-1 follow-up; very low effort.
- [OSS Growth Hacker] Reframe the CHANGELOG entry to lead with the 6-IDE convergence milestone and move the BREAKING migration note to a secondary block; add a one-sentence release-post hook: "6 AI IDEs, 1 skill path -- auto-migrated on next apm install." -- Leading with BREAKING trains readers to dread upgrades. The convergence milestone is the positioning story APM needs in its durable release record. Narrative cost is zero; community trust and conversion impact are meaningful.
Architecture
classDiagram
direction LR
class PrimitiveMapping {
<<ValueObject>>
+subdir str
+extension str
+format_id str
+deploy_root str
+output_compare bool
+__post_init__()
}
class TargetProfile {
<<ValueObject>>
+name str
+root_dir str
+primitives dict
+pack_prefixes tuple
+effective_pack_prefixes tuple
+deploy_path(root) Path
+for_scope(user_scope) TargetProfile
+supports(primitive) bool
}
class MigrationPlan {
<<ValueObject>>
+src_path str
+dst_path str
+dep_name str
}
class MigrationResult {
<<DataObject>>
+deleted list
+failed list
+skipped_no_file list
+updated_deps set
}
class skill_path_migration {
<<Module>>
+_LEGACY_SKILL_PATTERN Pattern
+detect_legacy_skill_deployments() list
+check_collisions() list
+execute_migration() MigrationResult
}
class lockfile_enrichment {
<<Module>>
+_CROSS_TARGET_MAPS dict
+enrich_lockfile_for_pack() str
}
class apply_legacy_skill_paths {
<<Pure>>
+apply_legacy_skill_paths() list
+should_use_legacy_skill_paths() bool
}
TargetProfile *-- PrimitiveMapping : primitives
skill_path_migration ..> MigrationPlan : produces
skill_path_migration ..> MigrationResult : returns
apply_legacy_skill_paths ..> TargetProfile : transforms via replace
apply_legacy_skill_paths ..> PrimitiveMapping : resets deploy_root
lockfile_enrichment ..> TargetProfile : reads effective_pack_prefixes
class PrimitiveMapping:::touched
class TargetProfile:::touched
class skill_path_migration:::touched
class lockfile_enrichment:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["apm install bundle --target windsurf"] --> B["targets_phase: active_targets()"]
B --> C["KNOWN_TARGETS[windsurf] TargetProfile\nroot_dir=.windsurf\nskills.deploy_root=.agents\npack_prefixes=(.windsurf/ .agents/)"]
C --> D{legacy_skill_paths?}
D -- yes --> E["apply_legacy_skill_paths()\nreset skills.deploy_root=None"]
D -- no --> F["SkillIntegrator.sync_integration()\ndeploy_root=.agents active"]
E --> F2["SkillIntegrator.sync_integration()\ndeploy_root=None uses root_dir=.windsurf"]
F --> G["write .agents/skills/name/SKILL.md"]
F2 --> G2["write .windsurf/skills/name/SKILL.md"]
G --> H{existing lockfile\nhas .windsurf/skills/ entries?}
G2 --> I2["persist apm.lock.yaml -- legacy mode"]
H -- yes --> J["detect_legacy_skill_deployments()\n_LEGACY_SKILL_PATTERN includes windsurf"]
H -- no --> K["persist apm.lock.yaml -- no migration needed"]
J --> L["check_collisions()\ncompare bytes: src vs dst"]
L -- no collisions --> N["execute_migration()\ndelete .windsurf/skills/name/\nswap lockfile entries"]
L -- collision found --> O["emit error + --legacy-skill-paths hint\nskip migration for safety"]
N --> P["persist apm.lock.yaml\nentries updated to .agents/skills/name/SKILL.md"]
O --> P
K --> R([done])
I2 --> R
P --> R
sequenceDiagram
participant U as User
participant CLI as apm install
participant TP as targets_phase
participant SI as SkillIntegrator
participant MP as skill_path_migration
participant LF as apm.lock.yaml
U->>CLI: apm install bundle --target windsurf
CLI->>TP: resolve_targets(windsurf, user_scope=False)
TP-->>CLI: TargetProfile(skills.deploy_root=.agents, pack_prefixes=(.windsurf/ .agents/))
CLI->>SI: sync_integration(target, project_root)
SI->>LF: write .agents/skills/name/SKILL.md
Note over CLI: migration phase
CLI->>MP: detect_legacy_skill_deployments(lockfile, root)
Note over MP: _LEGACY_SKILL_PATTERN scans deployed_files for .windsurf/skills/ entries
MP-->>CLI: [MigrationPlan(src=.windsurf/skills/name/SKILL.md, dst=.agents/skills/name/SKILL.md)]
CLI->>MP: check_collisions(plans, root)
MP-->>CLI: [] no collisions
CLI->>MP: execute_migration(plans, lockfile, root)
MP->>LF: unlink .windsurf/skills/name/SKILL.md
MP->>LF: rmdir empty .windsurf/skills/name/ via _cleanup_empty_parents
MP->>LF: swap deployed_files in lockfile
CLI->>LF: persist apm.lock.yaml
LF-->>U: install complete; legacy path removed
Recommendation
Merge when ready. Zero blocking findings across eight active panelists. The PR correctly extends the proven Codex deploy_root pattern to Windsurf with a clean auto-migration path and an explicit opt-out. The two highest-signal post-merge actions are: (1) add the windsurf collision integration test to close the evidence-backed portability-by-manifest gap, and (2) add the two ensure_path_within guards in check_collisions to restore defense-in-depth parity with execute_migration. Both should be tracked as follow-up issues filed before or immediately after merge. The COLLISION_HINT env-var fix and doc corrections are low-effort wins worth landing in the same release window. File a standalone issue for the enterprise/security.md blast-radius documentation before the next enterprise-targeted release post.
Full per-persona findings
Python Architect
- [nit] apply_legacy_skill_paths reconstructs PrimitiveMapping field-by-field instead of dataclasses.replace(skills_pm, deploy_root=None) at
src/apm_cli/integration/targets.py:857
In targets.py:apply_legacy_skill_paths, new_pm is built by explicitly copying subdir, extension, format_id, and hard-coding deploy_root=None, omitting output_compare. The post_init invariant means there is no correctness risk today, but any new field added to PrimitiveMapping must also be added to this manual copy, whereas dataclasses.replace(skills_pm, deploy_root=None) copies all fields automatically and makes the intent self-documenting. The function already imports dataclasses.replace for the TargetProfile replace call one line below.
Suggested: Replace the four-line PrimitiveMapping(...) construction with: new_pm = replace(skills_pm, deploy_root=None)
CLI Logging Expert
-
[recommended] test_windsurf_auto_migration_from_windsurf_skills checks disk state but never asserts the user-visible migration summary message. at
tests/integration/test_agent_skills_target.py:894
The PR adds a new integration test for the windsurf auto-migration path but r2.output is only checked for exit_code == 0. MIGRATION_SUMMARY_TEMPLATE ('Migrated {count} skill file(s) from legacy per-client paths to .agents/skills/') is the primary user signal that migration ran. If the migration phase is accidentally skipped or the _rich_info call is dropped in a later refactor, the test still passes.
Suggested: After assert r2.exit_code == 0, add: assert 'Migrated' in r2.output, f'migration summary must appear in output: {r2.output!r}' -
[recommended] local_bundle_handler.py migration block never checks _mig_result.failed; failed deletions are silently swallowed with no user warning. at
src/apm_cli/install/local_bundle_handler.py:250
pipeline.py emits _rich_warning when execute_migration returns a non-empty failed list. local_bundle_handler.py computes _mig_result but only reads .deleted and .skipped_no_file. For a windsurf user whose .windsurf/skills/(pkg)/ is read-only, the migration appears to succeed but the stale file persists silently. -
[nit] MIGRATION_SUMMARY_TEMPLATE count includes skipped_no_file, so 'Migrated N' overstates the number of actual file moves. at
src/apm_cli/install/pipeline.py:809
A user who manually deleted .windsurf/skills/my-pkg/ before running apm install would see 'Migrated 1 skill file(s)' even though nothing was moved. -
[nit] Verbose migration detail line hard-codes two leading spaces alongside symbol='info', producing uneven column offset. at
src/apm_cli/install/pipeline.py:820
STATUS_SYMBOLS convention: the [i] prefix is itself the visual indent marker. The extra spaces yield an uneven column offset inconsistent with single-space convention elsewhere.
DevX UX Expert
-
[recommended] COLLISION_HINT omits APM_LEGACY_SKILL_PATHS=1, leaving CI users without their escape hatch at
src/apm_cli/install/skill_path_migration.py:74
The runtime collision error message tells the user 'pass --legacy-skill-paths to keep per-client paths.' In CI the user cannot pass CLI flags -- they need the env var. The docs cover it but none are visible when apm install fails mid-pipeline.
Suggested: COLLISION_HINT = ("Remove the conflicting file(s) and re-runapm install, or pass --legacy-skill-paths (or set APM_LEGACY_SKILL_PATHS=1 in CI) to keep per-client paths.") -
[recommended] --legacy-skill-paths flag help text uses .cursor/skills/ as sole example, invisible to Windsurf users at
src/apm_cli/commands/install.py:1091
A Windsurf user whose skills stopped appearing in .windsurf/skills/ will runapm install --helpto find the escape hatch. The flag help reads 'Deploy skill files to per-client paths (e.g. .cursor/skills/)...' -- a Cursor-centric example. -
[nit] uninstall.md harness-folder list omits .agents/ for Windsurf skills, not updated in this PR at
docs/src/content/docs/reference/cli/uninstall.md:81
A Windsurf user post-Converge windsurf skills onto .agents/skills/ (Cascade native cross-agent discovery) #1520 whose skills now live under .agents/ may not find the confirmation they expect when reading the uninstall reference.
Supply Chain Security Expert
-
[recommended] check_collisions reads file bytes without ensure_path_within, violating defense-in-depth for the migration read path at
src/apm_cli/install/skill_path_migration.py:180
execute_migration calls ensure_path_within before every unlink/rmtree. check_collisions does not. validate_path_segments rejects '..' but NOT symlinks. A lockfile entry '.windsurf/skills/evil-dir/SKILL.md' where evil-dir is a directory symlink pointing outside the project root would pass validate_path_segments and cause check_collisions to read an external file.
Suggested: Before src_abs.read_bytes() and dst_abs.read_bytes() in check_collisions, call ensure_path_within(src_abs, project_root) and ensure_path_within(dst_abs, project_root). Pattern mirrors execute_migration line 223. -
[recommended] enterprise/security.md not updated: 6-harness shared .agents/skills/ namespace expands blast radius but the security model doc is silent on this at
docs/src/content/docs/enterprise/security.md
Pre-convergence, a compromised Windsurf-targeted package was siloed to .windsurf/skills/. Post-convergence it deploys to .agents/skills/ and is simultaneously ingested by Copilot, Cursor, Codex, Gemini, OpenCode, and Windsurf. A security model page must document this trade-off. -
[nit] No test exercises symlink-based information disclosure in check_collisions at
tests/unit/install/test_skill_path_migration.py
TestCheckCollisions has no test asserting that check_collisions is safe when src_abs or dst_abs resolves via a symlink to a path outside project_root.
OSS Growth Hacker
-
[recommended] CHANGELOG frames a convergence milestone as a breaking change; leading with BREAKING trains readers to dread upgrades at
CHANGELOG.md:12
The actual story -- 6 AI IDEs sharing one skill path, auto-migration -- is buried. '(6 clients total)' appears as a parenthetical. The conversion goal is undercut.
Suggested: Lead with the milestone: 'Windsurf joins the cross-tool .agents/skills/ convergence -- 6 AI IDEs now share one skill path. [BREAKING] APM no longer writes a separate .windsurf/skills/ copy...' -
[recommended] Migration.md caution block opens with a path-list, not a user-benefit statement at
docs/src/content/docs/getting-started/migration.md:107
A Windsurf user arriving mid-upgrade needs to see the benefit first then the mechanical path change. The auto-migration promise does not appear until sentence 3. -
[nit] Auto-migration is the key friction eliminator but is not the headline; surfacing it earlier converts caution-readers into confident upgraders
-
[nit] No standalone release-post hook sentence distilling '6 AI IDEs, 1 path' anywhere in the changed files
Doc Writer
-
[recommended] targets-matrix.md claude section lists skills path as .agents/skills/ -- should be .claude/skills/ at
docs/src/content/docs/reference/targets-matrix.md:103
Line 103 reads '.agents/skills/(name)/SKILL.md' under the claude section. The code deploys claude skills to .claude/skills/. This is a pre-existing error exposed by the windsurf edit to the same file.
Suggested: Change '.agents/skills/(name)/SKILL.md' (in the claude section) to '.claude/skills/(name)/SKILL.md' -
[recommended] manifest-schema.md shows stale per-client skills paths for cursor, opencode, and gemini at
docs/src/content/docs/reference/manifest-schema.md:156
The PR correctly updated the windsurf row but three adjacent rows remain stale: cursor says '.cursor/skills/', opencode says '.opencode/skills/', gemini says '.gemini/skills/'. The code confirms all three use deploy_root='.agents'.
Suggested: Update cursor, opencode, and gemini rows to show '.agents/skills/' consistent with the code. -
[nit] instructions-and-agents.md migration callout references historical .windsurf/skills/ without noting the new .agents/skills/ deploy destination at
docs/src/content/docs/producer/author-primitives/instructions-and-agents.md:204
A user following this callout post-Converge windsurf skills onto .agents/skills/ (Cascade native cross-agent discovery) #1520 upgrade will end up with a skill deploying to .agents/skills/, not .windsurf/skills/, with no callout documentation pointing that out.
Test Coverage Expert
-
[recommended] No windsurf-specific collision integration test: hand-edited .windsurf/skills/ content mismatch on migration is unguarded at integration tier at
tests/integration/test_agent_skills_target.py
CHANGELOG promises 'foreign/hand-authored skills are untouched; aborts with a clear error on a content collision.' The existing test_auto_migration_collision_skips_gracefully proves the collision mechanism for copilot. No test exercises the windsurf variant. Confirmed absent by grep.
Proof (missing at):tests/integration/test_agent_skills_target.py::test_windsurf_auto_migration_collision_skips_gracefully-- proves: When a user hand-edits .windsurf/skills/(name)/SKILL.md, apm install detects the mismatch and skips deletion with an actionable error [portability-by-manifest,devx] -
[nit] test_install_target_all_legacy_paths_writes_per_client and test_install_legacy_flag_via_env_var omit windsurf from the multi-client all-legacy assertion at
tests/integration/test_agent_skills_target.py:604
Both tests iterate over .github, .claude, .cursor, .opencode, .codex, .gemini but do not create a .windsurf/ sentinel or assert .windsurf/skills/ output. Windsurf is now the 7th converged client.
Proof (missing at):tests/integration/test_agent_skills_target.py::test_install_target_all_legacy_paths_writes_per_client-- proves: apm install --target all --legacy-skill-paths writes windsurf skills to .windsurf/skills/ consistent with all other 6 legacy per-client dirs [portability-by-manifest,devx]
Performance Expert
-
[nit] detect_legacy_skill_deployments scans the full lockfile dep/file graph on every apm install, not just the first one at
src/apm_cli/install/pipeline.py:783
pipeline.py calls detect_legacy_skill_deployments unconditionally when ctx.existing_lockfile is set. Post-migration the lockfile carries only .agents/skills/ paths; every path fails the regex fast. At D=20, F=20, this is 400 sub-microsecond regex fails per install -- genuine noise today, but a lockfile migration-version sentinel would collapse warm-path cost from O(D*F) to O(1). -
[nit] check_collisions reads full file bytes without a file-size short-circuit at
src/apm_cli/install/skill_path_migration.py:181
SKILL.md files are small (<10KB). The concern is pattern: if a future primitive ships larger blobs, same code would read MB-scale files on migration. A size check before read_bytes() costs one stat() per candidate.
Auth Expert -- inactive
No auth surface touched: changed files (src/apm_cli/bundle/lockfile_enrichment.py, src/apm_cli/install/skill_path_migration.py, src/apm_cli/integration/targets.py, CHANGELOG.md, docs/, tests/) are purely about workspace filesystem path migration for Windsurf skill deployment and do not affect core/auth.py, token_manager.py, credential resolution, host classification, token precedence, HTTP authorization headers, or remote-host fallback semantics.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Generated by PR Review Panel for issue #1803 · sonnet46 21.1M · ◷
github-actions
Bot
removed
the
panel-review
|
@danielmeppiel |
|
@Aaryan-Dadu thanks a lot for this, I'll need to close as duplicate of #1802 which landed a bit before |
Description
Switch the windsurf target's
skillsdeploy path from.windsurf/skills/<name>/SKILL.mdto the cross-tool.agents/skills/<name>/SKILL.md, so windsurf joins the existing convergence with copilot, cursor, codex, gemini, and opencode (6 clients). Cascade natively discovers.agents/skills/at both workspace and user scope, so the windsurf-native copy is no longer needed.Uses the same
deploy_root=".agents"extension point as the 5 already-converged targets;--legacy-skill-paths/APM_LEGACY_SKILL_PATHS=1still restores the pre-convergence.windsurf/skills/layout.deploy_root=".agents"; addpack_prefixes=(".windsurf/", ".agents/")soapm pack --target windsurfships both roots (mirrors codex).windsurfto_LEGACY_SKILL_PATTERNso the firstapm installafter upgrade migrates stale.windsurf/skills/<pkg>/to.agents/skills/and deletes the old copy (idempotent, collision-safe)..agents/skills/..windsurf/-only detection guard.Detection is unchanged: windsurf still auto-detects off
.windsurf/, never the shared.agents/directory.BREAKING CHANGE: windsurf skills now deploy to
.agents/skills/<name>/SKILL.mdinstead of.windsurf/skills/<name>/SKILL.md. The firstapm installafter upgrading auto-migrates lockfile-tracked.windsurf/skills/<pkg>/directories to.agents/skills/and removes the stale copies; if you never re-runapm install, delete them manually withrm -rf .windsurf/skills/<pkg>/. Pass--legacy-skill-paths(or setAPM_LEGACY_SKILL_PATHS=1) to keep the old layout.Fixes #1520
Type of change
Testing
Spec conformance (OpenAPM v0.1)
If this PR changes behaviour that an OpenAPM v0.1
req-XXXcovers,confirm the three-step ritual (see CONTRIBUTING.md "Adding or
changing a normative requirement"):
docs/src/content/docs/specs/openapm-v0.1.mdupdated(new/changed
<a id="req-XXX"></a>anchor + prose + Appendix Crow).
docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.ymlupdated.
@pytest.mark.req("req-XXX")test undertests/spec_conformance/added or extended.CONFORMANCE.{md,json}regenerated viauv run --extra dev python -m tests.spec_conformance.gen_statementand committed.