Fix --target intellij validation (#1957)

[sergio-sisternes-epam]

Why

apm install --target intellij rejects intellij as an unknown target at the CLI parser layer, even though the IntelliJ client adapter, factory registration, auto-detection logic, MCP install/cleanup code, and documentation all exist and work correctly. The --runtime intellij legacy flag works fine because it bypasses target validation entirely.

The root cause: intellij is absent from VALID_TARGET_VALUES in target_detection.py. This frozenset is the union of ALL_CANONICAL_TARGETS, EXPERIMENTAL_TARGETS, EXPLICIT_ONLY_TARGETS, and TARGET_ALIASES -- but intellij was in none of them.

Approach

Introduces a MCP_ONLY_TARGETS frozenset for pseudo-targets that have a client adapter but no KNOWN_TARGETS entry (they map to a canonical target for primitive deployment via RUNTIME_TO_CANONICAL_TARGET). This new set is:

• Included in VALID_TARGET_VALUES so the CLI parser accepts --target intellij
• Allowed in the "all,<target>" combination check (same pattern as all,agent-skills)
• Excluded from "all" expansion (intellij is MCP-only, not a full harness target)
• Forwarded through direct --mcp installs so JetBrains Copilot's user-scope mcp.json is written
• Normalized to copilot for organization policy evaluation

apm-spec-waiver: This restores documented IntelliJ behavior through existing target and MCP extension points; it does not extend the OpenAPM specification.

Tests added

• intellij presence in VALID_TARGET_VALUES
• Single --target intellij parses successfully
• Multi-target intellij,claude accepted
• all,intellij combination allowed
• Real subprocess CLI/install flow writes the OS-specific JetBrains Copilot mcp.json
• MCP-only policy normalization and mapping invariants

Scenario evidence

Scenario (user words)	APM principle	Test path
apm install --mcp NAME --target intellij exits successfully and writes JetBrains Copilot's OS-specific mcp.json with the requested server.	Multi-harness support, DevX	tests/integration/test_intellij_target.py::TestIntelliJCliE2E::test_mcp_install_writes_intellij_config
An organization policy that allows copilot also permits the MCP-only intellij target.	Governed by policy	tests/unit/install/test_policy_target_check_phase.py::TestTargetOverride::test_intellij_override_is_normalized_to_copilot
Every MCP-only target has a canonical policy mapping.	Governed by policy	tests/integration/test_intellij_target.py::TestIntelliJConstants::test_all_mcp_only_targets_have_canonical_mapping

Mutation-break evidence: removing direct --target forwarding caused the subprocess integration test to fail because JetBrains Copilot's mcp.json was not written; restoring forwarding returned the test to green.

Fixes: #1957

[Copilot]

Pull request overview

This PR fixes apm install --target intellij being rejected at the parser/validation layer by adding intellij to the set of accepted --target tokens, while keeping it excluded from plain "all" expansion (as an MCP-only pseudo-target). It also adds unit tests to prevent regressions.

Changes:

• Introduce MCP_ONLY_TARGETS = {"intellij"} and include it in VALID_TARGET_VALUES.
• Allow "all,intellij" in parse_target_field() by exempting MCP-only targets from the "all" cannot be combined restriction.
• Add unit tests covering intellij acceptance (single, multi-target, and all,intellij).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/apm_cli/core/target_detection.py	Adds MCP_ONLY_TARGETS and updates target validation + "all" combination logic to accept intellij.
tests/unit/core/test_target_detection.py	Adds regression tests ensuring intellij is accepted by the --target parsing/validation flow.

[github-actions]

APM Review Panel: ship_with_followups

Fixes --target intellij CLI validation so JetBrains MCP routing is no longer silently broken; panel unanimous with no blocking findings, five follow-ups led by a policy-gate alias miss and a discovery gap.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

PR #2041 closes issue #1957 by introducing MCP_ONLY_TARGETS = frozenset({"intellij"}) in target_detection.py and threading it through VALID_TARGET_VALUES and parse_target_field. The abstraction is correct, the all-expansion exclusion follows the established copilot-cowork precedent, and the added parser-layer unit tests pass. Auth and performance experts were correctly inactive. No panelist raised a blocking finding. This is a net-positive community contribution from an EPAM engineer who hit real friction with a documented but broken feature.

Three convergent panel signals warrant pre-merge attention. First, and sharpest: the supply-chain-security-expert identifies that policy_target_check.py passes the raw string 'intellij' to _check_compilation_target without aliasing through RUNTIME_TO_CANONICAL_TARGET. An org with allow: [copilot] in its policy will see --target intellij falsely blocked even though intellij resolves to copilot. This is fail-closed and safe in the threat sense, but it is a correctness regression for exactly the enterprise segment this EPAM contribution signals, and it should be resolved before the fix is considered complete at the policy layer. Second, cli-logging-expert and devx-ux-expert independently flag that the --target help text has no mention of intellij, meaning a user running apm install --help will conclude the flag is invalid and fall back to --runtime intellij -- the legacy path this PR is designed to replace. The fix is two sentences of help text; the cost of not shipping it is that the primary discovery path for the feature remains broken. Third, doc-writer and oss-growth-hacker converge on a missing CHANGELOG entry; a community fix that closes a documented issue with no release attribution is wasted momentum.

The test-coverage-expert returned two evidence: outcome=missing findings. The all-expansion regression trap (test_intellij_not_in_all_canonical_targets) is tagged governed-by-policy and protects the invariant that --target all never silently deploys to IntelliJ -- a promise the PR prose asserts but no automated lock currently enforces. The integration test gap covers the full apm install --target intellij CLI path; hermes and openclaw have integration tests, intellij has none, and an integration test is the tier-floor evidence for a CLI-validation bug fix. The python-architect's apm.yml asymmetry -- singular target: intellij now accepted, plural targets: [intellij] still fails at CANONICAL_TARGETS -- is lower urgency given the PR's stated scope, but the maintainer should document the intentional split or resolve it in a fast follow to avoid surprising users authoring multi-target apm.yml files.

Dissent. doc-writer rates the ide-tool-integration.md update (replacing --runtime intellij with --target intellij in the JetBrains guide example) as RECOMMENDED; devx-ux rates it NIT. I side with doc-writer: a guide that teaches the legacy flag for the one target this PR fixes is user-visible drift, not cosmetic. It should land with or immediately after this PR, not deferred indefinitely.

Aligned with: portable-by-manifest -- Partial: singular target: intellij in apm.yml now accepted via parse_target_field; plural targets: [intellij] still fails against CANONICAL_TARGETS in apm_yml.py -- a newly visible asymmetry. secure-by-default -- Preserved: no validation bypass introduced; the policy gate is fail-closed so no privilege escalation is possible; the alias normalization gap produces correctness failures, not security failures. governed-by-policy -- Gap: policy_target_check.py passes raw 'intellij' rather than canonical 'copilot' to _check_compilation_target; orgs with allow: [copilot] policies incorrectly see --target intellij rejected. multi-harness-multi-host -- Satisfied: IntelliJ MCP adapter, integrator, and RUNTIME_TO_CANONICAL_TARGET remap all pre-existed; this PR completes the integration at the flag validation surface. oss-community-driven -- Strong: EPAM engineer identifies and fixes a documented-but-broken JetBrains gap, demonstrating active enterprise pickup. pragmatic-as-npm -- Partially satisfied: --target intellij is now accepted at parse time, but --help invisibility means the feature is not self-discoverable until the help-text fix lands.

Growth signal. JetBrains IDEs represent roughly 25 million active users; an EPAM engineer both hitting and fixing this gap is an early enterprise-adoption signal worth broadcasting. Recommended sequencing: (1) land the policy-gate alias fix and help-text update in this PR or as a same-day fast follow so the feature is actually usable end-to-end; (2) add the CHANGELOG [Unreleased] entry so the fix is attributable at release and issue #1957 reporters have a clear resolution signal; (3) add IntelliJ to the README hero tool list at the next refresh cycle; (4) consider a community announcement to JetBrains/Kotlin/Java communities once discovery is unbroken. Do not amplify before the help-text gap closes -- announcing a feature users cannot find from --help is a community trust regression, not a growth event.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	MCP_ONLY_TARGETS is the right abstraction and correctly wired; one recommended follow-up on the target:/targets: YAML validator asymmetry; two nits on stale comment and undocumented invariant.
CLI Logging Expert	0	1	1	Help text omits intellij from Values: list; provenance silently shows copilot when --target intellij is used. No console helper regressions.
DevX UX Expert	0	1	2	Fix is functionally correct but --target help text still omits intellij, leaving the primary discovery path broken for the feature this PR ships.
Supply Chain Security Expert	0	1	1	No security bypass introduced; policy gate uses raw 'intellij' string instead of canonical 'copilot', causing fail-closed false positives under allow/enforce policies.
OSS Growth Hacker	0	1	1	Unblocks documented JetBrains target for ~25M JetBrains users; CHANGELOG entry missing, README hero still silent on IntelliJ.
Doc Writer	0	2	1	CHANGELOG [Unreleased] has no entry for #1957; ide-tool-integration.md still directs users to legacy --runtime intellij; targets-matrix omits intellij entirely.
Test Coverage Expert	0	2	1	Parser-layer unit tests are solid; two gaps: missing ALL_CANONICAL_TARGETS exclusion lock for intellij and no integration test for apm install --target intellij.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Supply Chain Security Expert] Apply RUNTIME_TO_CANONICAL_TARGET alias normalization in policy_target_check.py before passing to _check_compilation_target -- Orgs with allow: [copilot] policies see --target intellij falsely rejected today. Fail-closed is safe, but it breaks valid enterprise workflows for exactly the customer segment this EPAM contribution signals, and leaves the policy layer semantically inconsistent with the parser layer.
2. [CLI Logging Expert] Add intellij to the --target flag Values: help text in install.py with a note that it is MCP-only (devx-ux-expert convergent) -- The primary discovery path for the feature this PR ships is currently broken: --help does not mention intellij, causing users to conclude the flag is invalid and fall back to --runtime intellij. The fix is two sentences of help text and closes the entire discovery gap.
3. [Doc Writer] Add CHANGELOG [Unreleased] ### Fixed entry for --target intellij and update ide-tool-integration.md JetBrains example from --runtime to --target (oss-growth-hacker convergent) -- Consumers relying on CHANGELOG to track when issue [BUG] MCP server integration lacks IntelliJ as target #1957 is resolved will have no signal at release; the guide actively teaches the legacy flag for the one target this PR fixes.
4. [Test Coverage Expert] Add test_intellij_not_in_all_canonical_targets and test_all_expansion_excludes_intellij to test_target_detection.py (evidence: missing, governed-by-policy) -- The PR asserts intellij is excluded from --target all expansion but no automated guard enforces this invariant; a future contributor can silently break the promise.
5. [Test Coverage Expert] Create tests/integration/test_intellij_target.py mirroring test_hermes_target.py to exercise the full apm install --target intellij CLI-to-MCP-adapter path (evidence: missing, integration-with-fixtures tier required) -- Hermes and openclaw have integration tests; intellij has zero. Without it the fix can silently regress across refactors.

