feat(cursor): add slash command support for Cursor 1.6+#1046
feat(cursor): add slash command support for Cursor 1.6+#1046danielmeppiel merged 21 commits intomicrosoft:mainfrom
Conversation
Register the "commands" primitive in the Cursor target profile so apm install deploys .prompt.md files to .cursor/commands/*.md when a .cursor/ directory is present. Cursor 1.6 introduced custom slash commands [1] stored as plain Markdown in .cursor/commands/. Cursor has since de-emphasized commands in favor of skills/rules [2], but as long as it supports the commands surface it makes sense to deploy to it. Cursor itself does not consume YAML frontmatter, but we deliberately emit Claude-compatible frontmatter (description, allowed-tools, arguments, argument-hint) rather than stripping it. The frontmatter is harmless to Cursor -- the file is just a context-to-LLM routing mechanism -- and in practice, leaving structured input hints in the file makes the commands noticeably more successful at inferring required inputs from the user. This reuses the existing integrate_command() path (the else branch in CommandIntegrator.integrate_commands_for_target), so the only production code change is three lines in targets.py. Testing strategy: - Unit tests: opt-in guard, single/multi deploy, frontmatter preservation, sync removal, missing-dir graceful no-op - Integration tests: target gating dispatch (mocked integrators) and full end-to-end through integrate_package_primitives with real integrators verifying file output [1] https://cursor.com/changelog/1-6 [2] https://cursor.com/docs/plugins Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds Cursor 1.6+ slash command support by wiring the existing commands primitive into the Cursor target profile, plus updates docs and tests to reflect/cover the new dispatch path.
Changes:
- Register
commandsin the Cursor target profile so.prompt.mdcan deploy to.cursor/commands/*.mdwhen.cursor/exists. - Update docs and guide content to mention Cursor command deployment.
- Extend unit/integration tests and bucket-parity expectations to include
commands_cursor.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/integration/targets.py |
Adds Cursor commands primitive mapping to enable command integration for Cursor targets. |
tests/unit/integration/test_data_driven_dispatch.py |
Updates partition bucket parity expectations to include commands_cursor. |
tests/unit/integration/test_command_integrator.py |
Adds dispatch + end-to-end tests for Cursor command integration behavior. |
docs/src/content/docs/introduction/what-is-apm.md |
Documents Cursor support for .cursor/commands/. |
docs/src/content/docs/introduction/how-it-works.md |
Lists Cursor native integration paths including .cursor/commands/. |
docs/src/content/docs/integrations/ide-tool-integration.md |
Adds .cursor/commands/*.md to the Cursor integration table. |
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md |
Updates package authoring guidance to mention Cursor command deployment. |
- Docs: change "frontmatter preserved" to "normalized to supported command frontmatter" -- the shared command transformer keeps only description, allowed-tools, model, and argument-hint, dropping unknown keys like author or custom fields - Tests: rename test_frontmatter_preserved to test_frontmatter_normalized_to_supported_subset and assert that non-whitelisted fields (author, custom-field) are dropped - Tests: fix test_sync_removes_apm_commands to use manifest-based sync (managed_files set) instead of the legacy *-apm.md glob fallback, matching how Cursor commands are actually deployed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@copilot apply changes based on the comments in this thread |
|
Ah, I haven't signed up for it. I'll let Claude fix. |
# Conflicts: # packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
# Conflicts: # src/apm_cli/integration/targets.py # tests/unit/integration/test_data_driven_dispatch.py
|
|
||
| def test_cursor_target_dispatches_commands(self): | ||
| """When targets=[cursor], commands must be dispatched.""" | ||
| import tempfile, shutil |
There was a problem hiding this comment.
PEP 8: avoid importing multiple modules in a single import statement. This file already uses separate import lines (and has shutil/tempfile imported at module scope), so prefer import shutil and import tempfile (or just rely on the existing top-level imports) for consistency and linting.
| import tempfile, shutil |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t#1046) - targets.py: switch Cursor commands format_id from cursor_command to claude_command and document why. The shared command transformer is what actually runs today; the previous tag mis-advertised a Cursor- specific writer that does not exist and would silently lie about preserving Cursor-only frontmatter (author/input/mcp/parameters). Switch back to a dedicated cursor_command tag only when an integrator branch is added that preserves full Cursor metadata verbatim. - test_command_integrator.py: drop redundant per-test reimports of shutil/tempfile (PEP 8 nit, both are imported at module top). Other Copilot comments were already covered: - docs already say 'normalized to supported command frontmatter' - test_sync_removes_managed_commands exercises Cursor manifest mode - test_frontmatter_normalized_to_supported_subset asserts non-whitelisted fields (author, custom-field) are dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (10 items)
Nits (14 items, skip if you want)
CEO arbitrationThree independent panelists (cli-logging-expert, devx-ux-expert, supply-chain-security-expert) converge on the same silent-drop failure: Cursor-specific frontmatter keys are discarded without any user-visible diagnostic. This is the PR's most consequential defect -- it violates APM's core "install adds, never silently mutates" contract and will erode package-author trust the first time someone ships Python-architect raises two additional required items: the Strategically, this PR carries the highest growth leverage of any recent commit -- Cursor plus Claude Code represents the majority of AI-native IDE users, and "one .prompt.md deploys everywhere" is a genuinely differentiated story. The required issues are all fixable in a single revision pass. Blocking on them is the right call: shipping a silent data-loss bug or a path traversal in the first Cursor integration would damage APM's credibility with the exact audience this feature is designed to win. Dissent resolved: devx-ux-expert elevated the Growth/positioning note: Cursor commands parity is the strongest "write once, use everywhere" story APM has shipped since multi-target deploy. Claude Code + Cursor together account for the majority of AI-native IDE users. Recommend a dedicated CHANGELOG entry and a social post framing "one .prompt.md = slash commands in Claude Code AND Cursor, zero config" timed to the next version cut. A Cursor partnership co-announcement, if a contact exists, would amplify the story significantly. Per-persona findings (full)Python ArchitectclassDiagram
class BaseIntegrator {
+partition_managed_files(managed_files) dict
+check_collision(...) bool
+sync_remove_files(...) dict
}
class CommandIntegrator {
+integrate_commands_for_target(target, pkg_info, root) IntegrationResult
+integrate_command(source, target, ...) int
+sync_for_target(target, pkg, root) dict
-_transform_prompt_to_command(source) tuple
-_write_gemini_command(source, target) void
}
class TargetProfile {
+name: str
+root_dir: str
+primitives: dict
+auto_create: bool
+detect_by_dir: bool
}
class PrimitiveMapping {
+subdir: str
+extension: str
+format_id: str
}
class IntegrationResult {
+files_integrated: int
+files_skipped: int
+target_paths: list
}
BaseIntegrator <|-- CommandIntegrator
CommandIntegrator ..> TargetProfile : reads primitives
TargetProfile *-- PrimitiveMapping
CommandIntegrator ..> IntegrationResult : returns
note for PrimitiveMapping "cursor target uses format_id='claude_command' (shared transformer, known debt)"
class CommandIntegrator:::touched
class PrimitiveMapping:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A[apm install command] --> B[integrate_package_primitives]
B --> C{target has 'commands' primitive?}
C -- No --> Z[return empty IntegrationResult]
C -- Yes --> D{auto_create=False AND .cursor/ missing?}
D -- skip --> Z
D -- proceed --> E["[FS] find_files_by_glob in .apm/prompts/"]
E --> F{for each .prompt.md}
F --> G[check_collision]
G -- collision --> H[skip file]
G -- clear --> I{format_id == 'gemini_command'?}
I -- Yes --> J["[FS] _write_gemini_command TOML"]
I -- No --> K[integrate_command via claude_command transformer]
K --> L[_transform_prompt_to_command]
L --> M[resolve_links]
M --> N[SecurityGate.scan_text]
N --> O["[FS] write .cursor/commands/name.md"]
O --> F
F -- done --> P[return IntegrationResult]
Design patterns
Required:
Nits:
CLI Logging ExpertRequired:
Nits:
DevX UX ExpertRequired:
Nits:
Supply Chain Security ExpertRequired:
Nits:
Auth ExpertInactive -- PR only modifies targets.py (PrimitiveMapping addition for Cursor commands), test files, and documentation; no auth, token, credential, or host-classification code is touched. OSS Growth HackerNo findings. Nits:
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 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
added
panel-rejected
…target-aware diagnostics Address all 10 required findings from APM Review Panel on PR microsoft#1046: - Extract _make_package() to module-level test helper (DRY across 6 classes) - Make integrate_command() target-aware via target_name kwarg; emit 'command arguments' instead of Claude-branded message for Cursor installs - Detect frontmatter keys not preserved by the shared claude_command transformer (author, mcp, parameters, ...) and surface diagnostics.warn() per file so users see the lossy transform at install time - Add install-time info note when .cursor/ is missing instead of silently skipping deployment (improves CI-vs-dev discoverability) - Validate base_name with validate_path_segments() and ensure_path_within() before joining into commands_dir to close path-traversal vector via malicious package filenames - Harden find_files_by_glob() to reject hardlinks/files whose resolved path escapes package_path (symlink check did not catch hardlinks) - Surface a one-time IDE consent warning per package when Cursor commands are deployed since they become invokable as IDE slash commands - Add TODO(cursor-command-format) with tracking issue URL on both targets.py and integrate_command() to make the shared-transformer debt trackable - Add commands_cursor identity entry to _BUCKET_ALIASES for explicit documentation parity with commands_opencode - CHANGELOG entry under [Unreleased] and ide-tool-integration.md note about the frontmatter subset limitation - New TestCursorCommandPanelFindings regression class covers all five panel-driven behaviours (dropped-key warn, traversal rejection, target-aware diagnostics, IDE consent, skip note) Verified: 6939 unit tests pass; ruff format + ruff check clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (5 items)
Nits (11 items, skip if you want)
CEO arbitrationThe panel surfaces a coherent cluster of required findings that collectively demand a REJECT verdict. The two most structurally serious findings -- raised independently by supply-chain-security-expert and echoed architecturally by python-architect and devx-ux-expert -- concern the consent model for cursor command deployment. A warn() call is not a consent gate; it is a notification. When a post-install step deploys executable command files into an IDE's command directory, silent continuation after a warning is functionally indistinguishable from no warning at all. This must become an explicit opt-in flag before the change ships. Compounding this, the asymmetric treatment of Cursor versus Claude Code, OpenCode, and Gemini -- all of which deploy identically invocable commands without any warning -- both undermines the security rationale and creates an attacker bypass route: simply declare a non-Cursor target. The hardcoded name-string branch that python-architect flags is the mechanical root of this asymmetry; the TargetProfile layer is the correct home for a Dissent resolved: cli-logging-expert and oss-growth-hacker filed zero required findings. On the consent question, this CEO sides with supply-chain-security-expert and devx-ux-expert: the distinction between notification and consent is not a matter of taste but of threat modeling, and the asymmetric model compounds the risk rather than mitigating it. Growth/positioning note: oss-growth-hacker's framing is the right external story: APM as the package manager that bridges AI packages to native IDE slash commands is differentiated and Cursor mindshare is currently high. The compounding effect for package authors is real. Two risks to action before launch: the CHANGELOG and docs are written for maintainers, not for the developer discovering APM for the first time -- rewrite the CHANGELOG entry around user benefit, not mechanism; and Cursor's own signals suggest commands are a legacy surface, so shipping skills/rules support before Cursor deprecates commands would materially strengthen the narrative. Per-persona findings (full)Python ArchitectclassDiagram
class TargetProfile {
+name: str
+root_dir: str
+auto_create: bool
+primitives: dict[str, PrimitiveMapping]
}
class PrimitiveMapping {
+subdir: str
+extension: str
+format_id: str
+deploy_root: str | None
}
class BaseIntegrator {
+find_files_by_glob()
+check_collision()
+resolve_links()
+partition_bucket_key()
}
class CommandIntegrator {
+find_prompt_files()
+_transform_prompt_to_command() tuple
+integrate_command(target_name)
+integrate_commands_for_target()
+sync_for_target()
}
class _PRESERVED_COMMAND_KEYS {
<<frozenset>>
description
allowed-tools
model
argument-hint
input
}
class DiagnosticCollector {
+warn()
+info()
+security()
}
BaseIntegrator <|-- CommandIntegrator
TargetProfile "1" *-- "many" PrimitiveMapping
CommandIntegrator ..> TargetProfile : reads
CommandIntegrator ..> DiagnosticCollector : emits
CommandIntegrator ..> _PRESERVED_COMMAND_KEYS : filters with
class CommandIntegrator:::touched
class _PRESERVED_COMMAND_KEYS:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A[integrate_commands_for_target\ntarget=cursor] --> B{mapping = target.primitives.get commands}
B -- None --> Z[return empty IntegrationResult]
B -- exists --> C{auto_create=False\nand .cursor/ missing?}
C -- yes --> D["[FS] diagnostics.info: skipped, create dir"]
D --> Z
C -- no --> E["[FS] find_prompt_files via find_files_by_glob"]
E --> F{prompt_files empty?}
F -- yes --> Z
F -- no --> G[init_link_resolver]
G --> H{target.name == cursor?}
H -- yes --> I[diagnostics.warn: consent warning]
I --> J
H -- no --> J[for each prompt_file]
J --> K[validate_path_segments base_name]
K -- PathTraversalError --> L[diagnostics.warn + skip]
K -- ok --> M["[FS] build target_path = commands_dir/base.md"]
M --> N[ensure_path_within target_path, commands_dir]
N -- PathTraversalError --> L
N -- ok --> O{check_collision?}
O -- collision --> L
O -- ok --> P{format_id == gemini_command?}
P -- yes --> Q["[FS] _write_gemini_command"]
P -- no --> R[integrate_command\nsource, target, target_name=cursor]
R --> R1[_transform_prompt_to_command]
R1 --> R2[dropped_keys = source_keys - _PRESERVED_COMMAND_KEYS]
R2 --> R3{dropped_keys?}
R3 -- yes --> R4[diagnostics.warn: lossy transform]
R3 -- no --> R5
R4 --> R5["[FS] resolve_links in content"]
R5 --> R6[SecurityGate.scan_text]
R6 --> R7["[FS] write .cursor/commands/name.md"]
Q --> S[files_integrated plus 1]
R7 --> S
S --> J
J -- done --> T[return IntegrationResult]
Design patterns
Required findings: 1 (see top) CLI Logging ExpertNo findings required. Nits:
DevX UX ExpertRequired findings: 2 (see top) Nits:
Supply Chain Security ExpertRequired findings: 2 (see top) Nits:
Auth ExpertInactive -- PR #1046 only modifies integration/command_integrator.py, integration/base_integrator.py, integration/targets.py, and test/doc files; no authentication, token management, or credential resolution code is touched. OSS Growth HackerNo findings required. Nits:
Verdict computed deterministically: 5 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 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
removed
the
panel-review
…efault, --allow-cursor-commands Round 4 panel raised 8 required findings against the cursor-commands gate. Round 5 addresses all of them: Architecture (python-architect) - Add frozen IntegrateOptions dataclass with extra_kwargs(prim) helper in install/services.py. Dispatch loop is now branchless — no more inline 'if _prim_name == "commands"' special case. - Hoist pkg_name once at top of integrate_commands_for_target. - Move executable-commands gate before find_prompt_files in the happy path; keep one find call inside the gate-skipped branch so the user-facing skip warning includes an exact count. CLI logging (cli-logging-expert) - _post_install_summary surfaces the consent flag in --verbose output: ' allow-cursor-commands: true|false' (gated on logger.verbose). - Add _should_emit_passthrough_notice helper + _passthrough_notified instance set on CommandIntegrator. One-shot info diagnostic fires once per (install, target) on Cursor installs explaining that Cursor command files keep the Claude-compatible frontmatter subset intentionally for cross-tool compatibility. - Combine missing-.cursor/-dir hint with consent-flag mention so users see both signals in one diagnostic. DevX UX (devx-ux-expert) - Rename CLI flag --allow-executable-commands -> --allow-cursor-commands (target-scoped, user-friendly). Internal pipeline parameter remains allow_executable_commands so future targets can share the gate plumbing via their own --allow-<target>-commands flags. Mapping happens at the CLI boundary in install(). - Update skip warning to name the new flag. Supply-chain security (supply-chain-security-expert) - Flip TargetProfile.requires_executable_consent default to True at the dataclass level. New targets now refuse command deployment unless an opt-out is explicit. Add explicit =False with rationale comment on each existing target (claude, opencode, gemini), each referencing enterprise/security.md. - Add new '## Post-install code execution (Threat microsoft#8)' section to docs/src/content/docs/enterprise/security.md with the per-target posture matrix (cursor=gated, claude/opencode/gemini=opt-out), rationale for the asymmetry, npm --ignore-scripts mental model, and future-targets guidance. - Remove dead bool() cast in should_deploy_executable_commands. - Remove inline TODO comments referencing PR URL. OSS growth (oss-growth-hacker) - Rewrite CHANGELOG entry to lead with user outcome ('Cursor users can install APM package slash commands with one flag…'). Security rationale moved to end. Cursor 1.6+ lifecycle note included inline. - Add Cursor 1.6+ lifecycle caveat to: - docs/.../integrations/ide-tool-integration.md - docs/.../reference/cli-commands.md - packages/apm-guide/.apm/skills/apm-usage/commands.md - packages/apm-guide/.apm/skills/apm-usage/package-authoring.md Tests - Update test assertions to use new flag name. - Add requires_executable_consent=False to the synthetic test profile in tests/unit/integration/test_data_driven_dispatch.py (consequence of fail-closed default). - Full unit suite: 6924 passed. Round 5 panel: 6/6 specialists APPROVE; CEO synthesizer APPROVE. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
added
panel-review
APM Review Panel Verdict: REJECT
Required before merge (2 items)
Nits (10 items, skip if you want)
CEO arbitrationThe REJECT verdict is deterministic and correctly computed: the oss-growth-hacker raised two REQUIRED findings, both targeting CHANGELOG.md. The technical specialists -- python-architect, cli-logging-expert, devx-ux-expert, and supply-chain-security-expert -- returned a combined zero REQUIRED findings. That asymmetry is itself the strategic signal, not a reason to discount the growth-hacker's call. The two REQUIRED findings are genuinely blocking. CHANGELOG.md is the highest-traffic artifact for existing adopters evaluating whether to act on a release. A feature that the project itself flags as potentially short-lived (Cursor is de-emphasizing commands) must earn its opt-in. If the hook does not hand the reader a repostable one-sentence value claim in the first line, adopters will either skip the feature or adopt it without understanding the risk surface. The lifecycle deprecation warning buried as the final clause of a 150-word paragraph is a community-trust failure waiting to happen. The recommended fix is low-cost and high-trust-leverage: a one-sentence hook and a prominently styled lifecycle callout. No code changes required to unblock APPROVE. Growth/positioning note: The security framing is the actual story: APM treats prompt deployment like npm treats scripts -- opt-in, explicit, auditable. That one-line parallel is absent from every user-facing surface in this PR. The recommended CHANGELOG hook doubles as launch copy for a short thread. Classified as moderate signal given the Cursor longevity risk; hold the full launch beat for the rules/skills equivalent or for confirmed Cursor stability. Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class TargetProfile {
<<ValueObject>>
+name str
+root_dir str
+primitives dict
+auto_create bool
+requires_flag str
+requires_executable_consent bool
}
class PrimitiveMapping {
<<ValueObject>>
+subdir str
+extension str
+format_id str
}
class IntegrateOptions {
<<ValueObject>>
+allow_executable_commands bool
+extra_kwargs(primitive_name) dict
}
class CommandIntegrator {
+integrate_commands_for_target(target, pkg, root, allow_executable_commands)
+integrate_package_commands(pkg, root, allow_executable_commands)
}
class InstallContext {
<<Accumulator>>
+project_root Path
+force bool
+verbose bool
+allow_executable_commands bool
}
class InstallRequest {
<<ValueObject>>
+allow_executable_commands bool
}
TargetProfile *-- PrimitiveMapping : primitives
CommandIntegrator ..> TargetProfile : reads requires_executable_consent
InstallContext ..> CommandIntegrator : supplies via integrate phase
IntegrateOptions ..> CommandIntegrator : extra_kwargs splat
InstallRequest ..> InstallContext : seeds
note for TargetProfile "fail-closed: default True\ncursor=True, claude/opencode/gemini=False"
note for IntegrateOptions "Centralises per-primitive kwarg\nrouting; frozen so gate cannot\nbe mutated mid-loop (Threat #8)"
class TargetProfile:::touched
class IntegrateOptions:::touched
class InstallContext:::touched
class InstallRequest:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
CLI["apm install --allow-cursor-commands"] --> parse["commands/install.py\n_install_apm_dependencies()\nallow_cursor_commands=True"]
parse --> optsbuild["InstallOptions.allow_executable_commands=True"]
optsbuild --> pipeline["install/pipeline.py\nrun_install_pipeline(allow_executable_commands=True)"]
pipeline --> ctx["InstallContext\n.allow_executable_commands=True"]
ctx --> integrate["install/phases/integrate.py run()\npasses ctx.allow_executable_commands"]
integrate --> svc["install/services.py\nintegrate_package_primitives(allow_executable_commands=True)"]
svc --> opts["IntegrateOptions(allow_executable_commands=True)\n[constructed per-target in loop]"]
opts --> loop["for target in targets\nfor prim_name in target.primitives"]
loop --> extra["IntegrateOptions.extra_kwargs('commands')\n-> {allow_executable_commands: True}"]
extra --> cmdint["CommandIntegrator\n.integrate_commands_for_target(..., allow_executable_commands=True)"]
cmdint --> gate{"should_deploy_executable_commands(target,\nallow_executable_commands=True)"}
gate -->|"requires_executable_consent=False\n(claude/opencode/gemini)"| deploy["[FS] write command files"]
gate -->|"requires_executable_consent=True AND flag=True\n(cursor + --allow-cursor-commands)"| deploy
gate -->|"requires_executable_consent=True AND flag=False\n(cursor without flag)"| skip["[I/O] find_prompt_files() once\ndiagnostics.warn(commands_skipped)\nreturn IntegrationResult(0,...)"]
Design patterns
Nits:
CLI Logging ExpertNits:
DevX UX ExpertNits:
Supply Chain Security ExpertNits:
Auth ExpertInactive -- No auth files changed; PR adds Cursor command deployment gate (--allow-cursor-commands) with no changes to AuthResolver, token management, credential resolution, or host classification. OSS Growth HackerRequired:
Nits:
Verdict computed deterministically: 2 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 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
added
panel-rejected
danielmeppiel
removed
the
panel-rejected
…fault Cursor slash commands have identical invocation semantics to Claude/OpenCode/Gemini -- the user must type /cmdname to invoke. They do not auto-execute on disk-write or IDE startup, so the npm --ignore-scripts / Threat microsoft#8 framing was incorrect. The gate created artificial asymmetry between targets with identical UX and violated portable-by-manifest. Removed: - --allow-cursor-commands CLI flag - TargetProfile.requires_executable_consent field - should_deploy_executable_commands() helper + gate block - IntegrateOptions per-primitive routing (only consumer was the gate) - allow_executable_commands plumbing through 7 layers - Threat microsoft#8 framing from docs + CHANGELOG Kept: - Cursor commands PrimitiveMapping (the actual feature) - Lossy-frontmatter diagnostics.warn() per file (real value: surfaces dropped Cursor-specific keys to package authors) - Cursor 1.6+ lifecycle note (Cursor de-emphasizing commands) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (6 items)
Nits (17 items, skip if you want)
CEO arbitrationThis PR is REJECTED on six required findings spanning performance, output quality, and security severity. The panel reached strong consensus on two of the six: cli-logging-expert and devx-ux-expert independently and identically flagged the per-file dropped-keys warn() as a terminal-flooding anti-pattern. The fix direction is also agreed -- aggregate dropped-key occurrences per target or per package and emit a single consolidated warning. No dissent to resolve there; both findings stand and compound each other. The devx-ux-expert additionally flagged the skip-notice firing once per package rather than once per install run, which creates a symmetric flooding problem on the non-Cursor path. Both output-path findings must be addressed together before merge. The security finding from supply-chain-security-expert is unambiguous and independent: PathTraversalError rejections are emitted as warn() rather than error(), which renders them invisible to any CI policy gate that filters on severity. A traversal rejection is an integrity event; it must surface as an error. This is a one-line change with no ergonomic cost and no trade-off to weigh. The python-architect required finding on pkg_resolved being recomputed inside the per-file loop is defensible at required level because the inline comment explicitly claims "loop-fast" while doing the opposite -- the comment actively misleads future contributors. Dissent resolved: python-architect elevated pkg_resolved-per-iteration to required; supply-chain-security-expert left it as a nit. CEO sides with the architect. The deciding factor is not the O(n) syscall cost -- that is minor at realistic package sizes -- but the "loop-fast" comment that asserts the opposite of what the code does. A misleading comment in a security-adjacent path is a correctness defect, not a style nit. The fix is a one-line hoist and a comment deletion; it should ship with this PR. Growth/positioning note: OSS Growth Hacker timing verdict: ship. Cursor has not removed slash commands; the feature is gated on .cursor/ presence so blast radius is minimal and rollback is trivial. The conversion gap is the first-run opt-in story -- creating .cursor/ is a non-obvious step for Cursor-native users who expect zero-config setup. The CHANGELOG and docs should name the Cursor Settings path that creates .cursor/ rather than leaving it as a raw mkdir instruction. This is a one-paragraph doc fix that materially reduces first-run friction for the Cursor user segment. Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class BaseIntegrator {
<<TemplateMethod>>
+find_files_by_glob(pkg, glob, subdirs) list
+check_collision(rel, target, pkg) bool
+validate_deploy_path(path) Path
+resolve_links(content, src, tgt) tuple
}
note for BaseIntegrator "Template Method: subclasses implement\nintegrate_X; base owns collision,\nmanifest-sync, path-security"
class CommandIntegrator {
<<ConcreteTemplate>>
-_passthrough_notified set
+find_prompt_files(pkg) list
+integrate_command(src, tgt, info, orig, diag, target_name) int
+integrate_commands_for_target(pkg, root, target, diag) IntegrationResult
-_transform_prompt_to_command(src) tuple
-_should_emit_passthrough_notice(name, fmt_id) bool
}
class PromptIntegrator {
<<ConcreteTemplate>>
+integrate_prompt(src, tgt, info, diag) int
}
class AgentIntegrator {
<<ConcreteTemplate>>
}
class TargetProfile {
<<ValueObject>>
+name str
+root_dir str
+auto_create bool
+primitives dict
+for_scope(user_scope) TargetProfile
}
class PrimitiveMapping {
<<ValueObject>>
+subdir str
+extension str
+format_id str
+deploy_root str
}
class IntegrationResult {
<<ValueObject>>
+files_linked int
+files_skipped int
+collisions int
+warnings list
+links_resolved int
}
class DiagnosticCollector {
<<CollectThenRender>>
+info(message, package, detail)
+warn(message, package)
+render_summary()
}
note for DiagnosticCollector "Collect-then-render: push during\noperation, render at install end"
class PathTraversalError {
<<Exception>>
}
BaseIntegrator <|-- CommandIntegrator
BaseIntegrator <|-- PromptIntegrator
BaseIntegrator <|-- AgentIntegrator
CommandIntegrator ..> TargetProfile : reads
CommandIntegrator ..> PrimitiveMapping : reads
CommandIntegrator ..> IntegrationResult : returns
CommandIntegrator ..> DiagnosticCollector : pushes to
CommandIntegrator ..> PathTraversalError : catches
TargetProfile *-- PrimitiveMapping : primitives
class CommandIntegrator:::touched
class BaseIntegrator:::touched
class PrimitiveMapping:::touched
class TargetProfile:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([apm install]) --> B[_integrate_package_primitives\ninstall.py]
B --> C{cursor in targets?}
C -- no --> Z([skip])
C -- yes --> D{.cursor/ dir exists?\ntarget.auto_create=False}
D -- missing --> E[diagnostics.info: Skipped .cursor/commands/\nonce per install run AFTER fix]
E --> Z
D -- present --> F[find_prompt_files\nbase_integrator.find_files_by_glob]
F --> G["[FS] glob *.prompt.md in .apm/prompts/"]
G --> H{is_symlink? skip}
H --> I["[FS] resolved = f.resolve()\npkg_resolved = package_path.resolve()\n(WARNING: called per-file, should be hoisted)"]
I --> J{resolved.is_relative_to\npkg_resolved?}
J -- no --> K[skip hardlink escape]
J -- yes --> L{_should_emit_passthrough_notice}
L -- yes --> M[diagnostics.info]
L -- no --> N
M --> N[validate_path_segments]
N -- PathTraversalError --> O["diagnostics.error (AFTER FIX)\nfiles_skipped++"]
N -- ok --> P[ensure_path_within]
P -- PathTraversalError --> Q["diagnostics.error (AFTER FIX)\nfiles_skipped++"]
P -- ok --> R[check_collision]
R -- collision --> S[diagnostics.warn]
R -- clear --> T[_transform_prompt_to_command]
T --> U["(command_name, post, warnings, dropped_keys)"]
U --> V{dropped_keys?}
V -- yes --> W["diagnostics.warn AGGREGATED (AFTER FIX)\none warn per target+package"]
V -- no --> X
W --> X{arguments mapped?}
X -- yes --> Y[diagnostics.info]
X -- no --> AA
Y --> AA["write .cursor/commands/<name>.md"]
AA --> BB[files_linked++]
BB --> CC([IntegrationResult])
Design patterns: Template Method -- Required: 1 (pkg_resolved per-loop, see above) CLI Logging ExpertRequired: 2 (per-file dropped-keys flood, passthrough notice leaks jargon) DevX UX ExpertRequired: 2 (per-file warn floods terminal, skip notice fires per package) Supply Chain Security ExpertRequired: 1 (traversal rejections must be error, not warn) Auth ExpertInactive -- No auth-related files changed; PR adds Cursor command deployment to command_integrator.py and targets.py with no effect on AuthResolver, token precedence, credential resolution, or HTTP authorization headers. OSS Growth HackerRequired: None. Side-channel: Timing is acceptable -- Cursor has not removed commands; feature is gated on .cursor/ presence (low blast radius). Conversion gap is the first-run opt-in story. "Same slash-command UX as Claude/OpenCode/Gemini" is technically accurate but the lossy frontmatter transform is real -- the per-file warn (once fixed to aggregate) is the right mitigation. Verdict computed deterministically: 6 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 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
added
panel-rejected
|
I am confused by this:
All coding agents (claude, gemini, copilot, etc) take |
|
Nevermind, I see the latest commit now drops this - 6adb357 |
|
@stbenjam I am refactoring the review panel, it will now show as advisory, not a gate. We are instrumenting this repo heavily with agents, but that's a learning process. I'll review this and merge soon. Thanks! |
danielmeppiel
removed
the
panel-rejected
# Conflicts: # docs/src/content/docs/integrations/ide-tool-integration.md # src/apm_cli/integration/targets.py
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 3 | 2 | Solid Cursor command dispatch using the data-driven target pattern. Two minor design gaps: identity alias in _BUCKET_ALIASES and stale class docstring; no blockers. |
| CLI Logging Expert | 0 | 2 | 2 | Two signal-to-noise issues: one-shot passthrough notice fires unconditionally (noise when no keys dropped); dropped-keys warn uses internal jargon. |
| DevX UX Expert | 0 | 2 | 2 | Happy-path noise and internal jargon in two diagnostic messages; commands.md skill not synced; feature shape and docs coverage otherwise solid. |
| Supply Chain Security Expert | 0 | 1 | 1 | No path-traversal or post-install-execution bypasses found; one defense-in-depth gap in post-transform scan policy; one nit on allowed-tools passthrough. |
| OSS Growth Hacker | 0 | 3 | 1 | Solid parity win for Cursor's 4M+ users, but CHANGELOG leads with a deprecation caveat and the feature stays invisible to top-of-funnel visitors. |
| Doc Writer | 1 | 2 | 1 | One blocking typo (opencode path), one recommended restructure (security.md placement), one recommended cell verbosity issue, one nit on duplication. |
| Test Coverage Expert | 0 | 2 | 1 | Cross-module integration surface has fixtures-tier test in unit/ folder (not run); hardlink containment guard in base_integrator.py has no test; all other surfaces covered. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Doc Writer] (blocking-severity) Fix
.opencode/command/*.md->.opencode/commands/*.mdin security.md -- Factual error in a security-scoped enterprise doc. One character, zero ambiguity. Must fix before merge. - [Test Coverage Expert] Add unit test for hardlink containment escape-path branch in base_integrator.py -- New security guard with no automated coverage on the escape path. Missing test on a secure-by-default surface inherits near-blocking weight per panel rules.
- [CLI Logging Expert] Gate passthrough notice on actual dropped keys; rewrite dropped-keys warning to remove "shared command transformer" jargon -- Two converging panelists (cli-logging + devx-ux) on the same two issues. Low-effort, high-signal UX fix that reduces terminal noise and makes policy failures legible to package authors.
- [Supply Chain Security Expert] Upgrade post-transform content scan from WARN_POLICY to BLOCK_POLICY in integrate_command() -- Defense-in-depth gap: transform-introduced content currently deploys even on critical scan findings. Cheap fix; consistent with secure-by-default principle.
- [OSS Growth Hacker] Rewrite CHANGELOG entry user-first; add one README line about slash command auto-deployment -- Current CHANGELOG leads with a deprecation caveat that chills adoption. README is silent on slash commands. Two-minute fix unlocks top-of-funnel discoverability for 4M+ Cursor users.
Architecture
classDiagram
direction LR
class BaseIntegrator {
<<Abstract>>
+link_resolver: UnifiedLinkResolver
+should_integrate(project_root) bool
+check_collision(target_path, rel_path, managed_files, force) bool
+validate_deploy_path(rel_path, project_root) bool
+partition_managed_files(managed_files, targets) dict
+sync_remove_files(project_root, managed_files, prefix) dict
+find_files_by_glob(package_path, pattern, subdirs) list
+init_link_resolver(package_info, project_root)
+resolve_links(content, source, target) tuple
-_BUCKET_ALIASES: dict
}
class CommandIntegrator {
<<ConcreteIntegrator>>
-_passthrough_notified: set
+find_prompt_files(package_path) list
+integrate_command(source, target, package_info, original_path, diagnostics, target_name) int
+integrate_commands_for_target(target, package_info, project_root, force, managed_files, diagnostics) IntegrationResult
+sync_for_target(target, apm_package, project_root, managed_files) dict
-_transform_prompt_to_command(source) tuple
-_should_emit_passthrough_notice(target_name, format_id) bool
-_write_gemini_command(source, target)
}
class IntegrationResult {
<<ValueObject>>
+files_integrated: int
+files_updated: int
+files_skipped: int
+target_paths: list
+links_resolved: int
}
class TargetProfile {
<<ValueObject>>
+name: str
+root_dir: str
+primitives: dict
+auto_create: bool
+detect_by_dir: bool
+for_scope(user_scope) TargetProfile
+deploy_path(project_root) Path
+supports(primitive) bool
}
class PrimitiveMapping {
<<ValueObject>>
+subdir: str
+extension: str
+format_id: str
+deploy_root: str
}
class SecurityGate {
<<IOBoundary>>
+scan_text(text, path, policy) ScanVerdict
}
class DiagnosticCollector {
<<Observer>>
+warn(message, package)
+info(message, package, detail)
+security(message, package, detail, severity)
+skip(rel_path)
}
note for CommandIntegrator "Template Method: integrate_commands_for_target\nis the fixed skeleton; _transform_prompt_to_command\nand _write_gemini_command are hot-swap steps.\nData-driven dispatch via format_id selects writer."
note for TargetProfile "cursor: commands PrimitiveMapping\nuses format_id='claude_command'\n(shared transformer, TODO cursor_command)"
BaseIntegrator <|-- CommandIntegrator
CommandIntegrator ..> IntegrationResult : returns
CommandIntegrator ..> TargetProfile : reads
CommandIntegrator ..> DiagnosticCollector : pushes to
CommandIntegrator ..> SecurityGate : calls
TargetProfile *-- PrimitiveMapping : contains
class CommandIntegrator:::touched
class TargetProfile:::touched
class BaseIntegrator:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([apm install --target cursor]) --> B[resolve_targets / active_targets]
B --> C{.cursor/ dir exists?}
C -- No, auto_create=False --> D[diagnostics.info: skipped .cursor/commands/]
C -- Yes --> E[integrate_commands_for_target\ncommand_integrator.py]
E --> F[_should_emit_passthrough_notice\ntarget=cursor, format_id=claude_command]
F -- first call --> G[diagnostics.info: Claude-compatible\nfrontmatter intentional]
F -- already notified --> H
G --> H[find_prompt_files\nbase_integrator.find_files_by_glob]
H --> I[FS: glob *.prompt.md in install_path/.apm/prompts]
I --> J{symlink? skip\nhardlink outside pkg? skip\ncontainment guard}
J --> K[for each prompt_file]
K --> L[validate_path_segments base_name\npath_security.py]
L -- PathTraversalError --> M[diagnostics.warn + skip]
L -- OK --> N[ensure_path_within target_path, commands_dir\npath_security.py]
N -- PathTraversalError --> M
N -- OK --> O[check_collision\nbase_integrator.py]
O -- collision+no force --> P[diagnostics.skip]
O -- OK --> Q{format_id?}
Q -- gemini_command --> R[FS: _write_gemini_command]
Q -- claude_command\ncursor/claude/opencode --> S[integrate_command\ntarget_name=cursor]
S --> T[_transform_prompt_to_command\nparse frontmatter, map keys]
T --> U[dropped_keys = source_keys - _PRESERVED_COMMAND_KEYS]
U --> V{dropped_keys?}
V -- yes --> W[diagnostics.warn: frontmatter keys not written]
V --> X[resolve_links via UnifiedLinkResolver]
X --> Y[SecurityGate.scan_text compiled output]
Y --> Z[FS: target.parent.mkdir + write .cursor/commands/name.md]
Z --> AA[IntegrationResult files_integrated++]
Recommendation
Fix the one-character OpenCode path typo in security.md before merge -- it is a factual error in an enterprise-facing doc. The remaining items (hardlink test, diagnostic message rewrites, BLOCK_POLICY upgrade, CHANGELOG rewrite) are high-signal but non-blocking; the PR author should resolve them in this PR if turnaround is fast, or commit to fast-follow PRs if not. The feature architecture is sound, the security primary gate is unaffected, and the Cursor parity win is strategically significant enough to not delay further for opinion-tier findings.
Full per-persona findings
Python Architect
- [recommended] Identity alias
commands_cursor -> commands_cursorin _BUCKET_ALIASES is a no-op and misleads readers atsrc/apm_cli/integration/base_integrator.py:183
Every other entry in _BUCKET_ALIASES maps a raw key to a different canonical name (e.g.commands_claude -> commands). The new entry maps to itself -- the dict.get(raw, raw) fallback already handles it. Readers will assume this entry does something meaningful.
Suggested: Remove'commands_cursor': 'commands_cursor'from _BUCKET_ALIASES -- the dict.get(raw, raw) default already handles it. If explicit registration is desired for discoverability, add a comment to that effect. - [recommended] Class-level docstring of CommandIntegrator still says
.claude/commands/only atsrc/apm_cli/integration/command_integrator.py:127
The class now handles any target. Stale single-target docstring will confuse the next contributor adding a fourth target.
Suggested: Replace with: "Integrates.prompt.mdfiles as slash commands for any target that declares acommandsprimitive (Claude Code, Cursor, OpenCode, Gemini CLI). Dispatch is data-driven viaTargetProfile.primitives['commands'].format_id." - [recommended]
integrate_commands_for_targetexistence-check usestarget.root_dirbuttarget_rootmay usemapping.deploy_rootatsrc/apm_cli/integration/command_integrator.py:437
The skip guard checks(project_root / target.root_dir).is_dir()buttarget_rootusesmapping.deploy_rootwhen set. Latent inconsistency for future targets using deploy_root for commands.
Suggested: Change the guard to:if not target.auto_create and not target_root.is_dir(): - [nit]
_should_emit_passthrough_noticehardcodes the string'claude'rather than referencing a constant atsrc/apm_cli/integration/command_integrator.py:150
If the claude target slug is ever renamed, this guard silently stops suppressing the notice for the canonical Claude deploy path. - [nit] Design-patterns note: Template Method + data-driven dispatch via format_id is the right shape. No Strategy Protocol warranted at current scale.
CLI Logging Expert
- [recommended] One-shot passthrough notice fires even when zero Cursor-specific keys are dropped at
src/apm_cli/integration/command_integrator.py
The_should_emit_passthrough_noticeguard deduplicates per (target, run) but fires for every install touching a cursor-style target's commands, even when nothing is lossy. Fails the Signal-to-noise test -- pure noise for packages using only preserved keys.
Suggested: Gate the passthrough notice on whether at least one file in the batch actually had dropped_keys, or drop the notice entirely -- the per-file dropped-keys warn already tells the story when relevant. - [recommended] Dropped-keys warn message uses internal implementation jargon at
src/apm_cli/integration/command_integrator.py
"frontmatter keys not written by the shared command transformer" -- "shared command transformer" is machine-internal language. Package authors don't know what this is.
Suggested: Replace with:f"{target_name.capitalize()} command {command_name}: frontmatter keys not supported for {target_name} commands and were dropped: {', '.join(dropped_keys)}." - [nit] Dropped-keys warn appends a hardcoded key list that duplicates
_PRESERVED_COMMAND_KEYSatsrc/apm_cli/integration/command_integrator.py
The trailing "Only description, allowed-tools, model, argument-hint, and input are preserved." will silently go stale if _PRESERVED_COMMAND_KEYS changes. - [nit] One-shot passthrough notice is attributed to whichever package happens to be processed first at
src/apm_cli/integration/command_integrator.py
package=pkg_name on the one-shot notice ties a target-level message to an arbitrary package name, which can confuse DiagnosticCollector rendering in multi-package installs.
Suggested: Pass package=None or package="" for the passthrough notice.
DevX UX Expert
- [recommended] Passthrough notice fires unconditionally on every Cursor install, polluting quiet happy path at
src/apm_cli/integration/command_integrator.py
diagnostics.info() items always render non-verbose. Fires even when zero keys are dropped. Anti-pattern: output floods terminal on success.
Suggested: Remove the passthrough notice entirely, or gate it on verbose / move to debug level. - [recommended] Dropped-keys warning uses internal implementation jargon invisible to package authors at
src/apm_cli/integration/command_integrator.py
"shared command transformer" is an internal class name. A package author writing YAML frontmatter has no mental model for "transformer".
Suggested: Rewrite to: "{Target} command {name}: frontmatter keys dropped (unsupported for this target): {keys}. Supported: description, allowed-tools, model, argument-hint, input." - [nit]
packages/apm-guide/.apm/skills/apm-usage/commands.mdnot updated atpackages/apm-guide/.apm/skills/apm-usage/commands.md
Shipped skill resource not synced with docs. Cursor users consulting the in-IDE reference will not discover the feature.
Suggested: Add a row or note underapm installdescribing .cursor/commands/*.md deployment, mirroring the cli-commands.md addition. - [nit] Skip message loses "in your project root" anchor at
src/apm_cli/integration/command_integrator.py
Dropping the spatial anchor "in your project root" reduces orientation for first-run users.
Suggested: Append "in your project root" to the skip message.
Supply Chain Security Expert
- [recommended] Post-transform content scan uses WARN_POLICY -- critical hidden-char findings do not gate the write at
src/apm_cli/integration/command_integrator.py
integrate_command()callsSecurityGate.scan_textwithWARN_POLICY(on_critical='warn'), then always writes the file. The pre-install BLOCK_POLICY gate remains the primary guard, but using WARN_POLICY here means a transform-introduced critical finding would silently deploy. Defense-in-depth gap; cheap to close.
Suggested: Replacepolicy=WARN_POLICYwithpolicy=BLOCK_POLICY, or add:if scan_verdict and scan_verdict.has_critical: files_skipped += 1; continuebefore the write. - [nit]
allowed-toolslist entries passed verbatim without per-entry validation atsrc/apm_cli/integration/command_integrator.py
Low risk in current trust model (users install trusted packages). Track as follow-up: surface resolved allowed-tools list via diagnostics.info() at install time so users see what permissions a command claims.
OSS Growth Hacker
- [recommended] CHANGELOG entry opens the feature and immediately warns it may be going away
Embedding "Cursor is de-emphasizing commands in favor of rules/skills -- monitor Cursor release notes" inside the same sentence that announces the feature is adoption-chilling.
Suggested: Trim CHANGELOG to the user benefit ("Cursor users now get slash commands on apm install") and move the lifecycle caveat to a follow-up sentence or footnote. - [recommended] README and quickstart are silent about slash commands; feature is invisible to top-of-funnel
README mentions Cursor 4 times but never mentions slash commands. Cursor users who land via search or word-of-mouth will not discover this feature.
Suggested: Add one line to the README features list (e.g., "slash commands deployed to .claude/, .cursor/, .opencode/, .gemini/ automatically") and one sentence in the quickstart output example. - [recommended] CHANGELOG entry is written for maintainers, not users
Mentions "diagnostics.warn() per file", "lossy transform", "Cursor-specific keys (author, mcp, parameters, ...) are dropped" -- implementation details that erode user confidence.
Suggested: Rewrite user-first: "Cursor users get slash commands automatically when .cursor/ exists -- no extra steps." - [nit] No runnable proof-of-concept in any doc
A single shell snippet (apm install <pkg> && ls .cursor/commands/) would make the feature tangible and shareable.
Auth Expert -- inactive
No auth files changed; PR deploys local .prompt.md files to .cursor/commands/ with no auth, token, or credential-resolution surface touched.
Doc Writer
- [blocking] OpenCode path in security.md table is
.opencode/command/*.md(missing trailing 's') atdocs/src/content/docs/enterprise/security.md
All other sources (source code, how-it-works.md, what-is-apm.md, package-authoring.md) use.opencode/commands/*.md. This is a factually wrong path that will mislead readers.
Suggested: Change.opencode/command/*.md->.opencode/commands/*.mdin the slash command deployment table. - [recommended] Slash command deployment section belongs in ide-tool-integration.md, not security.md at
docs/src/content/docs/enterprise/security.md
security.md is scoped to supply chain security, content scanning, path safety, and trust models. The new section describes a deployment feature. The only security-relevant claim (commands are not auto-invoked) is one sentence.
Suggested: Remove the "Slash command deployment" section from security.md. Add one sentence under the "File presence IS execution" paragraph. The full deployment table already lives in ide-tool-integration.md. - [recommended] New
.cursor/commands/*.mdtable cell is too verbose, breaking visual consistency atdocs/src/content/docs/integrations/ide-tool-integration.md
Every other row in the Cursor integration table is <=10 words. The new row is a multi-sentence paragraph. Also duplicates content already in security.md.
Suggested: Shorten cell to: "Slash commands from installed packages (.prompt.md files). Cursor 1.6+ only." Add a callout below the table for the lifecycle note. - [nit] Dropped-keys detail (
author,mcp,parameters) duplicated across security.md and ide-tool-integration.md atdocs/src/content/docs/enterprise/security.md
Per "state once, reference elsewhere" rule. ide-tool-integration.md should be canonical.
Test Coverage Expert
- [recommended]
test_full_dispatch_deploys_to_cursorcannot be certified (pytest unavailable in review environment) attests/unit/integration/test_command_integrator.py
The test reads correctly and structurally proves the promise, but the outcome cannot be certified aspassedper S7 PROBE RULE. Tier: integration-with-fixtures. Test:TestCursorCommandEndToEnd::test_full_dispatch_deploys_to_cursor. Proves: apm install deploys .prompt.md files to .cursor/commands/ via full dispatch pipeline with correct frontmatter. Proof (unknown): environment did not support running pytest. - [recommended] New hardlink containment guard in base_integrator.py has no test for the escape-path branch at
tests/unit/integration/test_base_integrator.py
Greppedtests/unit/integration/test_base_integrator.pyfor 'hardlink|containment|resolve.*relative' -- zero hits. Theis_relative_toescape branch is untested. This is a secure-by-default surface. Proof (missing at unit tier):tests/unit/integration/test_base_integrator.py::test_collect_source_files_rejects_hardlink_escaping_package_root-- proves: a package shipping a hardlink pointing outside its install directory cannot be deployed. [secure-by-default, governed-by-policy] - [nit] Unit coverage for TestCursorCommandIntegration and TestCursorCommandPanelFindings is structurally sound and well-targeted at
tests/unit/integration/test_command_integrator.py
All six TestCursorCommandIntegration tests and four TestCursorCommandPanelFindings tests present and covering the new surface.TestCursorCommandPanelFindings::test_path_traversal_filename_rejectedproves: a package shipping a traversal-laden filename is rejected and nothing is written to .cursor/commands/. [secure-by-default]
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #1046
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - feat(cursor): add slash command support for Cursor 1.6+ #1046
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 | noneGenerated by PR Review Panel for issue #1046 · ● 7.2M · ◷
github-actions
Bot
removed
the
panel-review
Apply the actionable findings from the apm-review-panel critique on PR microsoft#1046: * docs: fix `.opencode/command/*.md` -> `.opencode/commands/*.md` typo in the security guide (factual error in IDE integration map). * CHANGELOG: rewrite Cursor entry user-first; lifecycle caveat moves to a trailing sentence so the value prop reads first. * integrator: gate the one-shot 'cross-tool compatibility' notice on whether at least one file in the batch had dropped frontmatter keys. Previously fired on every Cursor install -> noise on the happy path. Cursor installs whose prompts only use the cross-tool subset now emit zero notice. Other targets (claude, opencode) unaffected. * integrator: rewrite the dropped-keys warn message to drop internal jargon ('shared command transformer'); use target-name framing ('not supported for cursor commands') and surface the canonical kebab-case key list (no camelCase aliases). * integrator: upgrade post-transform scan_text from WARN_POLICY to BLOCK_POLICY. When a critical finding is detected (e.g. a hidden tag char introduced by link resolution), skip the write and account for it in result.files_skipped. Pre-install BLOCK gate already scans source files, so this is defense-in-depth for the transform-introduced edge case; no regression for legitimate packages. * base_integrator: actually close the hardlink containment gap in find_files_by_glob. The prior is_relative_to() check did NOT catch hardlinks -- a hardlink resolves to its own path inside the package root. Add an st_nlink > 1 reject to prevent a malicious package shipping a hardlink to (e.g.) /etc/passwd from being read via integration. Update the inline comment to be accurate about what each guard actually does. Tests: * tests/unit/integration/test_base_integrator.py: new test_hardlink_escaping_package_root_is_excluded covering the st_nlink reject; cleaned up F821 noqa via real pytest import. * tests/unit/integration/test_command_integrator.py: 4 new tests in TestCursorCommandPanelFindings: - test_passthrough_notice_suppressed_on_clean_install - test_passthrough_notice_emitted_when_any_file_drops_keys - test_dropped_keys_warn_uses_user_facing_wording - test_critical_security_finding_blocks_write Updated the existing test_dropped_frontmatter_keys_warn assertion to match the new wording. * tests/integration/test_marketplace_plugin_integration.py: 2 new integration tests locking in cursor 1.6+ command deployment via the install pipeline (deploy when .cursor/ exists; respect opt-in by NOT creating .cursor/ when missing). Verification: uv run --extra dev ruff check src/ tests/ -> All checks passed! uv run --extra dev ruff format --check src/ tests/ -> 627 files OK uv run --extra dev pytest tests/unit ... -> 7222 passed Refs: microsoft#1046 (comment) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
commandsprimitive in the Cursor target profile soapm installdeploys.prompt.mdfiles to.cursor/commands/*.mdwhen a.cursor/directory is presentDesign rationale
Cursor 1.6 introduced custom slash commands stored as plain Markdown in
.cursor/commands/. Cursor has since de-emphasized commands in favor of skills/rules, but as long as it supports the commands surface it makes sense to deploy to it.Frontmatter strategy: Cursor itself does not consume YAML frontmatter -- the file is just a context-to-LLM routing mechanism. We deliberately emit Claude-compatible frontmatter (
description,allowed-tools,arguments,argument-hint) rather than stripping it. In testing, leaving structured input hints in the file makes commands noticeably more successful at inferring required inputs from the user. The frontmatter is harmless to Cursor.Implementation: This reuses the existing
integrate_command()path (theelsebranch inCommandIntegrator.integrate_commands_for_target), so the only production code change is 3 lines intargets.pyadding aPrimitiveMapping.Test plan
TestCursorCommandIntegration, 6 tests)test_cursor_target_dispatches_commands)integrate_package_primitivesdispatch with real integrators, verifying.cursor/commands/review.mdoutput including frontmatter content (TestCursorCommandEndToEnd)test_partition_parity_with_old_bucketsupdated withcommands_cursorbucket