Skip to content

Fix --target intellij validation (#1957)#2041

Open
sergio-sisternes-epam wants to merge 8 commits into
mainfrom
sergio-sisternes-epam-fix-intellij-target-validation
Open

Fix --target intellij validation (#1957)#2041
sergio-sisternes-epam wants to merge 8 commits into
mainfrom
sergio-sisternes-epam-fix-intellij-target-validation

Conversation

@sergio-sisternes-epam

@sergio-sisternes-epam sergio-sisternes-epam commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

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::test_intellij_target_is_normalized_to_copilot_for_policy
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 AI review requested due to automatic review settings July 5, 2026 23:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread src/apm_cli/core/target_detection.py
@sergio-sisternes-epam sergio-sisternes-epam added the panel-review Trigger the apm-review-panel gh-aw workflow label Jul 5, 2026
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

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 ·

@github-actions github-actions Bot removed the panel-review Trigger the apm-review-panel gh-aw workflow label Jul 5, 2026
sergio-sisternes-epam pushed a commit that referenced this pull request Jul 7, 2026
- Normalise intellij -> copilot in policy_target_check.py via
  RUNTIME_TO_CANONICAL_TARGET so org allow-lists are not falsely rejected
- Add intellij to --target help text in install.py with MCP-only note
- Add CHANGELOG entry under [Unreleased] Fixed
- Update ide-tool-integration.md from --runtime intellij to --target intellij
- Add constant-split guard tests: intellij not in ALL_CANONICAL_TARGETS,
  in MCP_ONLY_TARGETS, and all-expansion excludes intellij
- Create tests/integration/test_intellij_target.py with constant guards,
  CLI E2E parser acceptance, and policy normalisation tests

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sergio Sisternes and others added 4 commits July 9, 2026 19:59
Add intellij to VALID_TARGET_VALUES via a new MCP_ONLY_TARGETS set so
that `apm install --target intellij` passes the CLI parser. Previously
the adapter, factory registration, auto-detection, and MCP install logic
all existed but the --target flag rejected the token at parse time.

MCP-only pseudo-targets (like intellij) have a client adapter but no
KNOWN_TARGETS entry -- they map to a canonical target for primitive
deployment via RUNTIME_TO_CANONICAL_TARGET. The new MCP_ONLY_TARGETS
frozenset captures this category explicitly and is included in
VALID_TARGET_VALUES alongside the canonical, experimental, and
explicit-only sets.

Also allows `all,intellij` combination (same pattern as
`all,agent-skills`).

Closes #1957

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review feedback: detect_target() now maps explicit_target and
config_target 'intellij' to 'vscode' (same group as copilot/agents),
preventing silent fallthrough to auto-detect when --target intellij is
used with apm compile or apm pack.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Normalise intellij -> copilot in policy_target_check.py via
  RUNTIME_TO_CANONICAL_TARGET so org allow-lists are not falsely rejected
- Add intellij to --target help text in install.py with MCP-only note
- Add CHANGELOG entry under [Unreleased] Fixed
- Update ide-tool-integration.md from --runtime intellij to --target intellij
- Add constant-split guard tests: intellij not in ALL_CANONICAL_TARGETS,
  in MCP_ONLY_TARGETS, and all-expansion excludes intellij
- Create tests/integration/test_intellij_target.py with constant guards,
  CLI E2E parser acceptance, and policy normalisation tests

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
RUNTIME_TO_CANONICAL_TARGET maps vscode -> copilot too, so applying it
unconditionally broke existing tests that pass vscode as effective_target.
Guard the normalisation with an MCP_ONLY_TARGETS membership check so only
pseudo-targets like intellij are remapped.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam sergio-sisternes-epam force-pushed the sergio-sisternes-epam-fix-intellij-target-validation branch from 7d9659e to 6b7a0a2 Compare July 9, 2026 19:00
@danielmeppiel

Copy link
Copy Markdown
Collaborator

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 and others added 4 commits July 10, 2026 01:10
Forward --target through the direct MCP install path, prove the real CLI writes JetBrains Copilot configuration, and align policy, help, and reference documentation with the user-visible behavior. Addresses panel follow-ups for PR #2041.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Guard MCP-only membership checks for list-valued target inputs and retain the documented all-target expansion in CLI help.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make the subprocess regression select the worktree binary, harden policy mapping invariants, improve target diagnostics and typing, and complete IntelliJ documentation.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
apm-spec-waiver: This restores documented IntelliJ behavior through existing target and MCP extension points; it does not extend the OpenAPM specification.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] MCP server integration lacks IntelliJ as target

3 participants