fix(install): map APM prompt 'input' to Claude 'arguments' front-matter

[stbenjam]

Description

Slash commands installed from APM packages did not wire input: parameters through to Claude Code's native arguments: front-matter, so users got no argument hints when invoking the command. This is a blocker for teams rolling out APM -- every /command that expects arguments silently drops the hints.

Changes:

• Map input: (list, object-list, bare dict, single string) to arguments: in the compiled Claude command.
• Convert ${input:name} body references to $name placeholders.
• Auto-generate argument-hint from input names when not explicitly set.
• Move command-generation helpers out of ClaudeFormatter (compilation layer) into CommandIntegrator (integration layer) where they belong.
• Remove orphaned CommandGenerationResult dataclass (no production callers remained).
• Add defense-in-depth SecurityGate.scan_text() to the integrate_command() write path.
• Document input formats and target-specific mapping in three docs pages and the apm-usage skill resource.

Fixes # (issue)

N/A

Type of change

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

Testing

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

[Copilot]

Pull request overview

Fixes Claude Code slash command argument hints by mapping APM prompt input: front-matter into Claude command arguments: during install-time command integration, and by rewriting ${input:name} references in prompt bodies to Claude-style $name placeholders. This also removes the now-obsolete Claude command generation code from the compilation layer, adds/adjusts tests, and updates docs + changelog accordingly.

Changes:

• Map prompt input: -> Claude arguments: + auto-generate argument-hint (unless explicitly set) and rewrite ${input:name} -> $name in generated Claude command content.
• Move/remove Claude command generation helpers and the CommandGenerationResult dataclass from ClaudeFormatter (compilation layer).
• Add/adjust unit tests, documentation pages, skill resource docs, and a changelog entry.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/apm_cli/integration/command_integrator.py	Adds input->arguments mapping, placeholder rewriting, and a pre-write SecurityGate scan in the Claude command integration path.
src/apm_cli/compilation/claude_formatter.py	Removes Claude command generation responsibilities from the compilation layer.
tests/unit/integration/test_command_integrator.py	Adds unit + end-to-end coverage for input parsing and Claude command front-matter/content transformation.
tests/unit/compilation/test_claude_formatter.py	Removes tests that covered Claude command generation from ClaudeFormatter.
tests/unit/test_install_scanning.py	Removes tests tied to the deleted CommandGenerationResult dataclass.
docs/src/content/docs/guides/prompts.md	Documents accepted input: formats and Claude-specific mapping behavior.
docs/src/content/docs/integrations/ide-tool-integration.md	Documents Claude command mapping behavior for installs.
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md	Updates authoring guidance for prompt parameters and Claude mapping behavior.
CHANGELOG.md	Adds a Keep-a-Changelog entry under Fixed describing the Claude arguments hint fix.

[github-actions]

APM Review Panel Verdict: REJECT

Architecture is sound and the feature is strategically important, but two YAML-injection and fail-open security findings, a severity misfire in diagnostics, missing reference-doc coverage, a silent-mutation UX gap, and a misclassified CHANGELOG entry block merge.

Required before merge (10 items)

• [supply-chain-security-expert] Scan failure is fail-open: ImportError/OSError/ValueError silently skipped, file always written at src/apm_cli/integration/command_integrator.py

  • Why: If the security.gate module is absent or broken, the bare except (...): pass means scanning is silently skipped and the file is written unconditionally. ImportError in particular means a missing or tampered gate module becomes a no-op rather than a hard stop. This violates the Fail Closed principle.
  • Suggested fix: Re-raise ImportError so a missing gate is unrecoverable. At minimum: except ImportError: raise to fail closed when the gate itself cannot be loaded. Log a warning with context for OSError/ValueError rather than silencing them.

• [supply-chain-security-expert] Unsanitized package-supplied dict keys written verbatim into YAML frontmatter argument-hint field at src/apm_cli/integration/command_integrator.py

  • Why: _extract_input_names() accepts dict keys from untrusted package frontmatter with only a .strip() non-empty check. A malicious package can supply a key containing YAML-significant characters (newline, colon-space, quote) such as foo>\ninjected_key: injected_value, enabling injection of attacker-controlled YAML keys into the compiled output written to the user's disk. The arguments list suffers the same issue.
  • Suggested fix: Validate extracted names against a strict allowlist pattern (e.g. ^[A-Za-z][\w-]{0,63}$) and reject or skip any name that does not conform, emitting a warning.

• [cli-logging-expert] severity='warning' misfires: verdict.has_critical finding is downgraded to warning bucket at src/apm_cli/integration/command_integrator.py:194

  • Why: _render_security_group() splits on severity=='critical' vs 'warning'. Critical findings render red+bold with a count header; warnings render yellow with a softer message. The code checks verdict.has_critical but then passes severity='warning', so the finding silently lands in the yellow bucket, stripping the red/bold signal and suppressing per-file verbose detail. This actively hides critical security findings from users who rely on color and weight to triage output.
  • Suggested fix: Change severity='warning' to severity='critical' when verdict.has_critical is True. If the verdict exposes a severity string, use that directly.

