Add MCP Phase 2 CLI compatibility aliases by axisrow · Pull Request #9 · axisrow/direct-cli

axisrow · 2026-04-09T07:07:19Z

Summary

align direct-cli with the CLI surface expected by [MCP plugin] Этап 2 — MCP tools для всех 25 сервисов API yandex-direct-mcp-plugin#18
add MCP Phase 2-compatible top-level aliases for dynamic, smart, and negative keyword resources
add list and short-form command aliases expected by the plugin contract
fill missing update/delete/toggle command handlers and document the compatibility layer

Compatibility

keep existing command groups working
add canonical aliases such as dynamictargets, smarttargets, and negativekeywords
add alias subcommands such as list, checkcamp, checkdict, and has-volume

Testing

python3 -m pytest -q
python3 -m pytest tests/test_cli.py -q

Notes

This PR is focused on local CLI/help compatibility and test coverage for the MCP integration contract. Live API validation for newly exposed handlers still needs cassette or integration verification.

axisrow · 2026-04-09T07:11:55Z

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

claude · 2026-04-09T07:12:13Z

Claude finished @axisrow's task in 1m 53s —— View job

PR Review — MCP Phase 2 CLI Compatibility Aliases

Tasks:

Read CLAUDE.md for repo guidelines
Read changed files for critical issues
Analyze for bugs, security vulnerabilities, logical errors, data loss risks, performance problems
Post findings

Findings

Two definite bugs that will fail at runtime; one likely incorrect API call.

Bug 1 — `adimages delete`: wrong SelectionCriteria field and wrong option type

File: direct_cli/commands/adimages.py:94-112

@click.option("--id", "image_id", required=True, type=int, help="Image ID")
...
body = {"method": "delete", "params": {"SelectionCriteria": {"Ids": [image_id]}}}

The CLAUDE.md explicitly documents: "adimages uses AdImageHash, not Id."

The Yandex Direct API for adimages.delete expects AdImageHashes (an array of hash strings) in the SelectionCriteria, not Ids. Using Ids here will produce an API error. Compounding this, the --id option is typed as int, but an AdImageHash is a string (e.g., "hash_abc123").

The correct implementation should use --hash / AdImageHashes:

@click.option("--hash", "image_hash", required=True, help="Ad image hash")
...
body = {"method": "delete", "params": {"SelectionCriteria": {"AdImageHashes": [image_hash]}}}

Fix this →

Bug 2 — `bidmodifiers toggle`: Python `bool` sent where API expects string enum `"YES"`/`"NO"`

File: direct_cli/commands/bidmodifiers.py:114-143

@click.option("--enabled/--disabled", "enabled", default=True, ...)
...
body = {
    "method": "set",
    "params": {"BidModifiers": [{"Id": modifier_id, "Enabled": enabled}]}
}

Click's --enabled/--disabled flag produces a Python bool. When serialized to JSON by the HTTP client, True → true and False → false. The Yandex Direct API uses the BooleanEnum type for Enabled, which accepts the string values "YES" or "NO" — not JSON booleans. The API will reject this with a type validation error.

Fix:

"Enabled": "YES" if enabled else "NO",

Fix this →

Possible Bug — `agencyclients delete`: API may not support this operation

File: direct_cli/commands/agencyclients.py:93-112

The Yandex Direct API v5 does not expose a delete method for AgencyClients. Agency client accounts cannot be deleted via the API — they can only be modified. This command will produce an API error (Method not found or similar) at runtime. If you don't have a reference confirming this endpoint exists, this command should be removed or replaced with a clear print_error("Agency clients cannot be deleted via the API") stub.

No Issues Found

All list alias registrations via add_command(get, name="list") are correct.
The three top-level aliases (dynamictargets, smarttargets, negativekeywords) in cli.py are registered correctly.
keywordsresearch.deduplicate correctly uses result.data — consistent with the existing get and has_search_volume commands in that file.
changes short-form aliases (checkcamp, checkdict) are correct.
No security vulnerabilities — json.loads on CLI --json flags is fine for a local tool.
extra_json update-overwrite pattern (where target_data.update(extra) could overwrite Id) is a design choice consistent with the rest of the codebase; not a new risk.

Copilot

Pull request overview

Adds an MCP Phase 2 compatibility layer to the Direct CLI by introducing canonical top-level aliases for certain resources and adding commonly expected subcommand aliases (notably list and short-form names), while filling in a few previously missing handlers.