Architecture

classDiagram
    direction LR
    class TargetParamType {
        <<ClickParamType>>
        +convert(value, param, ctx) str or list
    }
    class parse_target_field {
        <<PureFunction>>
        +value str or list or None
        +source_path Path or None
    }
    class VALID_TARGET_VALUES {
        <<Frozenset>>
        union gate for all CLI tokens
    }
    class ALL_CANONICAL_TARGETS {
        <<Frozenset>>
        vscode claude cursor opencode codex gemini windsurf kiro
    }
    class MCP_ONLY_TARGETS {
        <<Frozenset>>
        intellij
    }
    class EXPLICIT_ONLY_TARGETS {
        <<Frozenset>>
        agent-skills antigravity
    }
    class EXPERIMENTAL_TARGETS {
        <<Frozenset>>
        copilot-cowork hermes openclaw
    }
    class RUNTIME_TO_CANONICAL_TARGET {
        <<Dict>>
        intellij maps to copilot
    }
    class IntelliJClientAdapter {
        <<MCPAdapter>>
        +target_name intellij
        +install_mcp_config()
    }
    class parse_targets_field {
        <<PureFunction>>
        apm_yml.py plural targets validator
    }
    class CANONICAL_TARGETS {
        <<Frozenset>>
        apm_yml.py no intellij entry
    }
    TargetParamType ..> parse_target_field : delegates
    parse_target_field ..> VALID_TARGET_VALUES : validates each token
    VALID_TARGET_VALUES ..> ALL_CANONICAL_TARGETS : union
    VALID_TARGET_VALUES ..> MCP_ONLY_TARGETS : union
    VALID_TARGET_VALUES ..> EXPLICIT_ONLY_TARGETS : union
    VALID_TARGET_VALUES ..> EXPERIMENTAL_TARGETS : union
    parse_target_field ..> EXPLICIT_ONLY_TARGETS : all-gate subtract
    parse_target_field ..> MCP_ONLY_TARGETS : all-gate subtract
    IntelliJClientAdapter ..> RUNTIME_TO_CANONICAL_TARGET : intellij maps to copilot
    parse_targets_field ..> CANONICAL_TARGETS : validates against
    class MCP_ONLY_TARGETS:::touched
    class parse_target_field:::touched
    class VALID_TARGET_VALUES:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    CLI["CLI: apm install --target intellij"] --> TPC["TargetParamType.convert\ntarget_detection.py"]
    TPC --> PTF["parse_target_field\ntarget_detection.py"]
    PTF --> VAL{"token in VALID_TARGET_VALUES?"}
    VAL -- "pre-PR: intellij absent" --> ERR["BadParameter raised"]
    VAL -- "post-PR: HIT via MCP_ONLY_TARGETS" --> SINGLE["return 'intellij' token"]
    SINGLE --> AT["active_targets(root, 'intellij')"]
    AT --> RTCA["RUNTIME_TO_CANONICAL_TARGET.get('intellij') = 'copilot'"]
    RTCA --> KT["KNOWN_TARGETS.get('copilot') -> copilot TargetProfile"]
    KT --> FS1["primitives deployed to .github/"]
    SINGLE --> RTR["_resolve_target_runtimes explicit_target='intellij'"]
    RTR --> GATE["_gate_project_scoped_runtimes intellij->copilot in active set"]
    GATE --> FS2["IntelliJClientAdapter.install_mcp_config"]
    SINGLE --> ALLP["all,intellij: non_all_tokens - EXPLICIT_ONLY - MCP_ONLY == empty"]
    ALLP --> EXP["return sorted(ALL_CANONICAL_TARGETS) + ['intellij']"]
    EXP --> DEDUP["active_targets: intellij->copilot deduped vscode->copilot already in seen set"]

