Skip to content

feat(cowork): experimental support for Microsoft 365 Copilot Cowork custom skills#926

Merged
sergio-sisternes-epam merged 10 commits intomainfrom
feat/cowork-skills
Apr 27, 2026
Merged

feat(cowork): experimental support for Microsoft 365 Copilot Cowork custom skills#926
sergio-sisternes-epam merged 10 commits intomainfrom
feat/cowork-skills

Conversation

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator

Description

Adds a first-class, experimentally-gated cowork target so APM users on the Microsoft 365 Copilot Frontier preview can deploy their skills to Cowork (Microsoft docs) with a single apm install --target cowork --global. Today they copy SKILL.md files into OneDrive by hand.

Behind apm experimental enable cowork (off by default). User scope only; project-scope rejected. Skills only; non-skill primitives skipped with one summary warning. Path-safety guards (validate_path_segments + ensure_path_within) on the OneDrive resolver and all cowork:// lockfile path I/O. Caps (50 skills / 1 MiB SKILL.md) are warn-only.

OneDrive resolution order: APM_COWORK_SKILLS_DIR env var > apm config cowork-skills-dir > macOS ~/Library/CloudStorage/OneDrive* glob > Windows %ONEDRIVECOMMERCIAL% then %ONEDRIVE% > none. Linux is env/config only by design (no auto-detect).

Also adds apm config set/get/unset cowork-skills-dir for persistent path overrides — set gated on the cowork flag; get and unset always available as a safety valve.

This is APM's first target outside the IDE — one manifest now governs both your coding agent and your enterprise AI assistant. Enable the flag, try it, and tell us what breaks.

Reviewed by the APM Expert Review Panel (Python Architect, CLI Logging, DevX UX, Supply Chain Security, OSS Growth Hacker, CEO arbitration). Verdict: REQUEST-CHANGES with 7 required fixes — all landed in this PR. 10 lower-severity items deferred per CEO ruling; security follow-ups are hard gates on promoting the flag out of experimental.

Fixes #913

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Full unit suite: 5517 pass / 0 fail (+141 new tests vs upstream/main baseline). Targeted suites for cowork paths, target gating, install phases, and config command all green. Docs build (cd docs && npm run build) green.

Copilot AI review requested due to automatic review settings April 24, 2026 23:27
@sergio-sisternes-epam sergio-sisternes-epam added the panel-review Trigger the apm-review-panel gh-aw workflow label Apr 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an experimentally-gated cowork install target so APM can deploy SKILL.md files into the Microsoft 365 Copilot Cowork OneDrive folder, including config/CLI support for persisting an override path, lockfile URI handling, and docs/tests.

Changes:

  • Introduces a new cowork TargetProfile with dynamic user-scope deploy root resolution and experimental flag gating.
  • Adds Cowork OneDrive path resolution + cowork://skills/... lockfile encoding/decoding and wires it into integration + uninstall flows.
  • Adds apm config set/get/unset cowork-skills-dir, extensive unit coverage, and new Cowork integration documentation + changelog entry.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/apm_cli/integration/cowork_paths.py Implements OneDrive skills dir resolution and lockfile path translation for cowork://.
src/apm_cli/integration/targets.py Adds cowork target, dynamic-root support, and flag-gated target selection.
src/apm_cli/integration/base_integrator.py Extends path validation/partitioning/removal to support cowork:// managed entries.
src/apm_cli/integration/skill_integrator.py Routes skill deployment to resolved_deploy_root for dynamic-root targets.
src/apm_cli/core/experimental.py Registers the cowork experimental flag and hint text.
src/apm_cli/config.py Adds persistent config helpers for cowork_skills_dir and unsetting config keys.
src/apm_cli/commands/config.py Adds CLI UX for cowork-skills-dir and config unset.
src/apm_cli/install/phases/targets.py Handles Cowork resolution errors, explicit-target gating, and project-scope rejection.
src/apm_cli/install/phases/integrate.py Adds warn-only Cowork caps checks (skill count and SKILL.md size).
src/apm_cli/install/services.py Records out-of-tree deployed files via cowork://... and emits once-per-run cowork warnings.
src/apm_cli/install/template.py Plumbs install context into integration services for cowork warnings.
src/apm_cli/install/context.py Adds cowork once-per-run warning guard state.
tests/unit/integration/test_cowork_paths.py Tests resolution precedence + path-safety and lockfile translation.
tests/unit/integration/test_cowork_target.py Tests cowork flag gating, dynamic-root semantics, and targets phase behaviors.
tests/unit/integration/test_base_integrator.py Tests cowork path validation, managed file partitioning, and removal behavior.
tests/unit/integration/test_skill_integrator.py Tests cowork deployment routing and sub-skill promotion behavior.
tests/unit/install/phases/test_targets_phase.py Tests project-scope gate and error handling for cowork resolution failures.
tests/unit/install/phases/test_integrate_phase.py Tests warn-only cap checks for cowork installs.
tests/unit/install/test_services.py Tests lockfile deployed-path encoding and once-per-run cowork warning emission.
tests/unit/test_config_command.py Adds storage + CLI tests for cowork-skills-dir and config unsetting.
tests/unit/core/test_experimental.py Verifies cowork flag registration, defaults, and hint URL shape.
tests/unit/core/test_scope.py Updates expected KNOWN_TARGETS set to include cowork.
docs/src/content/docs/integrations/cowork.md New Cowork integration page (setup, resolution, install behavior, caps, lockfile).
docs/src/content/docs/reference/experimental.md Documents the new cowork experimental flag.
docs/src/content/docs/reference/cli-commands.md Updates CLI reference for --target cowork and cowork-skills-dir config.
docs/src/content/docs/integrations/ide-tool-integration.md Mentions Cowork as a user-scope deployment target (experimental).
docs/astro.config.mjs Adds Cowork page to docs sidebar navigation.
packages/apm-guide/.apm/skills/apm-usage/commands.md Updates the embedded CLI reference skill content for new cowork features.
CHANGELOG.md Adds an Unreleased entry for the experimental Cowork target/config.
tests/unit/install/phases/__init__.py Added/updated as part of the new phase test suite.

Comment thread src/apm_cli/integration/cowork_paths.py Outdated
Comment thread src/apm_cli/install/services.py Outdated
Comment thread src/apm_cli/install/phases/targets.py Outdated
Comment thread src/apm_cli/integration/targets.py Outdated
Comment thread src/apm_cli/install/phases/integrate.py Outdated
@github-actions
Copy link
Copy Markdown

APM Review Panel Verdict

Disposition: REQUEST_CHANGES (one required pre-merge fix; all other findings are minor or follow-up)


Per-persona findings

Python Architect:

This PR introduces APM's first dynamic-root deployment target. The architecture is sound and the implementation is tidy. Key design choices reviewed below.

1. OO / class diagram

classDiagram
    direction LR
    class TargetProfile {
        <<ValueObject / DataClass>>
        +name str
        +root_dir str
        +primitives Dict
        +requires_flag Optional[str]
        +resolved_deploy_root Optional[Path]
        +user_root_resolver Optional[Callable]
        +for_scope(user_scope) TargetProfile
        +deploy_path(project_root) Path
    }
    class PrimitiveMapping {
        <<ValueObject>>
        +subdir str
        +extension str
        +format_id str
        +deploy_root Optional[str]
    }
    class CoworkPaths {
        <<IOBoundary / PureModule>>
        +resolve_cowork_skills_dir() Path
        +to_lockfile_path(abs, root) str
        +from_lockfile_path(lp, root) Path
        +is_cowork_path(lp) bool
    }
    class CoworkResolutionError {
        <<Exception>>
    }
    class BaseIntegrator {
        <<AbstractBase>>
        +validate_deploy_path(rel, root, targets) bool
        +sync_remove_files(root, mf, prefix) dict
        +partition_managed_files(mf, targets) dict
    }
    class SkillIntegrator {
        <<ConcreteIntegrator>>
        +integrate_package_skills(info, root, ...) IntegrationResult
    }
    class KNOWN_TARGETS {
        <<Registry>>
        copilot, claude, cursor, codex, cowork
    }

    TargetProfile *-- PrimitiveMapping : contains
    TargetProfile ..> CoworkPaths : resolver delegates
    CoworkPaths ..> CoworkResolutionError : raises
    BaseIntegrator ..> CoworkPaths : calls validate+translate
    SkillIntegrator --|> BaseIntegrator
    KNOWN_TARGETS o-- TargetProfile : entries

    class TargetProfile:::touched
    class CoworkPaths:::touched
    class CoworkResolutionError:::touched
    class BaseIntegrator:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading

2. Execution flow diagram

flowchart TD
    A["apm install --target cowork --global"] --> B["phases/targets.py: run(ctx)"]
    B --> C["resolve_targets(home, user_scope=True, explicit='cowork')"]
    C --> D["active_targets_user_scope('cowork')"]
    D --> E{_flag_gated cowork?}
    E -- "No" --> F["[FS] phases/targets.py: error + SystemExit(1)"]
    E -- "Yes" --> G["for_scope(user_scope=True)"]
    G --> H["[I/O] cowork_paths.resolve_cowork_skills_dir()"]
    H -- "env APM_COWORK_SKILLS_DIR" --> I["[FS] validate_path_segments + expanduser"]
    H -- "config cowork_skills_dir" --> J["[I/O] get_cowork_skills_dir() via config cache"]
    H -- "macOS auto-detect" --> K["[FS] ~/Library/CloudStorage/OneDrive* glob"]
    H -- "Windows auto-detect" --> L["[FS] ONEDRIVECOMMERCIAL / ONEDRIVE env"]
    H -- "None (unavailable)" --> M["phases/targets.py: error + SystemExit(1)"]
    I & J & K & L --> N["TargetProfile with resolved_deploy_root set"]
    N --> O["phases/integrate.py: run(ctx) -- per-package loop"]
    O --> P["services.integrate_package_primitives(...)"]
    P --> Q{cowork active + non-skill primitives?}
    Q -- "Yes, first time" --> R["[LOG] warn once via ctx.logger.warning"]
    Q --> S["SkillIntegrator._integrate_native_skill / _promote_sub_skills"]
    S --> T["[FS] target.resolved_deploy_root / skill_name / SKILL.md"]
    T --> U["services._deployed_path_entry(target_path, project_root, targets)"]
    U --> V["cowork_paths.to_lockfile_path(abs, cowork_root)"]
    V --> W{"ensure_path_within OK?"}
    W -- "Yes" --> X["[LOCK] lockfile entry: cowork://skills/name/SKILL.md"]
    W -- "No (PathTraversalError)" --> Y["except Exception: pass -- [BUG] absolute path fallback"]
    X --> Z["integrate.py: _check_cowork_caps warn-only"]
Loading

3. Design patterns

Design patterns

  • Used in this PR: Strategy -- user_root_resolver: Optional[Callable] on TargetProfile injects the OneDrive resolver at definition time, called lazily at for_scope() time; each target can provide its own resolver without subclassing.
  • Used in this PR: Value Object (frozen dataclass) -- TargetProfile is immutable; for_scope() produces a new copy with resolved_deploy_root populated, preserving immutability across the scope-resolution boundary.
  • Used in this PR: Registry -- KNOWN_TARGETS: Dict[str, TargetProfile] is the single authority for target discovery; adding a new target requires exactly one dict entry.
  • Pragmatic suggestion: hoist resolve_cowork_skills_dir() out of the per-path loops in BaseIntegrator.sync_remove_files and validate_deploy_path callers and pass the resolved root as an argument. This avoids repeated env-var + potential filesystem-glob calls per path. Not urgent for the experimental stage but the right shape once the feature graduates.

Issues:

  • Required (src/apm_cli/install/services.py:62-66): _deployed_path_entry catches Exception broadly in the to_lockfile_path call and silently falls back to storing target_path.as_posix() (an absolute path) in the lockfile. Absolute lockfile paths are non-portable: uninstall on any other machine or after moving OneDrive will fail silently to find the file. The comment "should not happen in practice" is the exact condition under which silent failures become hard bugs. Fix: narrow the catch to (PathTraversalError, ValueError) and re-raise everything else, or at minimum raise RuntimeError in the fallback branch.
  • Minor (src/apm_cli/integration/base_integrator.py:410-425): sync_remove_files calls resolve_cowork_skills_dir() inside the per-path loop. Config cache absorbs the config-read cost, but the macOS glob runs again on every cowork path when neither env var nor config is set. Hoist the resolution before the loop (follow-up, not a blocker for experimental).

CLI Logging Expert:

All user-facing cowork messages route correctly through the CommandLogger / DiagnosticCollector stack. No direct _rich_* calls appear in command code for cowork paths.

  • CoworkResolutionError docstring explicitly instructs callers to render via CommandLogger.error() -- correct pattern.
  • "cowork-skills-dir requires the cowork experimental flag. Run: apm experimental enable cowork" -- actionable, includes the fix.
  • "Cowork: no OneDrive path detected. Set APM_COWORK_SKILLS_DIR or run: apm config set cowork-skills-dir <path>" -- actionable, names two recovery paths.
  • cowork_nonsupported_warned: bool = False on InstallContext is the correct "warn once" implementation for the non-skill primitive warning.
  • Caps warnings route through both ctx.logger.warning and diagnostics.warn -- correct dual-path for verbose and summary display.
  • [!] symbol used for all cowork warnings -- consistent with STATUS_SYMBOLS convention.

No issues found.


DevX UX Expert:

The command surface is clean and follows established npm/pip/cargo conventions.

  • apm install --target cowork --global -- two-flag combo is discoverable; --target is already established, --global is already established.
  • apm experimental enable cowork -- consistent with existing experimental subcommand shape.
  • apm config set cowork-skills-dir <path> -- consistent with apm config set auto-integrate.
  • apm config get cowork-skills-dir / apm config unset cowork-skills-dir available without flag as "safety valve" -- the asymmetry (set requires flag, get/unset do not) is documented in the PR and in the docs; it is defensible as a recovery escape hatch.
  • cli-commands.md updated in the same PR (+28 lines) -- satisfies Rule 4.
  • packages/apm-guide/.apm/skills/apm-usage/commands.md updated (+8/-3) -- satisfies Rule 4.
  • The cowork.md doc is well-structured: caution admonition at top, resolution order as a table, per-platform matrix, copy-paste examples.
  • Minor: The error message when resolve_cowork_skills_dir() returns None for an explicitly-requested cowork target ("Cowork: no OneDrive path detected.") does not distinguish Linux from macOS/Windows. Linux users get the same message as macOS users who simply have no OneDrive installed. A Linux-specific hint ("Linux has no auto-detection; set APM_COWORK_SKILLS_DIR or apm config set cowork-skills-dir") would reduce confusion. Low priority for experimental stage.

Supply Chain Security Expert:

This PR writes user files to a path outside the project tree (OneDrive). The key threat surfaces reviewed:

  1. Path traversal: validate_path_segments applied to APM_COWORK_SKILLS_DIR, cowork_skills_dir config, and Windows env vars before path construction. ensure_path_within applied in both to_lockfile_path and from_lockfile_path. Double guard. ✅
  2. Lockfile integrity: cowork://skills/<rel> encoding with pre-parse validate_path_segments in from_lockfile_path before any construction. ✅
  3. Flag bypass check: requires_flag + _flag_gated in active_targets_user_scope correctly prevents cowork from appearing in auto-detected or --target all --global runs when the flag is off. Project-scope --target cowork is rejected before any filesystem activity. ✅
  4. Fail-closed validation: validate_deploy_path returns False on any exception for cowork:// paths. Correct. ✅
  5. sync_remove_files skip when OneDrive absent: when resolve_cowork_skills_dir() returns None, cowork paths are skipped (not deleted). This is the correct behavior (do not delete files you cannot locate), but it means lockfile entries for cowork paths persist as orphans if OneDrive becomes unavailable between install and uninstall. Not a security issue; the orphaned lockfile entries are inert. ✅ (data-retention follow-up, see optional items)
  6. _deployed_path_entry silent fallback: if to_lockfile_path raises, the bare except Exception: pass stores an absolute path. Absolute paths are not a security issue per se, but they weaken the lockfile integrity guarantee (non-portable). This overlaps with the Python Architect required action above.

No new credential leakage surfaces, no token over-scope, no install-time code execution.


Auth Expert: Not activated -- the PR touches no file in the fast-path trigger list and changes no authentication behavior, token management, credential resolution, or AuthResolver semantics. OneDrive env var reads (ONEDRIVECOMMERCIAL, ONEDRIVE) are filesystem path lookups, not credential management.


OSS Growth Hacker:

This PR has a strong growth angle that deserves to be called out explicitly in the release narrative.

  • Hook: "One manifest now governs both your coding agent and your enterprise AI assistant" -- this is the one-line claim worth reposting. It positions APM as ecosystem-neutral rather than IDE-specific.
  • Proof: Three commands to ship (apm experimental enable cowork && apm install --target cowork --global) -- low friction, copy-pasteable.
  • Friction reduction: Replaces manual OneDrive file copying. The "Today they copy SKILL.md files into OneDrive by hand" framing in the PR body is exactly the story angle for launch content.
  • Story fit: First non-IDE target extends the "package manager for AI-native development" narrative to enterprise AI assistants. Strong.
  • Compounding: The user_root_resolver + resolved_deploy_root pattern is now in the architecture; future non-IDE targets (Slack skills, GitHub Actions, etc.) can reuse it without new infrastructure.

Side-channel to CEO: This feature is directly aligned with the Lorenzo Storelli enterprise AI Controls prototype (github/agents/discussions/637). When the cowork flag graduates from experimental, the release narrative should connect explicitly to the "APM as the governance substrate for enterprise AI" positioning. The Growth Hacker recommends a dedicated launch post at that milestone, not just a CHANGELOG line.

  • CHANGELOG entry: Correct format (one line, PR number linked). Appropriately terse for an experimental feature.

CEO arbitration

The panel is in agreement: the implementation is architecturally sound, security-conscious, and well-tested (141 new tests, 5517 passing). The strategic call to ship this behind apm experimental enable cowork is exactly right -- it opens the M365 Copilot ecosystem without committing to a stable protocol before the Frontier preview has hardened.

One required fix before merge: _deployed_path_entry in src/apm_cli/install/services.py must not silently fall back to storing an absolute path when to_lockfile_path raises. The experimental gate limits blast radius today, but this is a lockfile integrity guarantee that must hold as the feature matures. The fix is one line: narrow the except clause to the expected exception types and let anything else propagate (or raise explicitly in the fallback branch). All other findings are either minor style items or appropriate follow-ups for when the flag graduates from experimental.

The Growth Hacker's side-channel is noted: when cowork exits experimental, the release narrative should frame this as APM's first step into enterprise AI governance, not just a convenience feature.


Required actions before merge

  1. src/apm_cli/install/services.py:62-66 -- narrow or replace the bare except Exception: pass in _deployed_path_entry. The current code silently stores target_path.as_posix() (an absolute path) in the lockfile when to_lockfile_path raises. Replace with:
    # Option A: narrow to expected exceptions, let others propagate
    except (PathTraversalError, ValueError):
        pass
    # Then: remove the "Last resort" fallback or raise instead of returning
    raise RuntimeError(
        f"Cannot translate {target_path!r} to a lockfile path. "
        f"This is a bug -- please report it."
    )
    # Option B: simpler -- just remove the inner try/except and let
    # to_lockfile_path's PathTraversalError propagate to the caller
    The "should not happen in practice" comment makes this a low-probability event, but silent lockfile corruption is disproportionately hard to debug. One of the two options above is required.

Optional follow-ups

  • Hoist resolve_cowork_skills_dir() out of the per-path loop in BaseIntegrator.sync_remove_files -- resolve once before the loop, pass the result to the translation. Eliminates repeated env-var + potential macOS glob calls when processing large managed-files sets. (Low urgency for experimental; important before GA.)
  • Add a Linux-specific hint to the "no OneDrive path detected" error message in phases/targets.py to distinguish Linux (no auto-detection by design) from macOS/Windows (OneDrive simply not installed).
  • When sync_remove_files finds cowork lockfile entries but resolve_cowork_skills_dir() returns None, emit a one-time [!] diagnostic noting that cowork files could not be cleaned up because OneDrive is unavailable. Currently they are silently skipped, leaving orphaned lockfile entries with no user-visible signal.
  • When the cowork flag graduates from experimental, connect the release narrative to the enterprise AI Controls positioning -- see OSS Growth Hacker side-channel above.

Generated by PR Review Panel for issue #926 · ● 1.6M ·

@sergio-sisternes-epam sergio-sisternes-epam force-pushed the feat/cowork-skills branch 2 times, most recently from 1402090 to 45913a2 Compare April 24, 2026 23:47
sergio-sisternes-epam pushed a commit that referenced this pull request Apr 25, 2026
The cowork target was registered in the integration layer and gated
behind an experimental flag, but TargetParamType.convert rejected it
at parse time because it was missing from VALID_TARGET_VALUES.
This made the runtime enable-hint in phases/targets.py unreachable.

Add EXPERIMENTAL_TARGETS as a separate parser-layer set so the parser
accepts the token while runtime gating in _flag_gated() and
phases/targets.py continues to enforce the flag. parse_target_arg("all")
expansion is unchanged.

Refs #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam pushed a commit that referenced this pull request Apr 25, 2026
Adds parser-layer cases for EXPERIMENTAL_TARGETS and a CliRunner E2E
test that exercises 'apm install --target cowork --global' through the
real click parser. The latter would have caught the bug fixed in 2f96dd5.

Refs #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

Update — parser-layer fix for --target cowork

While doing a manual smoke test on this branch I discovered that apm install --target cowork --global was being rejected at click parse time with 'cowork' is not a valid target. Root cause: cowork was registered in the integration layer (KNOWN_TARGETS) and the experimental flag layer (FLAGS), but missing from VALID_TARGET_VALUES in src/apm_cli/core/target_detection.py. Net effect: the runtime enable-hint in phases/targets.py:71-93 was unreachable code, and the cowork target was end-to-end unusable as shipped.

What changed

Two commits stacked on the PR:

  • 2f96dd5 fix(cli): accept cowork target at parser layer via EXPERIMENTAL_TARGETS
    • New EXPERIMENTAL_TARGETS = frozenset({"cowork"}) constant
    • Unioned into VALID_TARGET_VALUES
    • ALL_CANONICAL_TARGETS and parse_target_arg("all") deliberately unchanged — install-layer --target all semantics (which include cowork when the flag is on, per the existing test_all_user_scope_includes_cowork_when_flag_on_resolver_succeeds test) are preserved
  • 3129a4d test(cli): regression tests for cowork parser acceptance
    • 8 parser-layer cases in tests/unit/core/test_target_detection.py
    • 3 CliRunner E2E tests in tests/unit/install/test_install_target_cowork_e2e.py (this is what would have caught the original bug — existing cowork tests bypass the click parser)

Verification

  • flag OFF + --target cowork --global → reaches phases/targets.py enable-hint, exits 0 with [i] The 'cowork' target requires an experimental flag. Run: apm experimental enable cowork
  • flag ON + --target cowork --global → reaches OneDrive resolver, fails cleanly with [x] Cowork: no OneDrive path detected. when APM_COWORK_SKILLS_DIR is unset
  • flag ON + project scope (no --global) → cowork-requires-global error preserved
  • Full unit suite: 5592 passed (the pre-existing tests/unit/test_audit_report.py collection failure is unrelated to this change)

No CHANGELOG entry added — this fixes a feature still in the same Unreleased PR; the existing cowork "Added" line covers it.

Separate finding (out of scope, flagging here)

While tracing the --target all resolution path, I noticed active_targets("all") in project scope at src/apm_cli/integration/targets.py:554-556 returns KNOWN_TARGETS.values() without running _flag_gated(). Net effect: apm install --target all (no --global) hits the cowork-requires-global error path in phases/targets.py:99-108 even when the user did not explicitly ask for cowork. This is independent of the parser fix and pre-dates this PR's wiring; happy to open a follow-up issue if you confirm it's worth tracking under #933.

sergio-sisternes-epam pushed a commit that referenced this pull request Apr 25, 2026
…reproofing

Rename the experimental cowork feature to copilot-cowork to leave room
for future variants (e.g. claude-cowork). Pure rename - no behaviour
change. PR #926 is unmerged so no backward-compat shims are needed.

User-facing surface:
- Experimental flag:  cowork              -> copilot-cowork
- Target name:        cowork              -> copilot-cowork
- Env var:            APM_COWORK_SKILLS_DIR -> APM_COPILOT_COWORK_SKILLS_DIR
- Config key (CLI):   cowork-skills-dir   -> copilot-cowork-skills-dir
- Config key (store): cowork_skills_dir   -> copilot_cowork_skills_dir

Internal symbols (also renamed for symmetry with future claude-cowork):
- src/apm_cli/integration/cowork_paths.py
    -> src/apm_cli/integration/copilot_cowork_paths.py