Changes:

Register top-level canonical aliases: dynamictargets, smarttargets, negativekeywords.
Add list aliases across many resource groups (mapping to existing get), plus short-form aliases like changes checkcamp/checkdict and keywordsresearch has-volume.
Add missing update/delete/toggle handlers for several resources and document the compatibility layer in README.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_comprehensive.py	Extends expected top-level command registration list to include new canonical aliases.
tests/test_cli.py	Adds help/usage tests for canonical aliases, short aliases, and `list` alias behavior.
README.md	Documents plugin-compatible alias names (EN/RU sections).
direct_cli/cli.py	Registers the new top-level canonical alias command names.
direct_cli/commands/changes.py	Adds short aliases `checkcamp` and `checkdict`.
direct_cli/commands/keywordsresearch.py	Adds `deduplicate` command and `has-volume` alias for `has_search_volume`.
direct_cli/commands/dynamicads.py	Adds `update`/`delete` handlers and `list` alias.
direct_cli/commands/smartadtargets.py	Adds `update`/`delete` handlers and `list` alias.
direct_cli/commands/negativekeywordsharedsets.py	Adds `update`/`delete` handlers, imports `json`, and `list` alias.
direct_cli/commands/bidmodifiers.py	Adds `toggle` and `delete` handlers plus `list` alias.
direct_cli/commands/feeds.py	Adds `update`/`delete` handlers and `list` alias.
direct_cli/commands/agencyclients.py	Adds `add`/`delete` handlers, imports `json`, and `list` alias.
direct_cli/commands/adimages.py	Adds `add`/`delete` handlers, imports `json`, and `list` alias.
direct_cli/commands/adextensions.py	Adds `delete` handler and `list` alias.
direct_cli/commands/sitelinks.py	Adds `delete` handler and `list` alias.
direct_cli/commands/retargeting.py	Adds `delete` handler and `list` alias.
direct_cli/commands/vcards.py	Adds `delete` handler and `list` alias.
direct_cli/commands/turbopages.py	Adds `list` alias.
direct_cli/commands/leads.py	Adds `list` alias.
direct_cli/commands/keywords.py	Adds `list` alias.
direct_cli/commands/dictionaries.py	Adds `list` alias for `get`.
direct_cli/commands/creatives.py	Adds `list` alias.
direct_cli/commands/clients.py	Adds `list` alias.
direct_cli/commands/campaigns.py	Adds `list` alias.
direct_cli/commands/bids.py	Adds `list` alias.
direct_cli/commands/audiencetargets.py	Adds `list` alias.
direct_cli/commands/ads.py	Adds `list` alias.
direct_cli/commands/adgroups.py	Adds `list` alias.

Comments suppressed due to low confidence (1)

direct_cli/commands/adimages.py:116

adimages.add_command(get, name="list") relies on adimages.get() defaults, but get() hard-codes FieldNames to ['Id', 'Name', 'Status', 'AdImageHash'], which diverges from the central defaults in utils.COMMON_FIELDS for adimages (['AdImageHash', 'Name']). This creates inconsistent behavior across resources and makes the new list alias potentially incompatible with shared expectations; consider switching adimages.get() to use get_default_fields('adimages') (or otherwise aligning defaults).

adimages.add_command(get, name="list")

Copilot · 2026-04-09T07:12:17Z

+@adimages.command()
+@click.option("--id", "image_id", required=True, type=int, help="Image ID")
+@click.pass_context
+def delete(ctx, image_id):


adimages uses AdImageHash as its identifier (see utils.COMMON_FIELDS['adimages']), but this new delete command accepts an integer --id and builds SelectionCriteria.Ids. This will likely fail against the API and is inconsistent with how ad images are keyed. Consider switching the option to accept the hash (string) and delete by hash-based selection criteria instead of numeric IDs.

Copilot · 2026-04-09T07:12:18Z

+        )
+
+        body = {"method": "delete", "params": {"SelectionCriteria": {"Ids": [image_id]}}}
+


The delete request for ad images is constructed with SelectionCriteria: {Ids: [...]}. For ad images the API key is AdImageHash, so deletion should target hashes rather than IDs; otherwise the endpoint will likely return an error or delete the wrong entity type.

- adimages delete: use --hash/AdImageHashes instead of --id/Ids, fix get default fields to use COMMON_FIELDS - bidmodifiers toggle: send "YES"/"NO" strings instead of Python bool - agencyclients delete: replace with stub, API does not support delete Co-Authored-By: Claude Opus 4.6 <[email protected]>