• [cli-logging-expert] message and detail are identical strings -- detail field carries no additional value at src/apm_cli/integration/command_integrator.py:190

  • Why: DiagnosticCollector.security() accepts detail as a separate field so callers can put a short message in message (shown in all modes) and a longer context string in detail (shown in verbose mode). Passing the same warning string to both fields duplicates output in verbose mode and signals the caller did not think about the message/detail split.
  • Suggested fix: Pass message=target.name (or a short label) and detail with the actionable repair hint so the renderer can surface the fix only in verbose mode.

• [python-architect] diagnostics parameter typed as Any instead of Optional[DiagnosticCollector] at src/apm_cli/integration/command_integrator.py:151

  • Why: Using Any erases the contract: callers cannot discover the accepted type, type checkers cannot catch misuse, and the duck-typed hasattr(diagnostics, 'security') guard is fragile. DiagnosticCollector is already importable under TYPE_CHECKING in the same file.
  • Suggested fix: Add from apm_cli.utils.diagnostics import DiagnosticCollector inside the existing if TYPE_CHECKING: block, then change the signature to diagnostics: Optional[DiagnosticCollector] = None. Replace the hasattr guard with if diagnostics is not None:.

• [python-architect] Test fixtures use ${{input:name}} (double-brace) but docs canonicalize ${input:name} (single-brace) at tests/unit/integration/test_command_integrator.py:622

  • Why: Every user-facing example in docs shows single-brace ${input:name}. Test fixtures in TestInputToArgumentsEndToEnd and TestInputToArgumentsIntegration exclusively use double-brace ${{input:name}}. If double-brace is intentional, it must be documented. If it is a test-authoring accident, the tests do not exercise the primary documented format, leaving the most common user path untested.
  • Suggested fix: Add at least one fixture variant using single-brace ${input:name} to cover the documented canonical format. If double-brace is an intentional alternate, add a comment and a separate test class, and add a docs note under 'Input formats'.

• [devx-ux-expert] Silent content mutation during install violates 'install adds, never silently mutates' mental model at src/apm_cli/integration/command_integrator.py

  • Why: apm install rewrites ${input:name} to $name and injects arguments:/argument-hint into the installed file without any install-time feedback. The installed artifact is materially different from what the package author wrote. A user who inspects .claude/commands/foo.md will see different syntax than the source package with no explanation why.
  • Suggested fix: Emit a user-visible install-time message (not hidden behind --verbose) whenever input:-to-arguments mapping occurs, e.g. Mapped input: [feature_name, priority] to Claude arguments in review.prompt.md.

• [devx-ux-expert] cli-commands.md not updated; apm install behavior changed without reference-doc coverage at docs/src/content/docs/reference/cli-commands.md

  • Why: The reference doc for apm install is the canonical contract users and tool integrations rely on. This PR changes what install writes to disk (new frontmatter keys, rewritten body references). Users who already know apm install will not discover the input: mapping without reading the install integration guide specifically.
  • Suggested fix: Add a subsection under apm install in cli-commands.md describing that prompt files with input: frontmatter are transformed during install: arguments:/argument-hint are injected and ${input:name} references are rewritten to $name.

• [oss-growth-hacker] Feature is filed under 'Fixed' in CHANGELOG; should be 'Added' -- misclassification makes feature invisible to users scanning for new capabilities at CHANGELOG.md

  • Why: Users scan 'Added' to decide whether to upgrade. The most Claude-Code-relevant new capability in this cycle is invisible to existing users doing a quick changelog triage. 'Fixed' implies a regression was repaired; this feature did not exist before.
  • Suggested fix: Move the entry to ### Added.

• [oss-growth-hacker] CHANGELOG entry opens with mechanism not user outcome -- hook is buried at end of sentence at CHANGELOG.md

  • Why: A user scanning the changelog wants to know what changed for them, not which frontmatter keys got rewritten. The user-visible outcome is the subordinate clause at the end.
  • Suggested fix: Lead with the outcome: "Slash commands installed from APM packages now surface argument hints in Claude Code -- apm install automatically maps input: to Claude's arguments: frontmatter, rewrites ${input:name} references to $name, and auto-generates argument-hint."

Nits (14 items, skip if you want)