- resolve_cowork_skills_dir   -> resolve_copilot_cowork_skills_dir
- _resolve_cowork_root        -> _resolve_copilot_cowork_root
- _COWORK_SKILLS_SUBDIR       -> _COPILOT_COWORK_SKILLS_SUBDIR
- get/set/unset_cowork_skills_dir
    -> get/set/unset_copilot_cowork_skills_dir
- 3 test files renamed via git mv

Preserved (concept-level / shared across all future cowork variants):
- CoworkResolutionError exception class
- Lockfile prefix and URI scheme constants
- Cap helpers and warn flags

Docs:
- docs/src/content/docs/integrations/cowork.md
    -> docs/src/content/docs/integrations/copilot-cowork.md
- Sidebar slug, cross-references, CLI examples, env var, and the
  apm-usage skill (packages/apm-guide) all updated.

Validation: 5592/5592 unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

Rename: cowork -> copilot-cowork (futureproofing)

Pushed 0c77ac0 to rename the experimental flag and target from cowork to copilot-cowork so the namespace can host sibling variants (e.g. a future claude-cowork). Since this PR is unmerged, the rename is clean — no aliases, no deprecation warning, no migration shim.

User-facing surface:

Surface Before After
Experimental flag cowork copilot-cowork
Target --target cowork --target copilot-cowork
Env var APM_COWORK_SKILLS_DIR APM_COPILOT_COWORK_SKILLS_DIR
Config key (CLI) cowork-skills-dir copilot-cowork-skills-dir
Config key (storage) cowork_skills_dir copilot_cowork_skills_dir

Internal module/symbols also renamed for symmetry (copilot_cowork_paths.py, resolve_copilot_cowork_skills_dir, etc.). CoworkResolutionError and lockfile prefix/URI constants are kept generic — they describe shared behaviour that any future cowork variant would reuse.

Docs (integrations page, references, sidebar slug, apm-usage skill) updated to match.

Validation: full unit suite green (5592/5592). Branch is now 3 commits ahead of 45913a2.

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

Pre-merge polish: applied all panel follow-ups

Pushed 880bc0a addressing the remaining "optional but nice" findings from the APM Review Panel verdict and the Copilot inline reviews. Maintainer chose to land them now to minimise tech debt at GA, rather than deferring to #933.

Findings addressed in this commit

ID Type File Fix
P2 perf integration/base_integrator.py Hoisted resolve_copilot_cowork_skills_dir() out of the per-path loop in sync_remove_files. Lazy on first copilot-cowork:// path, cached for the rest. Zero cost when no cowork paths present.
P3 UX install/phases/targets.py Linux-specific error wording when the resolver returns None. Linux has no OneDrive auto-detection by design; the prior message implied detection had failed.
P4 visibility integration/base_integrator.py One-time [!] warning when copilot-cowork:// lockfile entries are present but the resolver returns None. Previously these orphans were silently skipped, leaving stale apm.lock entries with no user signal. Adds optional logger= kwarg with a _rich_warning fallback so no caller change is required.

Already addressed in earlier commits (re-verified clean)

ID Status
P1 REQUIRED — narrow except Exception: pass in _deployed_path_entry Fixed: now except ValueError: + RuntimeError fallback
C2 — duplicate "commands"/"prompts" entries in non-skill warning map Fixed: commands removed, comment explains why
C4 — unused _Path import in targets.deploy_path() Fixed during rename
C5 — unused _Path import in _check_cowork_caps() Fixed during rename

Validation

  • 13 new unit tests added (3 for P2, 3 for P3, 5 for P4, plus harnessing)
  • Targeted runs: 178 passed (base_integrator, copilot_cowork_paths, copilot_cowork_target, targets_phase, install_target_copilot_cowork_e2e)
  • Full unit suite: 5603 passed in 11.65s

Out of scope

PR is ready for re-review.

@danielmeppiel
Copy link
Copy Markdown
Collaborator

New target mandatory test requirement:

report back with a proof of integration test ran locally across all APM commands and functionalities touching the target, including:

  • install for all package types, including transitive dep
  • install for all primitive types, with a clear reference of primitives supported by the target, link to live documentation URL (e.g. instructions, subagents, skills, hooks, MCP)
  • marketplace additions
  • uninstall for all previous combinations
  • compile for AGENTS.md
  • user and global scopes for all combinations above

The test results are to be passed to the review panel for approval and posted here as a comment that shows a table of all cases tested above and the results

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

Local integration test report — copilot-cowork target (interim)

@danielmeppiel — proof of local end-to-end testing on feat/cowork-skills HEAD a8bfec2. Three independent E2E runs were executed against a real OneDrive mount on macOS over the course of this session.