Loading

Recommendation

Merge once the policy-gate alias fix (supply-chain finding, policy_target_check.py) and the --target help-text update (cli-logging and devx-ux convergent, install.py) are included in this PR or committed as same-day fast follows -- these two items directly gate enterprise correctness and feature discoverability for the capability being shipped. The CHANGELOG entry, ide-tool-integration.md doc update, and the two missing test-coverage items (all-expansion regression trap tagged governed-by-policy, and the integration test) should land in the same release window. The python-architect apm.yml plural-key asymmetry can trail as a documented follow-up issue. The core fix is correct and the PR author has earned a clean merge; close the discovery and policy gaps first.

---

Full per-persona findings

Python Architect

• [recommended] targets: [intellij] (plural YAML key) still rejected by apm_yml.CANONICAL_TARGETS after this fix at src/apm_cli/core/apm_yml.py:25
  After this PR the singular target: intellij in apm.yml is accepted via parse_target_field, but the plural targets: [intellij] still fails with UnknownTargetError because parse_targets_field validates against CANONICAL_TARGETS in apm_yml.py, which has no 'intellij' entry. Before this PR both forms failed; after this PR only the plural form fails -- a newly visible asymmetry.
  Suggested: Either add 'intellij' to CANONICAL_TARGETS in apm_yml.py, or add a note to MCP_ONLY_TARGETS docstring documenting the intentional split between the two validation paths.
