fix: gate .claude/commands/ deployment behind integrate_claude flag by neop26 · Pull Request #443 · microsoft/apm

neop26 · 2026-03-24T18:43:58Z

The CommandIntegrator was called unconditionally in _integrate_package_primitives(), ignoring the integrate_claude flag derived from the detected or configured target.

This caused .claude/commands/ to be created on every apm install, even when the project is configured with target: copilot (vscode), which sets integrate_claude=False.

The docstring of should_integrate_claude() explicitly states that 'commands' should be governed by this flag, matching the pattern already applied to Claude agents and hooks. The OpenCode commands integrator has a similar self-guard (.opencode/ must exist).

Fix: wrap integrate_package_commands() in if integrate_claude: so it is consistent with all other Claude-specific integration paths.

Reproducer: set target: copilot in apm.yml, add .prompt.md files under .apm/prompts/, run apm install — .claude/commands/ is created despite no Claude target being configured.

Description

Brief description of changes and motivation.

Fixes # (issue)

Type of change

Bug fix
New feature
Documentation
Maintenance / refactor

Testing

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

The CommandIntegrator was called unconditionally in _integrate_package_primitives(), ignoring the integrate_claude flag derived from the detected or configured target. This caused .claude/commands/ to be created on every apm install, even when the project is configured with target: copilot (vscode), which sets integrate_claude=False. The docstring of should_integrate_claude() explicitly states that 'commands' should be governed by this flag, matching the pattern already applied to Claude agents and hooks. The OpenCode commands integrator has a similar self-guard (.opencode/ must exist). Fix: wrap integrate_package_commands() in if integrate_claude: so it is consistent with all other Claude-specific integration paths. Reproducer: set target: copilot in apm.yml, add .prompt.md files under .apm/prompts/, run apm install — .claude/commands/ is created despite no Claude target being configured.

neop26 · 2026-03-24T18:47:55Z

@microsoft-github-policy-service agree

Copilot

Pull request overview

This PR fixes an install-time integration bug where Claude command deployment (.claude/commands/) ran unconditionally during apm install, ignoring the integrate_claude flag derived from the detected/configured target (e.g., target: copilot / vscode).

Changes:

Gate .claude/commands/ deployment by wrapping integrate_package_commands() in if integrate_claude:.
Align Claude command integration behavior with other Claude-specific integrations (agents/hooks) that already respect integrate_claude.

Copilot · 2026-03-24T18:48:18Z

+            _log_integration(f"  └─ {command_result.files_integrated} commands integrated -> .claude/commands/")
+        if command_result.files_updated > 0:
+            _log_integration(f"  └─ {command_result.files_updated} commands updated")


These log strings include Unicode box-drawing characters ("└" and "─"), which violates the repo's ASCII-only encoding rule for source/CLI output (see .github/instructions/encoding.instructions.md). Please switch this message prefix to plain ASCII (for example, a simple "-" or the bracket status symbols) so Windows cp1252 terminals do not crash with UnicodeEncodeError.

These log strings match the existing pattern used throughout _integrate_package_primitives() — all ~15 existing _log_integration calls on lines 849, 864, 879, etc. use the same └─ character. Changing only the new lines would create an inconsistency. If there's a repo-wide ASCII encoding requirement, I'm happy to raise a separate PR to update all occurrences consistently.

Copilot · 2026-03-24T18:48:19Z