axisrow · 2026-04-09T07:23:20Z

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

claude · 2026-04-09T07:23:33Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

axisrow · 2026-04-09T07:24:38Z

@claude re-review the PR after the latest fixes. Focus on critical issues only.

claude · 2026-04-09T07:24:51Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

axisrow · 2026-04-09T07:34:09Z

@claude review this PR. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems.

claude · 2026-04-09T07:34:25Z

Claude finished @axisrow's task in 2m 17s —— View job

PR Review — MCP Phase 2 CLI Compatibility Aliases

Tasks:

Read CLAUDE.md for repo guidelines
Read changed files for critical issues
Check if previously identified bugs were fixed
Analyze for new bugs, security vulnerabilities, logical errors, data loss risks, performance problems
Post findings

Previously Reported Bugs — Status

All three issues from the first review are resolved:

Bug 1 (adimages delete — wrong field/type): Fixed. Now uses --hash (string) and AdImageHashes in SelectionCriteria. (adimages.py:94-107)
Bug 2 (bidmodifiers toggle — Python bool instead of "YES"/"NO"): Fixed. Now correctly serializes as "YES" if enabled else "NO". (bidmodifiers.py:126)
Concern (agencyclients delete — unsupported API operation): Addressed. The command now immediately calls print_error and aborts with a clear message instead of sending an invalid request. (agencyclients.py:94-103)

New Findings

Two new runtime bugs introduced in this PR.

Bug 1 — `negativekeywordsharedsets add`/`update`: keywords wrapped in objects instead of plain strings

File: direct_cli/commands/negativekeywordsharedsets.py:76 and 114-115

# add — line 76
"NegativeKeywords": [{"Keyword": k.strip()} for k in keywords.split(",")]

# update — line 114-115
set_data["NegativeKeywords"] = [
    {"Keyword": k.strip()} for k in keywords.split(",")
]

The Yandex Direct API expects NegativeKeywords to be a flat array of strings, not an array of objects:

"NegativeKeywords": ["keyword1", "keyword2"]

Wrapping each keyword in {"Keyword": ...} will cause an API schema validation error on both add and update. The fix is simple:

# add
"NegativeKeywords": [k.strip() for k in keywords.split(",")]

# update
set_data["NegativeKeywords"] = [k.strip() for k in keywords.split(",")]

Fix this →

Bug 2 — `dynamicads add`/`update`: `Condition` is a plain string; API expects `Conditions` as an array of condition objects

File: direct_cli/commands/dynamicads.py:80 and 118

# add — line 80
target_data = {"AdGroupId": adgroup_id, "Condition": condition}

# update — line 118
target_data["Condition"] = condition

The Yandex Direct API for Webpages uses the field Conditions (plural), which must be an array of WebpageCondition objects, each with Arguments (array of strings) and Operator (enum, e.g. CONTAINS_ANY, EQUALS_ANY):

"Conditions": [{"Arguments": ["product"], "Operator": "CONTAINS_ANY"}]

The current code sends a single "Condition": "contains:product" string, which will fail API schema validation. The --condition option's format ("contains:product") also doesn't parse into the structured object the API requires.

The cleanest fix is to drop the --condition shortcut and instead require the full --json parameter for conditions, similar to how adextensions add works. Alternatively, if you want to keep a convenience flag, it needs to parse the string into the correct WebpageCondition shape.

Fix this →

No Issues Found

adimages get using Ids in SelectionCriteria is correct (distinct from delete which uses AdImageHashes).
agencyclients get using get_default_fields("clients") is intentional — the agencyclients key is absent from COMMON_FIELDS, and client fields apply equally.
All list aliases (add_command(get, name="list")) are correctly registered across all modules.
Top-level aliases dynamictargets, smarttargets, negativekeywords in cli.py:138-140 are correct.
changes checkcamp/checkdict aliases are correct.
No security vulnerabilities.

- negativekeywordsharedsets: send keywords as flat string array, not wrapped in {"Keyword": ...} objects - dynamicads: use "Conditions" (plural) in get fields, replace --condition with --json for add/update to match API structure Co-Authored-By: Claude Opus 4.6 <[email protected]>

axisrow · 2026-04-09T07:48:20Z

@claude review this PR after latest fixes. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences.

claude · 2026-04-09T07:48:34Z