• [nit] Stale comment at target_detection.py:630 says 'explicit-only ones' after MCP_ONLY_TARGETS is also now combinable with all at src/apm_cli/core/target_detection.py:630
  The comment still refers only to explicit-only tokens; a reader implementing a new MCP-only target would miss the established pattern.
  Suggested: Change to: '# "all" + explicit-only or MCP-only tokens (e.g. "all,agent-skills", "all,intellij"): expand "all" to canonical targets and append the pass-through tokens.'
• [nit] active_targets() all-branch silently relies on invariant that no MCP_ONLY_TARGETS member has a KNOWN_TARGETS entry at src/apm_cli/integration/targets.py:1147
  If a future contributor accidentally adds an MCP-only name to KNOWN_TARGETS, it would silently appear in 'all' expansions without any guard. A one-line comment would make the contract explicit.
  Suggested: Add before line 1147: '# MCP_ONLY_TARGETS members (e.g. intellij) have no KNOWN_TARGETS entry by contract; no filter needed here.'

CLI Logging Expert

• [recommended] --target help text Values: list does not include intellij at src/apm_cli/commands/install.py:945
  The --target option in install.py has a hard-coded 'Values:' sentence. intellij is now accepted by VALID_TARGET_VALUES but is invisible in the help string. A user running 'apm install --help' will not find intellij and will fall back to --runtime intellij -- the legacy path this PR is designed to replace.
  Suggested: Append to the Values: sentence: "'intellij' is also accepted (MCP-only: routes to the JetBrains Copilot MCP config; does not deploy harness primitives)."
• [nit] Provenance line silently shows 'copilot' when user passes --target intellij at src/apm_cli/install/phases/targets.py:379
  The intellij->copilot remap via RUNTIME_TO_CANONICAL_TARGET is invisible in the '[i] Targets: copilot (source: --target flag)' output. In verbose mode, a one-liner makes the mapping explicit without cluttering default output.
  Suggested: After _normalize_runtime_target_aliases, if any token was remapped and logger.is_verbose, emit a verbose-detail log line: 'intellij resolved to copilot (MCP-only alias)'.

DevX UX Expert

• [recommended] --target help text does not list intellij in the Values enumeration at src/apm_cli/commands/install.py:945
  The --target flag's inline help string lists values with no mention of intellij. Meanwhile --runtime help text explicitly lists intellij. A user reading 'apm install --help' will conclude --target intellij is invalid and fall back to --runtime intellij, exactly the legacy path this PR is trying to replace.
  Suggested: Append 'intellij' to the Values clause. A parenthetical such as '(intellij is MCP-only; maps to the copilot harness)' gives just enough context.
• [nit] ide-tool-integration.md JetBrains example still uses deprecated --runtime intellij at docs/src/content/docs/integrations/ide-tool-integration.md:144
  The guide teaches the old form for the one target this PR fixes. Now that --target intellij works, the guide actively directs users to the legacy path.
  Suggested: Change the example to 'apm install --target intellij (package)' and update the prose note at line 152 to recommend --target over --runtime.