+    if integrate_claude:
+        command_result = command_integrator.integrate_package_commands(
+            package_info, project_root,
+            force=force, managed_files=managed_files,
+            diagnostics=diagnostics,


Please add/adjust a unit test to cover this regression: when the detected/configured target yields integrate_claude=False (e.g., target=vscode/copilot), apm install should not call integrate_package_commands and should not create/deploy anything under .claude/commands/. This will prevent future accidental reintroduction of unconditional Claude command integration.

Done — added TestIntegratePackagePrimitivesTargetGating in test_command_integrator.py with two tests: one verifying integrate_package_commands is not called when integrate_claude=False (target=copilot/vscode), and one verifying it is called when integrate_claude=True.

…mands/ Covers the fix in _integrate_package_primitives(): - When integrate_claude=False (target=copilot/vscode), integrate_package_commands must not be called and .claude/commands/ must not be created. - When integrate_claude=True (target=claude/all), integrate_package_commands must be called.

danielmeppiel

Architectural Review

Direction: Correct. The fix is needed and well-scoped.

I audited all 13 integrator calls in _integrate_package_primitives(). This is the only remaining unguarded call -- every other non-universal integrator is protected by either an external if integrate_* gate or an internal self-guard (e.g., OpenCode commands checks .opencode/ exists at L197-202). The .claude/commands/ block at L974 has neither, so it creates .claude/ from nothing via mkdir(parents=True) when integrate_vscode=True but integrate_claude=False.

Our recent presence-only target detection changes (merged in a1ae126) do NOT prevent this -- the early return at L846 only fires when ALL flags are False, which doesn't apply here.

Required

Rebase onto current main. install.py has had 4+ significant commits since this PR was opened (target registry refactor, presence-only detection, review fixes). The PR will not merge cleanly as-is.

Suggested (not blocking)

Add a self-guard inside integrate_package_commands() for defense-in-depth, matching the pattern in integrate_package_commands_opencode():

# At the top of integrate_package_commands():
claude_dir = project_root / ".claude"
if not claude_dir.is_dir():
    return IntegrationResult(
        files_integrated=0, files_updated=0,
        files_skipped=0, target_paths=[], links_resolved=0,
    )

Consider using tmp_path in new tests (optional -- current file uses tempfile.mkdtemp() consistently, so either convention is fine).

Tests look good. Regression risk is very low -- existing Claude users are unaffected since integrate_claude=True when .claude/ exists.

…443) * fix: gate .claude/commands/ deployment behind integrate_claude flag The CommandIntegrator was called unconditionally in _integrate_package_primitives(), ignoring the integrate_claude flag derived from the detected or configured target. This caused .claude/commands/ to be created on every apm install, even when the project is configured with target: copilot (vscode), which sets integrate_claude=False. The docstring of should_integrate_claude() explicitly states that 'commands' should be governed by this flag, matching the pattern already applied to Claude agents and hooks. The OpenCode commands integrator has a similar self-guard (.opencode/ must exist). Fix: wrap integrate_package_commands() in if integrate_claude: so it is consistent with all other Claude-specific integration paths. Reproducer: set target: copilot in apm.yml, add .prompt.md files under .apm/prompts/, run apm install — .claude/commands/ is created despite no Claude target being configured. * test: add regression tests for integrate_claude gating of .claude/commands/ Covers the fix in _integrate_package_primitives(): - When integrate_claude=False (target=copilot/vscode), integrate_package_commands must not be called and .claude/commands/ must not be created. - When integrate_claude=True (target=claude/all), integrate_package_commands must be called. * test: fix regression test import --------- Co-authored-by: Daniel Meppiel <51440732+danielmeppiel@users.noreply.github.com>

neop26 requested a review from danielmeppiel as a code owner March 24, 2026 18:43

Copilot AI review requested due to automatic review settings March 24, 2026 18:43

Copilot started reviewing on behalf of neop26 March 24, 2026 18:45 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

neop26 and others added 3 commits March 25, 2026 08:27

test: fix regression test import

281f98b

Merge branch 'main' into fix/command-integrator-respects-target-flag

a7fa08d

danielmeppiel requested changes Mar 26, 2026

View reviewed changes

Merge branch 'main' into fix/command-integrator-respects-target-flag

7dc71c5

danielmeppiel merged commit fdd0003 into microsoft:main Mar 26, 2026
6 checks passed

danielmeppiel mentioned this pull request Mar 27, 2026

chore: bump to v0.8.6 #474

Merged

github-actions Bot mentioned this pull request Mar 28, 2026

[ca] CLI Version Update: MCP Gateway v0.2.7, APM v0.8.6 github/gh-aw#23313

Closed

This was referenced Mar 28, 2026

Bump MCP Gateway v0.2.6→v0.2.7 and APM v0.8.5→v0.8.6 github/gh-aw#23321

Merged

Add open source contributions section to profile README neop26/neop26#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: gate .claude/commands/ deployment behind integrate_claude flag#443

fix: gate .claude/commands/ deployment behind integrate_claude flag#443
danielmeppiel merged 5 commits into
microsoft:mainfrom
neop26:fix/command-integrator-respects-target-flag

neop26 commented Mar 24, 2026 •

edited

Loading

Uh oh!

neop26 commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

neop26 Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

neop26 Mar 24, 2026 •

edited

Loading

Uh oh!

danielmeppiel left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

neop26 commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Testing

Uh oh!

neop26 commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

neop26 Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

neop26 Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielmeppiel left a comment

Choose a reason for hiding this comment

Architectural Review

Required

Suggested (not blocking)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

neop26 commented Mar 24, 2026 •

edited

Loading

neop26 Mar 24, 2026 •

edited

Loading