Claude finished @axisrow's task in 2m 4s —— View job

PR Re-Review — MCP Phase 2 CLI Compatibility Aliases

Tasks:

Read CLAUDE.md for repo guidelines
Read changed files to check latest state
Verify previously reported bugs are fixed
Check for new critical issues
Post findings

Previously Reported Bugs — All Fixed

All bugs identified in prior review rounds are resolved:

Bug	File	Status
`adimages delete` used `Ids` (int) instead of `AdImageHashes` (string)	`adimages.py:94-108`	Fixed — now `--hash` + `AdImageHashes`
`bidmodifiers toggle` sent Python `bool` instead of `"YES"`/`"NO"`	`bidmodifiers.py:126`	Fixed — `"YES" if enabled else "NO"`
`agencyclients delete` called an unsupported API endpoint	`agencyclients.py:98-103`	Fixed — immediately errors with clear message
`negativekeywordsharedsets add/update` wrapped keywords in `{"Keyword": ...}` objects	`negativekeywordsharedsets.py:76, 114`	Fixed — now flat string list
`dynamicads add/update` sent `"Condition"` (string) instead of `"Conditions"` (array of objects)	`dynamicads.py`	Fixed — `--condition` removed, full `--json` required

New Findings

No new critical issues. Specific checks performed:

bidmodifiers.delete — SelectionCriteria: {Ids: [id]} is the correct pattern for this API.
negativekeywordsharedsets.delete — same, correct.
dynamicads.add now takes --json and passes through to Webpages — correct; the user is responsible for providing the Conditions array.
dynamicads.update — same pattern, correct.
smartadtargets add/update/delete — structure looks consistent with the API contract.
feeds.update uses "Source": url — this pattern is pre-existing in the add command on main and not new to this PR.
keywordsresearch.deduplicate — uses result.data consistently with other commands in the same file.
changes.checkcamp/checkdict aliases — registered correctly.
Top-level dynamictargets, smarttargets, negativekeywords aliases — registered correctly in cli.py:138-140.
All list alias registrations via add_command(get, name="list") — correct across all 20+ modules.
adimages.get using Ids in SelectionCriteria — correct (distinct from delete which uses AdImageHashes; get does accept numeric IDs).
No security vulnerabilities.

This PR is clean. All previously identified bugs are resolved and no new issues were found.

* fix(auth): stop --help and unit tests from hanging on client-login network call A regression from #480 made get_credentials() resolve the bare Client-Login via a network clients.get on every CLI invocation when an OAuth profile with an email login lacked the login_migration_checked flag — including plain `<group> --help`. That call had no timeout (neither the vendored client nor tapi2 set one), so a slow link or a Yandex SmartCaptcha gateway could hang the CLI indefinitely. Under CliRunner this also hung the unit suite (it stalled on test_registered_mapped_groups_show_docs_url, which fires ~30 `--help` calls). Three independent guards: - auth.py: cap the best-effort resolver with LOGIN_RESOLVE_TIMEOUT_SECONDS=8 and disable retries, so it can never wait forever. - cli.py: skip the resolver entirely on --help/--version/-h passes (allow_login_resolve=False) — no command runs, so no login is needed. - tests/conftest.py: autouse fixture neutralizes the network resolver for the whole unit suite; the module-level test-credential probe no longer triggers it. Adds regression tests asserting a --help pass makes zero resolver calls and that allow_login_resolve=False suppresses the migration. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix: resolve 13 confirmed bugs from the #483 bug hunt Verified each finding against the live API, the WSDL cache, and the official Yandex Direct docs before fixing. Bugs 10 and 14 were false positives (no UsageError source inside the try — style only) and are intentionally skipped. Functional fixes: - bids/keywordbids get: reject empty SelectionCriteria with a UsageError before the request (live API error 4001) (#1, #2). - bids get: re-raise UsageError so the new guard surfaces with exit code 2 (#8). - bids set-auto: require exactly one of CampaignId/AdGroupId/KeywordId via add_single_id_selector (docs: the three are mutually exclusive) (#4). - vendored to_columns: pad short report rows instead of IndexError (#3). - vendored error handler: error_data.get("error_detail") to avoid KeyError (#11). - reports build_report_request: reject empty FieldNames (live API error 8000) (#12). Swallowed-validation fixes (add `except click.UsageError/ClickException: raise` before the bare `except Exception`): balance (#6), strategies get (#7), campaigns get (#9), ads get (#13), negativekeywordsharedsets update (#5). Type annotation: parse_priority_goals_spec items -> List[Dict[str, Any]] (#15). build_api_coverage_report: supply --campaign-ids for bids/keywordbids get wire-capture so the empty-criteria guard does not break schema parity. Closes #483 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix: broaden #483 coverage and add CHANGELOG (cherry-picked from #484) Incorporates the genuinely-better parts of the parallel PR #484 into this branch, which already carries the regression tests, the #15 annotation fix, and the localized messages: - ads: re-raise click.UsageError in all lifecycle handlers (update, delete, archive, unarchive, suspend, resume, moderate) in addition to get — so the whole resource matches bug #13's "get + 8 lifecycle" scope. Defensive: these handlers have no pre-API UsageError source today, but the guard keeps them consistent and future-proof. - retargeting get/delete, advideos get: same defensive re-raise for uniformity (issue #483 items 10/14; not live defects, but harmless consistency). - bids/keywordbids get: the empty-criteria message now also lists --serving-statuses, which likewise populates SelectionCriteria (kept the t() localization wrapper). - CHANGELOG.md: document the bug-hunt fixes (#483) and the --help network-hang follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: axisrow <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