• [nit] 'all,intellij' is allowed but the help text gives no hint this combination exists at src/apm_cli/commands/install.py:945
  The --target help text mentions 'all,agent-skills' and 'all,antigravity' but is silent about intellij combinations. The pattern 'all,(mcp-only)' is now real and undocumented inline.
  Suggested: Extend the 'all' explanation: 'combine with agent-skills, antigravity, or intellij to add them (e.g. all,intellij adds the JetBrains MCP config on top of all harness targets).'

Supply Chain Security Expert

• [recommended] policy_target_check uses raw target_override ('intellij') without alias canonicalization, producing false-positive blocks under copilot-restricting org policies at src/apm_cli/install/phases/policy_target_check.py:58
  effective_target is taken directly from ctx.target_override ('intellij'), and synthetic_yml = {'target': 'intellij'} is passed to _check_compilation_target. An org with allow: ['copilot'] will see --target intellij falsely rejected even though intellij resolves to copilot. Fail-closed is secure, but it is a correctness regression for any org whose policy allows 'copilot'.
  Suggested: Apply RUNTIME_TO_CANONICAL_TARGET.get(effective_target, effective_target) normalization before building synthetic_yml in policy_target_check.py.
• [nit] MCP_ONLY_TARGETS has no enforced contract requiring a RUNTIME_TO_CANONICAL_TARGET entry per element at src/apm_cli/core/target_detection.py:424
  A future contributor adding a new MCP-only target without a RUNTIME_TO_CANONICAL_TARGET entry would silently produce a self-mapping target, potentially causing the targets phase to skip primitive deployment without error.
  Suggested: Add a comment after the frozenset definition asserting: '# Invariant: every MCP_ONLY_TARGET must have a RUNTIME_TO_CANONICAL_TARGET entry.'

OSS Growth Hacker

• [recommended] No CHANGELOG entry for this fix -- JetBrains users who hit [BUG] MCP server integration lacks IntelliJ as target #1957 will not know it is resolved at CHANGELOG.md
  The [Unreleased] block in CHANGELOG.md is currently empty. This is a discoverable community fix that closes a documented-but-broken feature for a massive IDE ecosystem -- it earns a ### Fixed line.
  Suggested: Add under [Unreleased] ### Fixed: '- apm install --target intellij now accepted by the CLI parser; intellij was absent from VALID_TARGET_VALUES despite the adapter and docs all working correctly. Introduces MCP_ONLY_TARGETS for pseudo-targets that map to a canonical runtime but are not full harness targets. (by @sergio-sisternes-epam, closes [BUG] MCP server integration lacks IntelliJ as target #1957)'
• [nit] README hero line omits IntelliJ/JetBrains -- silent to the JetBrains community at the top of the funnel at README.md
  Now that --target intellij works and is documented, adding IntelliJ to the hero line converts JetBrains visitors who scan the top 30 lines looking for their IDE.
  Suggested: Append 'IntelliJ' to the hero tool list in README.md.

Auth Expert -- inactive

The changed files (target_detection.py, test_target_detection.py) are not in the auth activation list, and the fallback self-check confirms the intellij adapter and MCP integrator contain no auth flows, credential resolution, or token management code affected by adding 'intellij' to VALID_TARGET_VALUES.

Doc Writer

• [recommended] CHANGELOG [Unreleased] has no entry for this bug fix at CHANGELOG.md
  The [Unreleased] section is empty. Consumers relying on CHANGELOG to know when a bug is fixed will have no signal for issue [BUG] MCP server integration lacks IntelliJ as target #1957.
  Suggested: Add under [Unreleased] ### Fixed: '- apm install --target intellij no longer fails with an unknown-target error; intellij is now included in VALID_TARGET_VALUES as an MCP-only target. (closes [BUG] MCP server integration lacks IntelliJ as target #1957) (Fix --target intellij validation (#1957) #2041)'
• [recommended] ide-tool-integration.md directs users to the legacy --runtime flag for explicit IntelliJ targeting at docs/src/content/docs/integrations/ide-tool-integration.md
  Lines 144 and 152 show the legacy --runtime intellij form. Leaving the doc pointing to the legacy flag is drift now that --target intellij is the correct path.
  Suggested: Update the JetBrains code example to apm install --target intellij <package>. Update the note at line 152 to: 'Use --target intellij to deploy to JetBrains explicitly; --runtime intellij is a legacy alias that remains functional.'
• [nit] targets-matrix.md has no entry for intellij at docs/src/content/docs/reference/targets-matrix.md
  Every other valid --target value has a row or prose mention. intellij appears nowhere in the matrix.
  Suggested: Add a sentence to the MCP/experimental prose block: 'intellij is an MCP-only target (no deploy root for instructions, prompts, skills, or agents); it writes to the JetBrains Copilot user-scope mcp.json and is excluded from --target all expansion.'

Test Coverage Expert

