fix(cli): skip update check for unknown subcommands to fail fast#1541
Open
maro114510 wants to merge 1 commit into
Open
fix(cli): skip update check for unknown subcommands to fail fast#1541maro114510 wants to merge 1 commit into
maro114510 wants to merge 1 commit into
Conversation
Author
|
@microsoft-github-policy-service agree |
maro114510
requested review from
danielmeppiel and
sergio-sisternes-epam
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the CLI’s update-notification behavior so it only runs for recognized subcommands, and adds integration tests to verify update checks are skipped for invalid/unknown commands.
Changes:
- Gate
_check_and_notify_updates()behind a “known subcommand” check in the CLI entrypoint. - Add integration tests covering invalid command, valid command, and no-subcommand scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/integration/test_version_notification.py | Adds integration tests ensuring the update check is skipped for invalid/unknown commands and when no subcommand is provided. |
| src/apm_cli/cli.py | Changes update-check gating logic to run only when a recognized subcommand is invoked. |
Comment on lines
+67
to
69
| # Check for updates only for known commands; skip on invalid input to fail fast. | ||
| if not ctx.resilient_parsing and ctx.invoked_subcommand in cli.commands: | ||
| _check_and_notify_updates() |
danielmeppiel
added a commit
to danielmeppiel/genesis
that referenced
this pull request
May 29, 2026
…S discipline Dispatched v0.3+ panel against microsoft/apm#1541 (+41/-2, 2 files, small CLI fix). Results: - Executor cost: ~$0.21 (vs $2.85 on PR #1424) - Per-kLoC: $5.12 (vs $1.15 baseline) — fixed Sonnet-executor overhead dominates at small scale (~93% of total cost) - Panel cost shape holds in dollar terms; per-kLoC ratio inverted - Arbiter trigger correctly did NOT fire (0 BLOCKERs) KEY EMPIRICAL VALIDATION of v0.3.4 PER-LENS DIFFERENTIATION: The executor reflected per-lens against the CAPABILITY PROFILE template and concluded 4/5 lenses genuinely TRIVIAL, but security lens was INADEQUATE on TRIVIAL/Haiku — it surfaced a real MEDIUM bypass concern but could not validate it without out-of-diff function body access. This empirically generates the per-element justification the v0.3.4 corpus requires architects to record at design time. Recommended carve-out: 'Security lens uses Haiku when all referenced functions are in-diff; escalates to REVIEWER with tool access when it must reason about out-of-diff internals.' Implication for PR #1424: security lens was likely mis-bound to TRIVIAL. The blocker false-positive (_substitute_plugin_root alleged undefined, refuted only via out-of-diff gh api lookup) is consistent with TRIVIAL- class inadequacy on cross-file reasoning. A v0.3.4 re-architect would correctly bind security to REVIEWER, expected cost delta +$0.50-1.00 per run with measurable security finding fidelity improvement. REPORT updated with multi-scenario section (small-PR + different-skill). Deferral list narrowed: full S1-S5 × {v0.2,v0.3+} matrix remains follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for maintaining this project. I opened this PR to fix a small issue I noticed.
Description
The update check ran in the CLI group handler, which Click invokes before resolving the subcommand name.
As a result, even a typo like
apm updsatewould block on a GitHub API call (synchronous, up to 2 s timeout) before theNo such commanderror was shown.The docstring said "non-blockingly" but the call was fully synchronous.
The guard
ctx.invoked_subcommand in cli.commandsis the same dict lookup Click uses internally when resolving commands, so there's no duplication of logic—just an early check.One side effect: bare
apmwith no subcommand also skips the check now (invoked_subcommandisNone), which is intentional since there's nothing to execute.Type of change
Testing