Add MCP Phase 2 CLI compatibility aliases

33079f9

Copilot AI review requested due to automatic review settings April 9, 2026 07:07

Copilot started reviewing on behalf of axisrow April 9, 2026 07:08 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

axisrow merged commit 4b4d3c7 into main Apr 9, 2026
1 check passed

axisrow deleted the feat/mcp-phase2-cli-contract branch April 9, 2026 08:01

axisrow mentioned this pull request Apr 11, 2026

test: Этап 3 — доработки sandbox integration_write (record/replay + regression-guards) #16

Closed

axisrow mentioned this pull request Apr 23, 2026

breaking: define canonical direct-cli naming convention and command examples #42

Closed

axisrow mentioned this pull request May 21, 2026

CLI subtype validation: validate --type-specific flags against WSDL subtype fields #210

Closed

axisrow mentioned this pull request May 30, 2026

fix: resolve 13 confirmed bugs from the #483 bug hunt #486

Merged

		)

		body = {"method": "delete", "params": {"SelectionCriteria": {"Ids": [image_id]}}}

Conversation

axisrow commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Compatibility

Testing

Notes

Uh oh!

axisrow commented Apr 9, 2026

Uh oh!

claude Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — MCP Phase 2 CLI Compatibility Aliases

Findings

Bug 1 — adimages delete: wrong SelectionCriteria field and wrong option type

Bug 2 — bidmodifiers toggle: Python bool sent where API expects string enum "YES"/"NO"

Possible Bug — agencyclients delete: API may not support this operation

No Issues Found

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

axisrow commented Apr 9, 2026

Uh oh!

claude Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

axisrow commented Apr 9, 2026

Uh oh!

claude Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

axisrow commented Apr 9, 2026

Uh oh!

claude Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — MCP Phase 2 CLI Compatibility Aliases

Previously Reported Bugs — Status

New Findings

Bug 1 — negativekeywordsharedsets add/update: keywords wrapped in objects instead of plain strings

Bug 2 — dynamicads add/update: Condition is a plain string; API expects Conditions as an array of condition objects

No Issues Found

Uh oh!

axisrow commented Apr 9, 2026

Uh oh!

claude Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Re-Review — MCP Phase 2 CLI Compatibility Aliases

Previously Reported Bugs — All Fixed

New Findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

axisrow commented Apr 9, 2026 •

edited

Loading

claude Bot commented Apr 9, 2026 •

edited

Loading

Bug 1 — `adimages delete`: wrong SelectionCriteria field and wrong option type

Bug 2 — `bidmodifiers toggle`: Python `bool` sent where API expects string enum `"YES"`/`"NO"`

Possible Bug — `agencyclients delete`: API may not support this operation

claude Bot commented Apr 9, 2026 •

edited

Loading

claude Bot commented Apr 9, 2026 •

edited

Loading

claude Bot commented Apr 9, 2026 •

edited

Loading

Bug 1 — `negativekeywordsharedsets add`/`update`: keywords wrapped in objects instead of plain strings

Bug 2 — `dynamicads add`/`update`: `Condition` is a plain string; API expects `Conditions` as an array of condition objects

claude Bot commented Apr 9, 2026 •

edited

Loading