• [recommended] No regression-trap asserting intellij is excluded from --target all expansion at tests/unit/core/test_target_detection.py
  The PR asserts intellij is 'excluded from all expansion'. The copilot-cowork precedent establishes a named pattern: every target-category split must have a constant-split guard. The existing test_all_combined_with_intellij_allowed asserts the combination IS allowed, not that bare all excludes intellij. Grep of test file confirmed: no test for 'intellij not in ALL_CANONICAL_TARGETS'.
  Suggested: Add: test_intellij_not_in_all_canonical_targets (asserts 'intellij' not in ALL_CANONICAL_TARGETS) and test_all_expansion_excludes_intellij (asserts 'intellij' not in normalize_target_list('all')).
  Proof (missing at unit): tests/unit/core/test_target_detection.py::test_intellij_not_in_all_canonical_targets -- proves: '--target all' never silently deploys to IntelliJ; intellij is opt-in only [devx, governed-by-policy]
• [recommended] No integration test exercises apm install --target intellij through the CLI runner at tests/integration/test_intellij_target.py
  This PR fixes a CLI validation rejection bug. Tier-floor for CLI surface changes requires integration-with-fixtures evidence. Hermes and openclaw have integration tests (test_hermes_target.py, test_openclaw_target.py); grep of tests/integration/ for 'intellij' returned zero matches.
  Suggested: Create tests/integration/test_intellij_target.py mirroring test_hermes_target.py with a CliRunner-backed test: result = runner.invoke(cli, ['install', '--target', 'intellij', '--global']); assert result.exit_code == 0.
  Proof (missing at integration-with-fixtures): tests/integration/test_intellij_target.py::TestIntelliJParserE2E::test_target_intellij_accepted_and_exits_zero -- proves: 'apm install --target intellij' routes to the IntelliJ MCP adapter and exits 0 rather than 'unknown target' [devx, multi-harness-support]
• [nit] test_all_combined_with_intellij_allowed uses parse_target_field directly rather than TargetParamType.convert, inconsistent with parallel all-combination tests at tests/unit/core/test_target_detection.py
  The other 'all,X' tests use self.tp.convert. Minor -- parse_target_field IS the shared implementation so coverage is functionally equivalent.
  Suggested: Change to use self.tp.convert('all,intellij', None, None) for consistency with the test class pattern.

Performance Expert -- inactive

No performance-sensitive paths touched. Change is limited to a module-level frozenset literal constructed once at import time, one frozenset union at import time, and one additional frozenset difference inside the CLI argument parser -- all O(1) or O(n) with n equal to the number of user-supplied target tokens. None of the four install phases (Resolve, Fetch, Materialize, Verify) are touched.

_{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 #2041 · 722.1 AIC · ⌖ 17.8 AIC · ⊞ 5.5K · ◷

[danielmeppiel]

APM Review Panel: ship_with_followups

IntelliJ target validation is directionally right, but the direct MCP install path and regression evidence need one fold-in pass.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

The panel found that parser acceptance alone does not fulfill issue #1957: the direct --mcp ... --target intellij flow was not forwarding the selected target to MCP integration, the documented command shape was invalid, and no empirical install test proved that JetBrains Copilot config was written. The remaining architecture, policy, help, and reference-doc suggestions are small and in scope.

Aligned with: portable-by-manifest, multi-harness support, pragmatic package-manager UX.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	1	Remove duplicated IntelliJ mapping truth.
CLI Logging Expert	0	0	1	Surface policy normalization in verbose output.
DevX UX Expert	0	1	2	Keep CLI references and help discoverable.
Supply Chain Security Expert	0	0	1	Fail closed if an MCP-only mapping is missing.
OSS Growth Hacker	0	0	2	Use user-facing release wording.
Doc Writer	1	1	1	Correct the command and target references.
Test Coverage Expert	0	2	0	Add real CLI/install evidence and policy coverage.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Doc Writer] (blocking-severity) Forward --target intellij through direct MCP installation and correct the documented command.
2. [Test Coverage Expert] Add an empirical subprocess test that writes the IntelliJ MCP config.
3. [Test Coverage Expert] Exercise IntelliJ-to-Copilot policy normalization.
4. [Python Architect] Remove duplicated hardcoded IntelliJ mapping.
5. [Supply Chain Security Expert] Replace the fail-open canonical mapping fallback.

Recommendation

Fold these bounded changes into this PR, then re-run the panel against the user-observable install flow.

---

Full per-persona findings

The highest-signal evidence is tests/integration/test_intellij_target.py: it invokes the CLI without an MCP name and only checks that an error string is absent. It does not execute MCP integration or assert that github-copilot/intellij/mcp.json is written. The docs example also supplies no value to --mcp. Other findings concern duplicated mapping logic, policy mapping fallback, verbose diagnostics, and reference coverage.

_{This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.}

[danielmeppiel]

APM Review Panel: needs_rework

PR #2041 fixes single-target --target intellij validation, but this SHA is not ready to ship because multi-target installs can crash and policy enforcement still misses resolved-target paths.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