• [python-architect] integrate_command emits both message and detail with identical strings to diagnostics.security -- detail should carry more context than message at src/apm_cli/integration/command_integrator.py:190
• [python-architect] _extract_input_names docstring does not mention that bare-dict key order follows Python 3.7+ insertion order; YAML loading order should be noted at src/apm_cli/integration/command_integrator.py:51
• [python-architect] CommandIntegrationResult module-level alias has no all guard and leaks into star imports; prefer TypeAlias declaration at src/apm_cli/integration/command_integrator.py
• [python-architect] SecurityGate is imported inline inside integrate_command via a try/except; sibling module imports belong at the top of the file at src/apm_cli/integration/command_integrator.py:175
• [cli-logging-expert] Warning message leads with filename, not outcome; prefer "Critical hidden characters detected in {target.name}" over "{target.name}: critical hidden characters" at src/apm_cli/integration/command_integrator.py:178
• [cli-logging-expert] The shared warnings list mixes transform warnings and security findings then routes all through diagnostics.security(); non-security warnings will be miscategorized as CATEGORY_SECURITY at src/apm_cli/integration/command_integrator.py:188
• [cli-logging-expert] hasattr(diagnostics, 'security') duck-type check is fragile; prefer isinstance(diagnostics, DiagnosticCollector) at src/apm_cli/integration/command_integrator.py:189
• [devx-ux-expert] Four input: formats shown with equal weight in prompts.md creates choice paralysis; lead with the recommended simple-list format and collapse alternatives at docs/src/content/docs/guides/prompts.md
• [devx-ux-expert] Auto-generated argument-hint uses underscores (<feature_name>) where CLI tool conventions use hyphens (<feature-name>) in help text at src/apm_cli/integration/command_integrator.py
• [supply-chain-security-expert] WARN_POLICY on critical hidden chars warns but still writes the file; consider upgrading to BLOCK_POLICY for has_critical findings at src/apm_cli/integration/command_integrator.py
• [supply-chain-security-expert] argument-hint value is a bare YAML scalar containing angle brackets; should be double-quoted by the frontmatter serializer to prevent misinterpretation at src/apm_cli/integration/command_integrator.py
• [oss-growth-hacker] README Claude Code callout not updated to mention that prompts with input: produce typed slash command argument hints in Claude Code at README.md
• [oss-growth-hacker] No screenshot or visual proof of the argument-hint tooltip in Claude Code UI; the feature is UX-visible but the docs only show YAML at docs/src/content/docs/integrations/ide-tool-integration.md
• [oss-growth-hacker] 'Target-specific mapping' heading in prompts.md is a maintainer's phrase; rename to something user-searchable such as 'Claude Code: automatic argument mapping' at docs/src/content/docs/guides/prompts.md

CEO arbitration

The feature under review -- automatic mapping of input: frontmatter to Claude arguments: at install time -- is architecturally sound and strategically important. All five active panelists find it directionally correct. The REJECT verdict is driven by ten required findings that cluster into three distinct risk bands: security, type safety, and user communication.

The two supply-chain findings are the most urgent blockers. A fail-open ImportError catch means a missing or tampered gate module silently becomes a no-op; this inverts the expected security posture. The unsanitized dict-key path is a YAML injection vector: package-supplied input names pass through only a whitespace check before being written verbatim into compiled frontmatter, enabling a malicious package to inject attacker-controlled YAML keys into files written to the user's disk. Both findings are one-pass fixes, but neither can ship as-is.

The second risk band covers the severity misfire and the duplicate message/detail pair in diagnostics. These are tightly coupled: the severity='warning' downgrade actively hides critical security findings from users who rely on color and weight to triage output, undermining the value of the security gate. The third band covers type safety (Any vs Optional[DiagnosticCollector]), the brace-format gap in tests, the silent-mutation UX gap (no install-time feedback when content is transformed), and the missing cli-commands.md coverage. All are low-effort fixes that belong in the same focused pass.

Dissent resolved: Two dissents required resolution. First, the CHANGELOG Fixed-vs-Added classification was filed as required by oss-growth-hacker and as a nit by devx-ux-expert. The growth-hacker framing is correct: this is a net-new capability, not a defect correction, and misclassification actively hides it from users scanning the Added section on upgrade. The fix is one line. Second, the identical message/detail strings were filed as a nit by python-architect and as required by cli-logging-expert. The cli-logging-expert prevails because the two issues are mechanically linked: the severity bug is independently required, and fixing detail in isolation leaves severity wrong. Both are treated as required and should be fixed together.