This is an interim report: the matrix below honestly flags three cells that I have not yet exercised locally (#8 project scope, #9 compile, #10 marketplace). I am running them now and will follow up with a complete matrix in a second comment.

Scope clarification

The copilot-cowork target is a single-primitive target by design — it deploys only skills to the OneDrive Cowork/skills/ directory consumed by GitHub Copilot. Other primitive types (instructions, subagents, hooks, MCP) are not in scope for this target — they continue to deploy via existing targets (copilot, claude, etc.). This is documented at docs/src/content/docs/integrations/copilot-cowork.md.

Results matrix

# Case Scope Result Evidence
1 apm install --target copilot-cowork — package with skills primitive user (--global) PASS 2 skills deployed to <OneDrive>/Documents/Cowork/skills/<skill-a>/SKILL.md and <skill-b>/SKILL.md; lockfile records cowork://skills/<skill-a> + cowork://skills/<skill-b>
2 Transitive dependencies installed via cowork target user (--global) PASS A test package pulled in 41 APM deps including 32 transitive skill files; all deployed to cowork root and tracked in ~/.apm/apm.lock.yaml
3 instructions / subagents / hooks / MCP primitives via copilot-cowork both N/A — target by design only deploys skills. Other primitives continue to install via their existing targets in the same apm install invocation.
4 Idempotent re-install (no changes) user PASS apm.lock.yaml unchanged -- skipping write on second invocation; no file rewrites
5 apm uninstall <pkg> — removes lockfile entries user (--global) PASS Both cowork://skills/... lockfile entries removed; apm.yml updated
6 apm uninstall <pkg> — removes deployed files from OneDrive disk user (--global) PASS (after fix) [+] Cleaned up 32 integrated skills; ls of <OneDrive>/Documents/Cowork/skills/ confirms both skill directories deleted. This was the bug found on first E2E run; fixed across commits e85534c -> 1446734 -> a8bfec2.
7 Re-install after uninstall (recovers cleanly) user PASS Skill directories re-deployed with fresh timestamps; no stale state
8 apm install (project scope, no --global) with --target copilot-cowork project NOT YET TESTED — will run as part of follow-up. The target writes to OneDrive regardless of scope (because OneDrive is a user-level location), so I want to confirm the project-scope behaviour explicitly.
9 apm compile produces AGENTS.md correctly with copilot-cowork enabled user NOT YET TESTED — will run as part of follow-up. copilot-cowork does not contribute to AGENTS.md (it only deploys skills, not prompts/instructions); expect zero impact but will verify.
10 Marketplace operations (apm marketplace) with cowork-deployed packages user NOT YET TESTED — will run as part of follow-up. Marketplace operates on packages/registries; the cowork target is purely the deploy destination. No mechanical interaction expected but will validate.
11 OneDrive auto-detection on macOS user PASS OneDrive mount auto-detected; <OneDrive>/Documents/Cowork/skills/ resolved without APM_COPILOT_COWORK_SKILLS_DIR override
12 APM_COPILOT_COWORK_SKILLS_DIR env-var override user PASS Used in 3 E2E runs to redirect cowork root to /tmp/... for isolation
13 copilot_cowork_skills_dir config override (apm config set ...) user NOT YET RUN END-TO-END — implementation present at src/apm_cli/integration/copilot_cowork_paths.py:110-124, behaviour identical to env var, covered by unit tests. Will exercise as part of follow-up.
14 Pre-existing skills outside APM are not touched on uninstall user PASS — verified across 3 E2E runs: pre-existing user-authored skills retained throughout install + uninstall + re-install cycles

Bugs found and fixed locally before posting

The local testing surfaced three coupled bugs in the uninstall path that are not covered by unit tests alone — the bugs only manifest in real apm uninstall --global flows. All three are fixed and shipped on this PR:

Commit Layer Root cause
e85534c cleanup.py + targets.py get_integration_prefixes gated cowork prefix on resolved_deploy_root (always None on the static registry) instead of user_root_resolver
1446734 skill_integrator.py + uninstall guard SkillIntegrator.sync_integration prefix-tuple did not include cowork://; _skill_dirs_exist guard checked only local project_root subdirs so cowork-only state never triggered the sync call
a8bfec2 uninstall partition + targets _buckets["skills"] was always empty for cowork because partition_managed_files uses resolved_deploy_root; even when populated, passing targets=_resolved_targets excluded copilot-cowork from prefix construction inside sync_integration

Without these fixes, apm uninstall silently left skill directories orphaned on disk in OneDrive forever.

Test artefacts

Test commands used (HEAD a8bfec2):

apm experimental enable copilot-cowork
APM_COPILOT_COWORK_SKILLS_DIR=/tmp/cowork-e2e-$$ \
  apm install --target copilot-cowork --global
# verify deploy, lockfile, idempotency
apm uninstall <pkg> --global
# verify on-disk deletion + lockfile cleanup
apm install --target copilot-cowork --global   # re-install verification
  • Unit suite: 5606/5606 passing at a8bfec2 (excluding 3 pre-existing failures unrelated to this PR: test_audit_report.py Python 3.11 f-string SyntaxError, test_helpers.py system python binary missing, test_runtime_factory.py system llm binary missing).
  • Cowork-specific unit + integration regression suite: 13/13 passing.

Follow-up

Running #8, #9, #10, #13 next, then re-rebasing onto the latest main and re-running the unit suite. I will post a complete final matrix in a follow-up comment. PR remains in this state until then.

sergio-sisternes-epam pushed a commit that referenced this pull request Apr 27, 2026
The cowork target was registered in the integration layer and gated
behind an experimental flag, but TargetParamType.convert rejected it
at parse time because it was missing from VALID_TARGET_VALUES.
This made the runtime enable-hint in phases/targets.py unreachable.

Add EXPERIMENTAL_TARGETS as a separate parser-layer set so the parser
accepts the token while runtime gating in _flag_gated() and
phases/targets.py continues to enforce the flag. parse_target_arg("all")
expansion is unchanged.

Refs #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam pushed a commit that referenced this pull request Apr 27, 2026
Adds parser-layer cases for EXPERIMENTAL_TARGETS and a CliRunner E2E
test that exercises 'apm install --target cowork --global' through the
real click parser. The latter would have caught the bug fixed in 2f96dd5.

Refs #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam pushed a commit that referenced this pull request Apr 27, 2026
…reproofing

Rename the experimental cowork feature to copilot-cowork to leave room
for future variants (e.g. claude-cowork). Pure rename - no behaviour
change. PR #926 is unmerged so no backward-compat shims are needed.

User-facing surface:
- Experimental flag:  cowork              -> copilot-cowork
- Target name:        cowork              -> copilot-cowork
- Env var:            APM_COWORK_SKILLS_DIR -> APM_COPILOT_COWORK_SKILLS_DIR
- Config key (CLI):   cowork-skills-dir   -> copilot-cowork-skills-dir
- Config key (store): cowork_skills_dir   -> copilot_cowork_skills_dir

Internal symbols (also renamed for symmetry with future claude-cowork):
- src/apm_cli/integration/cowork_paths.py
    -> src/apm_cli/integration/copilot_cowork_paths.py
- resolve_cowork_skills_dir   -> resolve_copilot_cowork_skills_dir
- _resolve_cowork_root        -> _resolve_copilot_cowork_root
- _COWORK_SKILLS_SUBDIR       -> _COPILOT_COWORK_SKILLS_SUBDIR
- get/set/unset_cowork_skills_dir
    -> get/set/unset_copilot_cowork_skills_dir
- 3 test files renamed via git mv

Preserved (concept-level / shared across all future cowork variants):
- CoworkResolutionError exception class
- Lockfile prefix and URI scheme constants
- Cap helpers and warn flags

Docs:
- docs/src/content/docs/integrations/cowork.md
    -> docs/src/content/docs/integrations/copilot-cowork.md
- Sidebar slug, cross-references, CLI examples, env var, and the
  apm-usage skill (packages/apm-guide) all updated.

Validation: 5592/5592 unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

Local integration test report — copilot-cowork target (final)

@danielmeppiel — follow-up to my interim report above. The four open cells (#8, #9, #10, #13) have now been executed locally against the rebased branch (HEAD af48756, clean rebase onto main, 5753 / 5753 unit tests pass, CI green). Real OneDrive mount remained empty throughout via APM_COPILOT_COWORK_SKILLS_DIR redirection.

Final results matrix

# Case Result Evidence
1 install (user --global), package with skills PASS 2 skills deployed, lockfile records cowork://skills/...
2 install with transitive deps PASS 41 deps + 32 transitive skills tracked in lockfile
3 other primitive types via copilot-cowork N/A by design — target deploys only skills
4 idempotent re-install PASS apm.lock.yaml unchanged -- skipping write
5 uninstall removes lockfile entries PASS cowork://... entries cleared
6 uninstall removes deployed skills on disk PASS (after fixes e2f541a -> 44b7da6 -> 53d5b62) [+] Cleaned up N integrated skills confirmed by ls
7 re-install after uninstall PASS fresh deploy timestamps
8 install without --global (project scope) PASS Guard fires at install: [x] The 'copilot-cowork' target requires --global (user scope). Run: apm install --target copilot-cowork --global — exit 1, deploy dir never created, zero cowork pollution
9 apm compile with cowork active PASS AGENTS.md generated cleanly (5222 bytes); zero "cowork" references in output. Cowork is a deploy target only and contributes nothing to compiled context — confirmed by inspection
10 marketplace ops with cowork-deployed packages PASS apm marketplace list/browse/add all run cleanly; marketplace state lives in ~/.apm/marketplaces.json and is fully orthogonal to cowork lockfile entries — no cross-talk
11 OneDrive auto-detection (macOS) PASS mount auto-resolved without override
12 APM_COPILOT_COWORK_SKILLS_DIR env-var override PASS exercised in every E2E run
13 apm config set copilot-cowork-skills-dir end-to-end PASS set / get / install resolution / unset all behave correctly. Config-only resolution (env var unset) deploys to the configured path
14 pre-existing skills outside APM untouched PASS verified across all E2E runs

Test environment

  • Branch HEAD: af48756 (rebased onto upstream/main 5c0976b, 8 commits replayed cleanly, 6 conflicts resolved across CHANGELOG.md, two doc files, services.py, template.py, two test files)
  • Unit suite: 5753 / 5753 passing, 26 subtests, no regressions from main
  • CI: all checks green on Linux (CI / Deploy Docs / CodeQL)
  • Cowork-specific regression suite: 13 / 13 passing
  • Real OneDrive cowork dir: empty before, during, after — verified

Notes / minor observations surfaced during testing

These are not blockers for this PR (none are introduced by copilot-cowork), but worth flagging:

  • --global uninstall scope hint: apm uninstall <pkg> --global invoked from a subdirectory with a local apm.yml operates on ~/.apm/apm.yml with only an [i] info log. Worth strengthening to a [!] warning + confirmation prompt in a follow-up (pre-existing, repo-wide behaviour).
  • Config key casing: CLI accepts copilot-cowork-skills-dir (hyphens); JSON stores copilot_cowork_skills_dir (underscores). apm config get rejects the underscore form. Minor UX nit.
  • 50-skill cap is advisory-only by design — it warns but does not fail the install (documented behaviour).

Verdict

All cells in the matrix accounted for. Three bugs found locally (uninstall path) were fixed and shipped on this PR before this report; no further functional gaps observed. Ready for panel review.

Sergio Sisternes and others added 7 commits April 27, 2026 09:55
…ustom skills (#913)

Adds a first-class, experimentally-gated 'cowork' target so APM users on
the Microsoft 365 Copilot Frontier preview can deploy their skills to
Cowork (https://learn.microsoft.com/microsoft-365/copilot/cowork) with a
single 'apm install --target cowork --global'.

Behind 'apm experimental enable cowork'. Skills only; non-skill
primitives skipped with one summary warning. User scope only;
project-scope rejected. Path-safety guards on the OneDrive resolver and
all lockfile path I/O. Caps (50 skills / 1 MiB SKILL.md) are warn-only.

Resolution order for the OneDrive skills directory:
  APM_COWORK_SKILLS_DIR env > apm config cowork-skills-dir > macOS
  CloudStorage glob > Windows ONEDRIVECOMMERCIAL/ONEDRIVE > none.

Adds 'apm config set/get/unset cowork-skills-dir' (set gated on flag;
get/unset always available as a safety valve).

Lockfile entries use a synthetic 'cowork://' URI scheme translated at
I/O boundaries.

Closes #913.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The cowork target was registered in the integration layer and gated
behind an experimental flag, but TargetParamType.convert rejected it
at parse time because it was missing from VALID_TARGET_VALUES.
This made the runtime enable-hint in phases/targets.py unreachable.

Add EXPERIMENTAL_TARGETS as a separate parser-layer set so the parser
accepts the token while runtime gating in _flag_gated() and
phases/targets.py continues to enforce the flag. parse_target_arg("all")
expansion is unchanged.

Refs #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds parser-layer cases for EXPERIMENTAL_TARGETS and a CliRunner E2E
test that exercises 'apm install --target cowork --global' through the
real click parser. The latter would have caught the bug fixed in 2f96dd5.

Refs #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…reproofing

Rename the experimental cowork feature to copilot-cowork to leave room
for future variants (e.g. claude-cowork). Pure rename - no behaviour
change. PR #926 is unmerged so no backward-compat shims are needed.

User-facing surface:
- Experimental flag:  cowork              -> copilot-cowork
- Target name:        cowork              -> copilot-cowork
- Env var:            APM_COWORK_SKILLS_DIR -> APM_COPILOT_COWORK_SKILLS_DIR
- Config key (CLI):   cowork-skills-dir   -> copilot-cowork-skills-dir
- Config key (store): cowork_skills_dir   -> copilot_cowork_skills_dir

Internal symbols (also renamed for symmetry with future claude-cowork):
- src/apm_cli/integration/cowork_paths.py
    -> src/apm_cli/integration/copilot_cowork_paths.py
- resolve_cowork_skills_dir   -> resolve_copilot_cowork_skills_dir
- _resolve_cowork_root        -> _resolve_copilot_cowork_root
- _COWORK_SKILLS_SUBDIR       -> _COPILOT_COWORK_SKILLS_SUBDIR
- get/set/unset_cowork_skills_dir
    -> get/set/unset_copilot_cowork_skills_dir
- 3 test files renamed via git mv

Preserved (concept-level / shared across all future cowork variants):
- CoworkResolutionError exception class
- Lockfile prefix and URI scheme constants
- Cap helpers and warn flags

Docs:
- docs/src/content/docs/integrations/cowork.md
    -> docs/src/content/docs/integrations/copilot-cowork.md
- Sidebar slug, cross-references, CLI examples, env var, and the
  apm-usage skill (packages/apm-guide) all updated.

Validation: 5592/5592 unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply remaining APM Review Panel follow-ups on top of the rename:

P2 (perf): hoist resolve_copilot_cowork_skills_dir() out of the
  per-path loop in BaseIntegrator.sync_remove_files. Lazy resolution
  on first cowork:// path encountered, cached for the rest of the
  loop. Zero cost when no cowork:// paths are present.

P3 (UX):  Linux-specific error wording when the copilot-cowork
  resolver returns None. Linux has no OneDrive auto-detection by
  design; the previous message implied detection had failed.
  macOS / Windows wording preserved.

P4 (visibility): emit a one-time [!] warning from sync_remove_files
  when copilot-cowork:// lockfile entries are encountered but the
  resolver returns None. Previously these orphans were silently
  skipped, leaving stale entries in apm.lock with no user signal.
  Adds an optional logger= kwarg with a _rich_warning fallback so
  existing call sites need no change.

13 new unit tests cover the three behaviours. Full unit suite:
5603 passed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
apm uninstall left cowork-deployed skills on disk in OneDrive.
Subsequent apm install runs did not clean them up either, so stale
skill directories accumulated indefinitely.

Two coupled defects in the cleanup pipeline:

1. get_integration_prefixes (integration/targets.py) gated the
   'cowork://skills/' allowed-prefix on resolved_deploy_root, but
   that attribute is transient per-install state and is always None
   on the static KNOWN_TARGETS registry instance that cleanup uses
   (targets=None). Replaced with the capability flag
   user_root_resolver, which IS set on the static definition. The
   normal install path is unaffected because per-install targets
   also have a non-None user_root_resolver.

2. remove_stale_deployed_files (integration/cleanup.py) computed
   project_root / 'cowork://skills/...' for cowork lockfile entries,
   producing a nonsensical filesystem path that always failed
   .exists() and was silently classified as 'already gone'. Added
   explicit cowork:// handling: resolves the OneDrive root once
   (lazy, cached for the rest of the call), uses from_lockfile_path
   to translate the URI, then deletes the real file.

Edge cases:
- Cowork root resolves but file is gone -> idempotent no-op,
  lockfile entry still removed.
- Cowork root cannot be resolved (no env var, no config, Linux
  without auto-detect) -> file NOT deleted, lockfile entry NOT
  removed (so a later configured install can clean it up), one-
  time [!] warning naming the count + recovery commands.
- from_lockfile_path raises (containment violation, malformed
  URI) -> entry counted as failed, one-time warning, lockfile
  entry retained.

12 new unit tests cover the two fixes plus an integration-style
regression test for the original reproducer (drawio uninstall +
reinstall, real temp cowork root, assert skill dir is gone).

Full unit suite: 5614 passed (up from 5603).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rator

The previous fix (e85534c) targeted remove_stale_deployed_files, but
apm uninstall routes through SkillIntegrator.sync_integration, which
had two coupled gaps that left cowork skills orphaned on disk:

1. Prefix mismatch in skill_integrator.sync_integration: the prefix
   tuple only contained local skill dirs (.github/skills/, etc.), so
   cowork://skills/* lockfile entries failed startswith() and were
   silently skipped. Now extends the prefix tuple with
   COWORK_LOCKFILE_PREFIX when cowork:// entries are present and
   resolves them via lazy resolve_copilot_cowork_skills_dir() +
   from_lockfile_path translation. Edge cases mirror cleanup.py:
   resolver None -> one-time warn + skip, translation error ->
   counted as error, missing file -> idempotent.

2. Guard bypass in commands/uninstall/engine.py: _skill_dirs_exist
   only checked local project_root subdirs, so when only cowork
   entries existed, sync_integration was never called at all. Guard
   extended to also fire when the skills bucket contains cowork://
   entries.

Adds 9 new tests (7 unit + 2 integration regression). Full unit suite
5622 passing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sergio Sisternes and others added 2 commits April 27, 2026 09:55
Two coupled gaps in _sync_integrations_after_uninstall caused
apm uninstall to silently skip cowork skill deletion even after
the SkillIntegrator was updated in 1446734:

Gap 1: The _has_cowork_skills guard checked _buckets["skills"]
for cowork:// paths, but partition_managed_files() never routes
cowork:// URIs there.  The function uses resolved_deploy_root to
detect dynamic-root targets; the static KNOWN_TARGETS entry for
copilot-cowork always has resolved_deploy_root=None (it is set
only after for_scope() resolves OneDrive at install time).
Result: skill bucket was always empty, guard always returned False.

Gap 2: Even if cowork paths had reached sync_integration, they
would have failed the startswith(skill_prefix_tuple) guard inside
SkillIntegrator.sync_integration because _resolved_targets=[copilot]
produces skill_prefix_tuple=('.copilot/skills/',) -- no cowork
prefix.  The COWORK_LOCKFILE_PREFIX is only added when a target
with user_root_resolver is present, which requires copilot-cowork
to be in the source targets list.

Fix (engine.py, _sync_integrations_after_uninstall):
- Scan sync_managed directly for cowork:// paths (bypasses partition).
- Merge found paths into _buckets["skills"] before the sync call.
- Pass targets=None when cowork entries are present, so
  sync_integration uses KNOWN_TARGETS as source (which includes
  copilot-cowork with user_root_resolver set, causing
  COWORK_LOCKFILE_PREFIX to be added to skill_prefix_tuple).

Verified end-to-end: apm uninstall drawio --global now deletes
cowork://skills/drawiodiagram-ops and cowork://skills/drawiogenerate-diagram
from disk and prints "Cleaned up 2 integrated skills".
Unit suite: 5606 passed (1239 integration/install), 0 new failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Linux emits "Cowork has no auto-detection on Linux." while macOS emits
"no OneDrive path detected" from _resolve_copilot_cowork_root. Tests
hard-coded the macOS variant and broke CI on Linux. Accept either form.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam sergio-sisternes-epam added panel-review Trigger the apm-review-panel gh-aw workflow and removed panel-review Trigger the apm-review-panel gh-aw workflow labels Apr 27, 2026
@github-actions
Copy link
Copy Markdown

APM Review Panel Verdict

Disposition: REQUEST_CHANGES (two required pre-merge fixes; the rest of the implementation is sound and merge-ready once these are addressed)


Per-persona findings

Python Architect: This is a major architectural change: TargetProfile gains three new fields (user_root_resolver, resolved_deploy_root, requires_flag) and a new deploy_path() method, introducing the concept of dynamic-root targets whose deploy root is resolved at runtime via a callable rather than a static path. A new cowork:// URI scheme is used in the lockfile to keep paths portable across machines. The overall design is sound and idiomatic for APM.

1. OO / class diagram

classDiagram
    direction LR

    class TargetProfile {
        <<ValueObject>>
        +name str
        +root_dir str
        +primitives dict
        +user_root_dir str
        +user_supported bool
        +requires_flag str
        +user_root_resolver Callable
        +resolved_deploy_root Path
        +deploy_path(project_root, *parts) Path
        +for_scope(user_scope) TargetProfile
        +supports(primitive) bool
    }

    class PrimitiveMapping {
        <<ValueObject>>
        +name str
        +subdir str
        +deploy_root str
    }

    class BaseIntegrator {
        <<Abstract>>
        +validate_deploy_path(rel_path, project_root, targets) bool
        +sync_remove_files(project_root, managed_files, prefix, targets, logger) dict
        +partition_managed_files(managed_files, source) dict
        +cleanup_empty_parents(deleted, stop_at)
    }

    class SkillIntegrator {
        +integrate_package_skill(pkg, project_root, targets, managed_files) IntegrationResult
        +sync_integration(apm_package, project_root, managed_files) dict
    }

    class CopilotCoworkPaths {
        <<IOBoundary>>
        +resolve_copilot_cowork_skills_dir() Path
        +to_lockfile_path(absolute, cowork_root) str
        +from_lockfile_path(lockfile_path, cowork_root) Path
        +is_cowork_path(lockfile_path) bool
        COWORK_URI_SCHEME = "cowork://"
        COWORK_LOCKFILE_PREFIX = "cowork://skills/"
    }

    class CoworkResolutionError {
        <<Exception>>
    }

    class Config {
        <<IOBoundary>>
        +get_copilot_cowork_skills_dir() str
        +set_copilot_cowork_skills_dir(path) void
        +unset_copilot_cowork_skills_dir() void
    }

    class ExperimentalFlag {
        <<ValueObject>>
        +name str
        +description str
        +default bool
        +hint str
    }

    TargetProfile *-- PrimitiveMapping : primitives
    SkillIntegrator --|> BaseIntegrator
    TargetProfile ..> CopilotCoworkPaths : user_root_resolver calls
    CopilotCoworkPaths ..> CoworkResolutionError : raises
    CopilotCoworkPaths ..> Config : reads
    BaseIntegrator ..> CopilotCoworkPaths : URI translation

    class TargetProfile:::touched
    class CopilotCoworkPaths:::touched
    class BaseIntegrator:::touched
    class SkillIntegrator:::touched
    class Config:::touched
    class ExperimentalFlag:::touched

    classDef touched fill:#fff3b0,stroke:#d47600

    note for CopilotCoworkPaths "Adapter: absolute path <-> cowork:// URI"
    note for TargetProfile "Strategy: user_root_resolver callable\nenables dynamic-root targets without\nchanging BaseIntegrator"
Loading

2. Execution flow diagram

flowchart TD
    A["apm install --target copilot-cowork --global"]
    B["phases/targets.py::run()"]
    C{"_flag_gated(copilot-cowork)?"}
    D["_resolve_targets(project_root, user_scope=True)"]
    E["TargetProfile.for_scope(user_scope=True)"]
    F["[FS] user_root_resolver() -> _resolve_copilot_cowork_root()"]
    G["[FS] copilot_cowork_paths::resolve_copilot_cowork_skills_dir()"]
    H{"env APM_COPILOT_COWORK_SKILLS_DIR set?"}
    I["[FS] config::get_copilot_cowork_skills_dir()"]
    J{"macOS / Windows?"}
    K["[FS] ~/Library/CloudStorage/OneDrive* glob"]
    L["resolved_deploy_root = Path"]
    M["phases/integrate.py::run()"]
    N["services::integrate_package_primitives()"]
    O{"target.resolved_deploy_root is not None?"}
    P["[FS] skill_integrator: target_skill_dir = resolved_deploy_root/skill_name"]
    Q["[FS] shutil.copytree -> OneDrive path"]
    R["services::_deployed_path_entry()"]
    S{"path.relative_to(project_root) OK?"}
    T["[LOCK] to_lockfile_path() -> cowork://skills/..."]
    U["[LOCK] apm.lock.yaml updated"]
    V["phases/integrate.py::_check_cowork_caps() warn-only"]

    A --> B --> C
    C -- "flag ON" --> D --> E --> F --> G
    G --> H
    H -- "yes" --> L
    H -- "no" --> I
    I -- "set" --> L
    I -- "not set" --> J
    J -- "macOS" --> K --> L
    J -- "Windows" --> L
    L --> M --> N --> O
    O -- "yes (cowork)" --> P --> Q --> R
    R --> S
    S -- "ValueError (outside tree)" --> T
    T --> U --> V
Loading

Design patterns

  • Used in this PR: Strategy -- user_root_resolver callable on TargetProfile lets each dynamic-root target supply its own resolver without subclassing; Adapter -- copilot_cowork_paths translates between absolute filesystem paths and portable cowork:// URIs at I/O boundaries.
  • Pragmatic suggestion: The _cowork_root_resolved: bool + _cowork_root_cached: Optional[Path] two-variable lazy-init pattern is copy-pasted verbatim in three separate files (base_integrator.py::sync_remove_files, skill_integrator.py::sync_integration, cleanup.py::remove_stale_deployed_files). A small _CoworkRootCache dataclass or a functools.lru_cache on resolve_copilot_cowork_skills_dir would DRY this out and make it easier to add future dynamic-root targets without copy-paste drift.

CLI Logging Expert: Overall output architecture is correct -- new paths use CommandLogger methods with ASCII bracket symbols throughout. Two issues.

  1. Direct _rich_warning call in skill_integrator.py::sync_integration (lines ~1153-1162 in the diff): when cowork entries are skipped because OneDrive is unavailable during uninstall, the warning is emitted via a bare _rich_warning() import rather than through a logger. sync_integration has no logger parameter. The correct fix is to accept an optional logger parameter (mirroring the logger= added to sync_remove_files in the same PR) and route the message there. This is the anti-pattern flagged in the persona guide ("Using _rich_* directly instead of CommandLogger in command functions").

  2. ctx.logger.progress(...) with symbol="info" for a failed explicit-target request (phases/targets.py ~line 76): a progress() call is a green/neutral signal; it should not be used when user intent could not be satisfied. See DevX UX findings.

All other messages are well-formed: exact counts, names the thing, includes the fix. symbol="cross" is valid (maps to [x] in STATUS_SYMBOLS). Cap warning messages are appropriately one-per-run via ctx.cowork_nonsupported_warned.


DevX UX Expert: The new apm config unset subcommand and copilot-cowork-skills-dir config key follow npm/cargo conventions. OneDrive auto-detection resolution order is clearly documented. Linux "env/config only" is explicitly called out. Most of the command surface is clean.

One blocking UX issue: in phases/targets.py, when the user explicitly passes --target copilot-cowork and the copilot-cowork flag is disabled, the code emits a logger.progress() message and continues the install -- without deploying to cowork and without a non-zero exit. A user who writes apm install --target copilot-cowork --global in a script expects either success (cowork deployed) or a clear failure exit. Silent continuation violates the "failure mode is the product" principle. Fix: replace ctx.logger.progress(...) with ctx.logger.error(...) + raise SystemExit(1) in that branch. (Note: auto-detect should remain soft -- only explicit --target copilot-cowork should hard-fail when the flag is off.)

Secondary (non-blocking): apm config get copilot-cowork-skills-dir (specific-key form) prints without a leading 2-space indent, while bare apm config get uses indented output. Minor formatting inconsistency worth a follow-up.

Docs (copilot-cowork.md, cli-commands.md, experimental.md, commands.md in apm-guide) are all updated. ✓


Supply Chain Security Expert: Security posture is strong. All path resolution uses validate_path_segments + ensure_path_within from path_security.py. cowork:// lockfile entries are validated for containment in from_lockfile_path(). validate_deploy_path() in BaseIntegrator now correctly handles cowork URIs. Experimental flag gate prevents deployment until explicit opt-in. No new auth/token surface is introduced. Broad except Exception: return False in the validation branch is acceptable -- fails closed.

One required security fix: config.py::set_copilot_cowork_skills_dir accepts user-supplied path input, expands it, validates absoluteness -- but does NOT call validate_path_segments. The value is validated later when read back by resolve_copilot_cowork_skills_dir(), so defense-in-depth is partially present. However, the path safety rule requires validate_path_segments(value, context=) at parse/store time. Fix:

from apm_cli.utils.path_security import validate_path_segments, PathTraversalError
try:
    validate_path_segments(path, context="copilot-cowork-skills-dir config")
except PathTraversalError as exc:
    raise ValueError(f"Invalid path: {exc}") from exc

Add this before the expanduser call in set_copilot_cowork_skills_dir.

Documentation gap (follow-up, not blocking): docs/src/content/docs/reference/lockfile-spec.md is not updated in this PR to document the new cowork:// URI scheme. Lockfile consumers (other tools, auditors) cannot distinguish cowork entries without this.

macOS OneDrive glob results are filesystem-local and not a realistic attacker surface at user-scope. Windows env vars are correctly validated before construction. unset_copilot_cowork_skills_dir config write pattern is consistent with the existing unset_temp_dir added in this PR.


Auth Expert: Not activated -- the PR adds a file-deployment target routed through locally-synced OneDrive paths; no auth tokens, credential resolution, AuthResolver host classification, or HTTP authorization headers appear in any of the 37 changed files.


OSS Growth Hacker: The "APM's first target outside the IDE" milestone is a genuine story beat. The positioning in the PR body -- "one manifest now governs both your coding agent and your enterprise AI assistant" -- is sharp and repeatable. The experimental gate is the right call: it protects adoption credibility (no broken first-run for M365 users on non-OneDrive machines).

Two growth observations fed to CEO:

  1. The copilot-cowork / copilot_cowork / "Microsoft 365 Copilot Cowork" naming sprawl (three surface-level strings for one concept) will fragment search results and doc discoverability. Worth normalizing in a follow-up even if it is only cosmetic.
  2. When the flag graduates from experimental, the CHANGELOG entry + a focused release post on "APM: deploy skills to M365 Copilot Cowork with one command" will compound well with the Lorenzo Storelli enterprise-policy discussion thread. Flag this in WIP/growth-strategy.md as a pre-GA milestone story.

CEO arbitration

The panel reached broad agreement: this is a well-architected, well-tested addition (+141 tests, +5635 LOC) with a clean security posture. The two blocking issues are unambiguous: (1) missing validate_path_segments at store time in set_copilot_cowork_skills_dir violates the path safety rule and has no legitimate counter-argument; (2) an explicit --target copilot-cowork that silently continues without cowork deployment and exits 0 when the flag is off is a correctness bug for script/CI users -- explicit intent must resolve to success or a hard failure. Both fixes are narrow and low-risk. The Growth Hacker's naming note is valid but cosmetic; I defer it to a follow-up. The _rich_warning anti-pattern in skill_integrator.py::sync_integration is real but the output path (uninstall of cowork packages when OneDrive is unavailable) is low-frequency and non-blocking for this PR. The lockfile-spec doc gap is a follow-up issue, not a merge gate. Ratification: REQUEST_CHANGES on the two items below; all other specialist findings are deferred or optional.


Required actions before merge

  1. src/apm_cli/config.py::set_copilot_cowork_skills_dir -- add validate_path_segments(path, context="copilot-cowork-skills-dir config") (with PathTraversalError -> ValueError re-raise) before the os.path.expanduser() call. This is required by the path safety rule ("at parse time") and is missing despite being applied at read-time in resolve_copilot_cowork_skills_dir().

  2. src/apm_cli/install/phases/targets.py (the flag-OFF + explicit --target copilot-cowork branch, ~line 76) -- replace ctx.logger.progress(..., symbol="info") with ctx.logger.error(..., symbol="cross") and add raise SystemExit(1). An explicit --target copilot-cowork that the user requests must either succeed or fail loudly. The current silent-continue + exit 0 is a correctness bug for CI and scripted usage. Auto-detect behavior (no explicit --target) should remain soft (silent omit when flag is off).


Optional follow-ups

  • skill_integrator.py::sync_integration: add an optional logger parameter (mirroring sync_remove_files) and route the cowork-skipped warning through it instead of the bare _rich_warning import. Unblocks the CLI Logging anti-pattern.
  • Lazy cowork-root cache: extract the _cowork_root_resolved / _cowork_root_cached two-variable pattern (duplicated in base_integrator.py, skill_integrator.py, cleanup.py) into a _CoworkRootCache helper or apply functools.lru_cache to resolve_copilot_cowork_skills_dir. Makes adding future dynamic-root targets safer.
  • Lockfile spec: update docs/src/content/docs/reference/lockfile-spec.md to document the cowork://skills/<name>/SKILL.md URI format so lockfile consumers can parse it without reading source code.
  • Naming consistency: copilot-cowork (CLI), copilot_cowork (Python internal), and "Microsoft 365 Copilot Cowork" (full brand) are three forms of the same concept. Consider a glossary note in the doc or a short "also known as" callout to aid discoverability.
  • from_lockfile_path cleanup: the two-step prefix strip ([len(COWORK_URI_SCHEME):] then strip "skills/") can be replaced by [len(COWORK_LOCKFILE_PREFIX):] directly, eliminating the branching and using the existing constant.
  • user_root_resolver lambda: user_root_resolver=lambda: _resolve_copilot_cowork_root() can be user_root_resolver=_resolve_copilot_cowork_root (direct function reference, one less indirection layer).

Generated by PR Review Panel for issue #926 · ● 1.2M ·

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

sergio-sisternes-epam commented Apr 27, 2026

Hi @danielmeppiel — quick status update.

All panel feedback from the pre-merge review has been applied, and the branch is now rebased onto the latest main (HEAD 371b8e0).

Addressed in this round:

  • P2/P3/P4 refactors — applied in cca5c5e (constants extraction, scope guard centralisation, error message consistency).
  • Naming alignment — target and flag renamed to copilot-cowork end-to-end (10d3045), with parser-layer acceptance via EXPERIMENTAL_TARGETS and regression tests (a4e72c5, 76ec3d8).
  • Uninstall correctness — cowork skills are now properly removed on uninstall, both via SkillIntegrator and when only cowork entries are present in the lockfile (e2f541a, 44b7da6, 53d5b62).
  • Cross-platform CI — Linux test failure fixed by accepting both platform-specific cowork resolver error messages (af48756).
  • Local integration matrix — full 14-cell test matrix executed locally (results in #issuecomment-4325554366), all gap items pass.

Current state:

  • 5753/5753 unit tests passing.
  • CI green across the board (Tier 1 + Deploy Docs + CodeQL).
  • Working tree clean, no outstanding TODOs from the panel review.

Ready for another look whenever you have a moment. Thanks!

@sergio-sisternes-epam sergio-sisternes-epam added this pull request to the merge queue Apr 27, 2026
Merged via the queue into main with commit 2153872 Apr 27, 2026
9 checks passed
@sergio-sisternes-epam sergio-sisternes-epam deleted the feat/cowork-skills branch April 27, 2026 09:57
danielmeppiel pushed a commit that referenced this pull request Apr 27, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel pushed a commit that referenced this pull request Apr 27, 2026
Promotes [Unreleased] to [0.10.0] - 2026-04-27. Each PR since v0.9.4
gets one 'so what' line:

- #926 Microsoft 365 Cowork target ships impl
- #790 marketplace authoring CLI (init, package add/set, build, check,
  outdated, doctor, publish) -- collapsed from 20+ bullets to one
- #722 marketplace plugin -> package rename + --help sectioning -- collapsed
- #980 README 'Coming from npx skills add' conversion block
- #981 docs auto-deploy on tag push (real fix for the #953 attempt)
- #985 pr-description-skill evals suite
- #984 pr-description-skill mermaid hardening
- #989 cowork sys.platform mock for Windows CI

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

panel-review Trigger the apm-review-panel gh-aw workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: experimental support for Microsoft 365 Copilot Cowork custom skills

3 participants