Panel convergence is strong, not split. The single-target IntelliJ path is real on this commit: the subprocess integration test proves exit 0, the OS-specific config write, and the requested server URL. The unit policy test also proves scalar intellij normalizes to copilot.

But the reproduced list-valued target_override TypeError, plus the finding that direct --mcp preflight and plural-target policy paths do not consistently consume the resolved target set, outweighs the green scalar evidence. Users will naturally try intellij,claude, and org admins will assume compilation.target.allow applies uniformly regardless of install path. Security and correctness should win over optimistic UX here.

Aligned with: OSS community-driven intent is strong, but secure-by-default, governed-by-policy, multi-harness, and pragmatic package-manager behavior require another correction pass.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	1	0	0	Multi-target policy checks still crash on list input.
CLI Logging Expert	1	1	0	Multi-target policy output crashes before structured diagnostics.
DevX UX Expert	1	0	0	Explicit IntelliJ selection may touch adjacent Copilot runtimes.
Supply Chain Security Expert	2	0	0	Direct MCP and plural-target policy paths are not uniformly governed.
OSS Growth Hacker	0	0	0	Adoption surfaces are otherwise clean.
Doc Writer	0	1	0	Auto-detection docs still overstate the non-MCP contract.
Test Coverage Expert	0	2	0	Scalar flow is proven; composed and governed flows lack integration proof.
Performance Expert	0	0	0	No performance defect found.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Supply Chain Security Expert] (blocking-severity) Unify direct --mcp preflight and post-target policy evaluation around the same resolved target set, including plural targets and MCP-only canonical mappings. -- compilation.target.allow must apply uniformly across install paths.
2. [Python Architect] (blocking-severity) Make target handling shape-stable so both str and list[str] forms are normalized before membership checks or policy evaluation. -- The reproduced TypeError turns valid multi-target input into a runtime crash.
3. [DevX UX Expert] (blocking-severity) Constrain explicit --target intellij installs to the intended JetBrains Copilot MCP side effects. -- A specific client target should not write adjacent client config.
4. [Test Coverage Expert] Add CLI coverage for --target intellij,claude, --target all,intellij, and plural-target policy enforcement. -- Existing passing evidence is scalar-only.
5. [Doc Writer] Reconcile target detection documentation after runtime behavior is corrected. -- Public claims must match the final install contract.

Architecture