Growth/positioning note: This PR contains the strongest Claude Code growth hook in the current unreleased cycle. The compounding value -- 'install any APM package and your Claude Code gets fully-typed slash commands automatically, with no action required from package authors' -- is concrete, demonstrable in under 30 seconds, and retroactively benefits every existing package at install time. Once the required findings are resolved in a single focused pass, lead with the user outcome in the CHANGELOG (the oss-growth-hacker's suggested rewrite is ready to use), update cli-commands.md, and consider a short screen recording showing the argument-hint tooltip appearing in Claude Code. The 'write once in APM format, get first-class Claude slash commands for free' proof point makes the 'package manager for AI-native development' positioning tangible in a way that abstract description cannot.

---

Per-persona findings (full)

Python Architect

classDiagram
    class BaseIntegrator {
        +find_files_by_glob()
        +resolve_links()
    }
    class CommandIntegrator {
        +find_prompt_files()
        +_transform_prompt_to_command()
        +integrate_command(diagnostics)
        +integrate_commands_for_target(diagnostics)
    }
    class ClaudeFormatter {
        +format_post()
    }
    class DiagnosticCollector {
        +security(message, package, detail, severity)
        +skip()
        +warning()
    }
    class SecurityGate {
        +scan_text(text, path, policy)
    }
    BaseIntegrator <|-- CommandIntegrator
    CommandIntegrator ..> ClaudeFormatter : removed dependency
    CommandIntegrator ..> DiagnosticCollector : routes warnings to
    CommandIntegrator ..> SecurityGate : scans compiled content
    note for ClaudeFormatter "CommandGenerationResult and generate_claude_commands() removed in this PR"
    class CommandIntegrator:::touched
    class ClaudeFormatter:::touched
    classDef touched fill:#ffe0b2,stroke:#e65100

Loading

flowchart TD
    A([apm install]) --> B[integrate_package_primitives]
    B --> C[integrate_commands_for_target]
    C --> D[integrate_command source target pkg_info diagnostics]
    D --> E[_transform_prompt_to_command]
    E --> E1["[I/O] frontmatter.load source"]
    E1 --> E2[_extract_input_names input_spec]
    E2 --> E3["claude_metadata arguments = input_names"]
    E3 --> E4[auto-generate argument-hint if absent]
    E4 --> E5["regex sub: ${input:name} -> $name"]
    E5 --> F[resolve_links]
    F --> G[frontmatter.dumps post]
    G --> H["[I/O] SecurityGate.scan_text WARN_POLICY"]
    H --> I{has_critical?}
    I -- yes --> J[append warning to warnings list]
    I -- no --> K[continue]
    J --> K
    K --> L{diagnostics?}
    L -- yes --> M[diagnostics.security message pkg warning]
    L -- no --> N[logger.warning]
    M --> O["[FS] target.parent.mkdir"]
    N --> O
    O --> P["[FS] write compiled content to target file"]

Loading

Design patterns

• Used in this PR: Collect-then-render -- warnings are collected during _transform_prompt_to_command and SecurityGate scan, then routed in a single loop at the end of integrate_command, keeping the transform step side-effect-free.
• Pragmatic suggestion: none -- the current shape is the simplest correct design at this scope. The move from ClaudeFormatter to CommandIntegrator is correct separation of concerns (formatters format, integrators integrate).

Required: 2. Nits: 4. See aggregated section above.

CLI Logging Expert

Required: 2. Nits: 3. See aggregated section above.

Key finding: severity='warning' when verdict.has_critical is True actively strips the red/bold render path. The security gate surface is only as useful as the signal it emits; a critical finding rendered in yellow is a UX regression in the diagnostic output.

DevX UX Expert

Required: 2. Nits: 2. See aggregated section above.

Key finding: The silent body rewrite (${input:name} -> $name) is a behavioral contract change for apm install. npm, pip, and cargo do not silently rewrite the content they install -- they copy, link, or compile with visible output. A user who discovers the rewrite by inspection rather than by install-time output loses trust in the tool's predictability.

Supply Chain Security Expert

Required: 2. Nits: 2. See aggregated section above.

Key finding: The YAML injection via unsanitized dict keys is the highest-severity finding in this panel. A malicious package that ships a .prompt.md with a crafted input: dict key can inject arbitrary YAML into files written to .claude/commands/ on the user's machine. The SecurityGate scan happens on the compiled output, but a YAML injection that produces valid-looking content will not necessarily trigger a hidden-character scan.

Auth Expert

Inactive -- No auth-relevant files touched; PR changes only command_integrator.py, claude_formatter.py, and docs for input-to-arguments frontmatter mapping.

OSS Growth Hacker

Required: 2. Nits: 3. See aggregated section above.

Side-channel note: This is the strongest Claude Code growth hook in the current unreleased cycle. The auto-generated argument-hint is a compounding feature -- every prompt in every existing and future package retroactively benefits at install time without any action from package authors. Once required findings are resolved, prioritize outcome-first CHANGELOG copy and a visual proof (screenshot or GIF) of the argument hint in Claude Code's slash command UI for community amplification.

_{Verdict computed deterministically: 10 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically.}

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

• #1039 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• fix(install): map APM prompt 'input' to Claude 'arguments' front-matter #1039 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by PR Review Panel for issue #1039 · ● 3.2M · ◷