flowchart TD
    A[\"apm install --mcp NAME --target intellij\"] --> B[\"TargetParamType validation\"]
    B --> C[\"_handle_mcp_install target=intellij\"]
    C --> D[\"run_mcp_install\"]
    D --> E[\"MCPIntegrator.install runtime=intellij explicit_target=intellij\"]
    E --> F[\"JetBrains mcp.json write\"]
    A --> G[\"policy_target_check\"]
    G --> H{\"target shape\"}
    H -->|scalar| I[\"intellij to copilot\"]
    H -->|list| J[\"TypeError before policy result\"]

Loading

Recommendation

Do not ship this SHA yet. First fix target-shape and policy-path inconsistencies so direct --mcp, scalar targets, and plural targets share one resolved-target contract. Then verify explicit IntelliJ selection only touches intended config, add composed-flow regression coverage, and rerun the panel.

---

Full per-persona findings

Python Architect

• [blocking] policy_target_check crashes on valid IntelliJ-containing target lists at src/apm_cli/install/phases/policy_target_check.py:74.
  effective_target in MCP_ONLY_TARGETS hashes a list and raises TypeError before policy evaluation.

CLI Logging Expert

• [blocking] Multi-target installs crash before structured policy output at src/apm_cli/install/phases/policy_target_check.py:74.
• [recommended] Legacy vscode allowlists may not be equivalent to copilot for IntelliJ policy normalization.

DevX UX Expert

• [blocking] Explicit IntelliJ selection can retain other runtimes whose canonical target is Copilot at src/apm_cli/integration/mcp_integrator.py:1068.

Supply Chain Security Expert

• [blocking] Direct targeted MCP installs can miss compilation.target preflight at src/apm_cli/commands/install.py:845.
• [blocking] Policy target checks ignore resolved plural targets at src/apm_cli/install/phases/policy_target_check.py:57.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

No authentication, token, credential, host-classification, header, or fallback surface changed.

Doc Writer

• [recommended] The targets matrix overstates IntelliJ auto-detection outside MCP runtime discovery.

Test Coverage Expert

• [recommended] IntelliJ target combinations lack real CLI integration coverage.
• [recommended] The Copilot allowlist promise remains unit-tier only.

Performance Expert

No findings.

_{This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.}

[danielmeppiel]

APM Review Panel: ship_now

PR #2041 adds user-facing IntelliJ target support to the APM CLI, broadening the multi-harness surface with clean architecture and full test coverage at SHA 55914b7e0.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

The panel converges on a clean engineering signal. Python architecture raised cosmetic nits only. CLI logging raised one recommended wording adjustment from "runtimes" to "targets" plus one nit. DevX UX, supply-chain security, documentation, test coverage, and performance found no concerns. Auth was inactive because no auth surface changed. The growth persona failed schema validation after two retries, so its return was replaced by the skill-defined placeholder with no findings.

The evidence is strong: exact-head CI is green, the full canonical lint passed, 398 relevant tests passed with one skip, and mutation-break guards proved plural normalization, direct MCP policy forwarding, and exact explicit runtime selection. All findings from the prior needs-rework pass were folded. The remaining polish can follow separately.

Aligned with: portable by manifest: IntelliJ remains in the manifest-driven target model; multi-harness/multi-host: JetBrains IDE support broadens editor coverage; pragmatic as npm: existing install flows gain the target without extra ceremony.

Growth signal. IntelliJ-family IDE support is a concrete differentiator worth highlighting in release notes, especially as a community contribution.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	3	Clean MCP-only pseudo-target layering; shape-preserving policy normalization is sound.
CLI Logging Expert	0	1	1	Output is consistent overall; one new progress line uses internal "runtimes" wording.
DevX UX Expert	0	0	0	IntelliJ is discoverable and composed-target behavior is documented consistently.
Supply Chain Security Expert	0	0	0	No security concerns; direct and plural policy enforcement is preserved.
OSS Growth Hacker	0	0	0	Schema failure after two retries; skill-defined placeholder used.
Doc Writer	0	0	0	Final docs match code, including separate file and MCP discovery behavior.
Test Coverage Expert	0	0	0	Critical scalar, plural, direct-policy, and side-effect contracts are defended.
Performance Expert	0	0	0	No install-path performance regression detected.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 2 follow-ups

1. [CLI Logging Expert] Align the new user-facing "runtimes" wording to "targets" -- this matches the manifest and documentation vocabulary.
2. [Python Architect] Consider the cosmetic defensive-check and lazy-import-comment cleanups in a future maintenance pass -- they do not affect behavior.

Architecture

classDiagram
    direction LR
    class TargetParamType {
        +convert(value, param, ctx)
    }
    class target_detection {
        +detect_target(project_root, explicit_target, config_target)
        +normalize_policy_targets(value)
    }
    class InstallContext {
        +target_override: str | list | None
    }
    class PolicyTargetCheckPhase {
        +run(ctx)
    }
    class MCPIntegrator {
        +install(deps, runtime, explicit_target)
    }
    class InstallPreflight {
        +run_policy_preflight(effective_target)
    }
    TargetParamType ..> target_detection : parses
    PolicyTargetCheckPhase ..> target_detection : normalizes
    PolicyTargetCheckPhase ..> InstallContext : reads override
    MCPIntegrator ..> target_detection : resolves exact targets
    InstallPreflight ..> target_detection : normalizes

Loading

flowchart TD
    A[CLI target input] --> B[Parse scalar or list]
    B --> C[Normalize policy targets]
    C --> D{Policy allows all targets?}
    D -->|No| E[Stop install]
    D -->|Yes| F[Resolve exact explicit runtime set]
    F --> G[Write only selected MCP configurations]

Loading

Recommendation

Ship now. Exact-head checks are green, regression guards passed, all prior rework findings were folded, and the final panel found no correctness concern. Track the terminology adjustment as optional follow-up polish.

---

Full per-persona findings

Python Architect

• [nit] Redundant isinstance(explicit_target, str) guard in detect_target at src/apm_cli/core/target_detection.py:130
  The signature and guarded call site already constrain this value to str | None; the defensive check is harmless but can suggest a broader contract.
  Suggested: Simplify the membership check when next touching this helper.
• [nit] Lazy import rationale could live beside the import at src/apm_cli/core/target_detection.py:509
  The lazy import correctly avoids the target module cycle, but a local explanation would make that choice easier to preserve.
• [nit] The lightweight adapter is the right design at this scope
  Shape-preserving normalization cleanly maps the MCP-only target domain to canonical policy targets; a class hierarchy would be unnecessary today.

CLI Logging Expert

• [recommended] Progress message uses internal "runtimes" vocabulary at src/apm_cli/integration/mcp_integrator_install.py:357
progress() is user-facing, and IntelliJ is a target that maps to a runtime rather than a runtime itself.
  Suggested: Prefer Target: intellij or Targets: intellij, claude.
• [nit] The removed outer verbose guard is intentionally asymmetric at src/apm_cli/install/mcp/command.py:118
verbose_detail() already self-gates, so the change is correct; a future cleanup could make the surrounding guards visually consistent.

DevX UX Expert

No findings.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

No findings. The persona returned malformed output after two retries; the schema-defined placeholder was used.

Auth Expert -- inactive

No authentication, token, credential, host classification, or authorization header files are changed.

Doc Writer

No findings.

Test Coverage Expert

No findings.

Performance Expert

No findings.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}