Skip to content

Add Claude Code GitHub Workflow#1

Merged
axisrow merged 2 commits into
mainfrom
add-claude-github-actions-1773386364626
Mar 13, 2026
Merged

Add Claude Code GitHub Workflow#1
axisrow merged 2 commits into
mainfrom
add-claude-github-actions-1773386364626

Conversation

@axisrow

@axisrow axisrow commented Mar 13, 2026

Copy link
Copy Markdown
Owner

🤖 Installing Claude Code GitHub App

This PR adds a GitHub Actions workflow that enables Claude Code integration in our repository.

What is Claude Code?

Claude Code is an AI coding agent that can help with:

  • Bug fixes and improvements
  • Documentation updates
  • Implementing new features
  • Code reviews and suggestions
  • Writing tests
  • And more!

How it works

Once this PR is merged, we'll be able to interact with Claude by mentioning @claude in a pull request or issue comment.
Once the workflow is triggered, Claude will analyze the comment and surrounding context, and execute on the request in a GitHub action.

Important Notes

  • This workflow won't take effect until this PR is merged
  • @claude mentions won't work until after the merge is complete
  • The workflow runs automatically whenever Claude is mentioned in PR or issue comments
  • Claude gets access to the entire PR or issue context including files, diffs, and previous comments

Security

  • Our Anthropic API key is securely stored as a GitHub Actions secret
  • Only users with write access to the repository can trigger the workflow
  • All Claude runs are stored in the GitHub Actions run history
  • Claude's default tools are limited to reading/writing files and interacting with our repo by creating comments, branches, and commits.
  • We can add more allowed tools by adding them to the workflow file like:
allowed_tools: Bash(npm install),Bash(npm run build),Bash(npm run lint),Bash(npm run test)

There's more information in the Claude Code action repo.

After merging this PR, let's try mentioning @claude in a comment on any PR to get started!

@axisrow axisrow merged commit b814846 into main Mar 13, 2026
1 check passed
@axisrow axisrow deleted the add-claude-github-actions-1773386364626 branch March 13, 2026 07:24
@claude claude Bot mentioned this pull request Mar 13, 2026
4 tasks
axisrow added a commit that referenced this pull request Apr 28, 2026
…y, advideos discovery (#119)

* fix: close #112 with imported XSD validation, runtime-deprecated registry, advideos discovery

Closes #112 by addressing the two open bug classes from the audit (the other
three were resolved on main).

PR-A — Runtime-deprecated method registry (closes AC #3):
- Add RUNTIME_DEPRECATED_METHODS to direct_cli/wsdl_coverage.py (registry of
  WSDL-present methods that Yandex rejects at runtime, e.g. agencyclients.add
  with error_code=3500).
- Add assert_not_runtime_deprecated() helper in direct_cli/utils.py raising
  click.UsageError with a stable replacement hint.
- Wire the assertion into agencyclients.add so both real-call and --dry-run
  paths fail with a clear message before request construction.
- Add 8th schema gate bucket runtime_deprecated_unguarded in
  scripts/build_api_coverage_report.py: walks RUNTIME_DEPRECATED_METHODS, uses
  the captured-client harness to verify each entry's CLI command actually
  rejects with UsageError before reaching the API. Bucket flips
  schema_parity_ok to False; mirrored into report["schema"] and a top-level
  report["runtime_deprecated_methods"] section for machine-readable parity.

PR-B — Imported XSD validation + AdVideos smoke discovery (closes AC #1, #4):
- Cache imported XSD schemas in tests/wsdl_cache/imports/ (general.xsd,
  generalclients.xsd, adextensiontypes.xsd). Add IMPORTED_XSD_REGISTRY
  (namespace -> filename) and fetch_imported_xsd() in wsdl_coverage.py.
- Resolve <xsd:import> nested types in get_operation_request_schema() via
  _load_schema_contexts(), and walk extension/@base inheritance so item_fields
  for types like gc:NotificationAdd and gc:ClientUpdateItem now contain
  resolved nested + inherited fields (e.g. Notification.EmailSubscriptions,
  Clients[].Settings).
- Add find_nested_schema_violations() in wsdl_coverage.py (recursive walk of
  body keys against the resolved schema).
- Add 9th schema gate bucket nested_schema_violations: dry-runs every entry
  in PAYLOAD_CASES (excluding DRY_RUN_PAYLOAD_EXCLUSIONS) and flags any
  unknown nested key.
- Extract get_first_advideo_probe_id from tests/test_integration.py into
  reusable direct_cli/_smoke_probes.py module with CLI entry point
  (python3 -m direct_cli._smoke_probes advideo). Honors
  YANDEX_DIRECT_TEST_ADVIDEO_ID env override; returns None on any failure
  so smoke scripts can skip without falsely passing. Includes graceful
  handling of create_client failures (auth/network).
- Update scripts/test_safe_commands.sh to use run_advideos_probe_get with
  [INFO] skip-on-no-id semantics instead of dummy --ids 1.
- Extend refresh_all_caches() and scripts/check_wsdl_drift.py to also
  refresh and monitor IMPORTED_XSD_REGISTRY entries.

Tests added (9 new):
PR-A: registry shape, block invocation (real + --dry-run paths), schema gate
flags unguarded entry.
PR-B: imported XSD cache↔registry parity, resolves imports
(Notification.{Email,Lang,EmailSubscriptions}), resolves extension/@base
fields (clients.update Clients[].Notification/Settings), payload nested
violation flagging, smoke probe returns None gracefully without credentials.

Schema gate goes from 7 to 9 buckets, all green: schema_parity_ok=True.
Full test suite: 338 passed, 29 skipped (integration, no creds), 0 failed.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(schema-gate): unify dry_run_failed classification across click versions

Older click (Python 3.9) returns exit_code=0 with usage/error text in
stdout when a missing CLI group is invoked, while newer click returns
exit_code=2. The previous logic took two paths (exit-code-based
dry_run_failed vs. JSON-parse-based invalid_json), so the same input
produced different bucket kinds across versions.

Collapse to a single dry_run_failed kind: if either the exit code is
non-zero or the output is not parseable JSON, the case is reported as
dry_run_failed with the relevant error text. invalid_json was redundant
and not asserted anywhere.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix: address PR #119 review findings

- Dedupe nested-schema violation reports: the recursive call already
  walks the same expected-key logic for each child, so the inline loop
  that appended unknown direct keys produced duplicate entries
  (clients.update Email/ClientId would each show up twice in the gate
  output). Delegate unknown-key detection entirely to the recursion.
- Add XSD_BASE_URL constant for tests/wsdl_cache/imports/ refresh; the
  hardcoded soap.direct.yandex.ru URL is now grep-able and documented
  alongside WSDL_BASE_URL.
- Add test_runtime_deprecated_methods_have_capture_fixture to enforce
  that every RUNTIME_DEPRECATED_METHODS entry has a complete
  RUNTIME_DEPRECATED_CAPTURE_FIXTURES record covering all of the target
  command's required options. Without this, Click would fail with
  "Missing option ..." before the deprecation guard runs and the gate
  would misclassify a guarded method as unguarded. Added a docstring on
  RUNTIME_DEPRECATED_CAPTURE_FIXTURES referencing the same invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* perf(schema-gate): avoid duplicate get_operation_field_name_enums calls

The _field_name_operations comprehension called the function twice per
operation: once in the if-filter and once as the value. Each call walks
get_operation_request_schema → _load_schema_contexts → parse_wsdl_field_enums,
so for ~27 services × ~5 operations this was ~270 redundant
schema-context loads per coverage report build. Compute the dict once,
then filter empty values.

No behavior change. Pre-existing inefficiency that became more visible
once _load_schema_contexts started resolving imported XSD types.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
axisrow added a commit that referenced this pull request May 18, 2026
…via CLI

Addresses Codex review finding #2 on #179: write-mutating tests were
inheriting the v4_live_read file-level marker, breaking the trust
boundary between read-only and write test suites.

- Removes test_v4_live_wordstat_lifecycle_contract_opt_in_write and
  test_v4_live_forecast_lifecycle_contract_opt_in_write from
  tests/test_v4_live_contracts.py (which is correctly marked v4_live_read).
- Adds test_live_draft_v4wordstat_lifecycle and
  test_live_draft_v4forecast_lifecycle to tests/test_integration_live_write.py
  under the integration_live_write marker.
- Rewrites them to use the CLI (via _invoke_live / CliRunner) instead of
  native call_v4, matching the convention of every other live-write test
  in that file.
- Drops in-test YANDEX_DIRECT_LIVE_WRITE checks — the file-level skipif
  already enforces the env gate.

Verified:
  pytest --collect-only -m v4_live_read tests/test_v4_live_contracts.py
    → 9 read-only tests (no lifecycle probes).
  pytest --collect-only -m integration_live_write
    tests/test_integration_live_write.py
    → both v4 lifecycle tests included.
  env -u YANDEX_DIRECT_LIVE_WRITE pytest <…lifecycle…>
    → SKIPPED via file-level skipif.

Codex review finding #1 (bare `pytest` hits live API when profile is
active) is intentionally deferred: project decision is that v4 live
read tests should run whenever a profile is available.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
axisrow added a commit that referenced this pull request May 19, 2026
…177)

* test(v4_live): add live contracts for v4forecast, v4tags, v4wordstat

Refs #174.

- wordstat lifecycle (create/list/delete) under v4_live_read marker
- forecast lifecycle (create/list/delete) under v4_live_read marker
- tags GetCampaignsTags read-only via existing _campaign_id helper
- tags GetBannersTags read-only via existing _campaign_id helper

GetForecast and GetWordstatReport are out of scope (require polling).
UpdateBannersTags / UpdateCampaignsTags are out of scope (write).
Tests auto-skip without YANDEX_DIRECT_TOKEN / YANDEX_DIRECT_LOGIN.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(v4_live): gate wordstat/forecast lifecycle behind LIVE_WRITE opt-in

Addresses Codex review on #177: CreateNew*/Delete* are write operations
and must not run as part of the read-only v4_live_read tier. Follows the
existing pattern of test_v4_live_create_invoice_contract_opt_in_write
(line 140) — env-gate inside the test body with explicit _opt_in_write
suffix in the function name.

Behaviour:
- Without YANDEX_DIRECT_LIVE_WRITE=1: tests skip (even with token+login).
- With YANDEX_DIRECT_LIVE_WRITE=1: tests run and mutate the live account
  with a single create/delete cycle.

Tags read-only tests (GetCampaignsTags, GetBannersTags) are unchanged —
they remain pure reads under v4_live_read.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test: relax 'No such option' asserts for Click 8.x format

Click 8.x changed the missing-option error format from
  "No such option: --foo"
to
  "No such option '--foo'. Did you mean '--bar'?"

This broke three legacy-flag rejection tests on Python 3.11/3.13 CI
runners (3.9 still ships an older Click). Split each assertion into
two checks — "No such option" + the flag name — which match both the
old and new formats.

Fixes the CI failure inherited from main on #177.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(v4_live): apply Claude review polish (next/single-arg/TODOs)

Addresses Claude review on #177:
- Use next() to fetch the just-created report/forecast from the list and
  validate its schema directly, instead of asserting on reports[0] which
  may belong to a prior run.
- Drop redundant third argument to call_v4(..., "List*", None) — param
  defaults to None.
- Add TODOs flagging two assertions to tighten after first live run:
  Delete* return value (== 1 per docs) and GetBannersTags field set
  (likely {BannerID, TagIDS}, mirroring UpdateBannersTags writes).

Behaviour unchanged for both skip path and live path; lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(v4_live): resolve credentials from direct auth profile when env is unset

Companion fix to #179 (auth-profile fallback in WRITE_SANDBOX runners).
Previously _credentials() silently skipped all v4_live_read tests for
profile-only users — manual pytest -m v4_live_read showed no signal.

Now: when env vars are missing, try direct_cli.auth.get_credentials() to
resolve the profile (matches the priority used by every CLI subcommand).
Skip message also tells the user both options.

Refs #178.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(v4_live): move write lifecycle probes to integration_live_write via CLI

Addresses Codex review finding #2 on #179: write-mutating tests were
inheriting the v4_live_read file-level marker, breaking the trust
boundary between read-only and write test suites.

- Removes test_v4_live_wordstat_lifecycle_contract_opt_in_write and
  test_v4_live_forecast_lifecycle_contract_opt_in_write from
  tests/test_v4_live_contracts.py (which is correctly marked v4_live_read).
- Adds test_live_draft_v4wordstat_lifecycle and
  test_live_draft_v4forecast_lifecycle to tests/test_integration_live_write.py
  under the integration_live_write marker.
- Rewrites them to use the CLI (via _invoke_live / CliRunner) instead of
  native call_v4, matching the convention of every other live-write test
  in that file.
- Drops in-test YANDEX_DIRECT_LIVE_WRITE checks — the file-level skipif
  already enforces the env gate.

Verified:
  pytest --collect-only -m v4_live_read tests/test_v4_live_contracts.py
    → 9 read-only tests (no lifecycle probes).
  pytest --collect-only -m integration_live_write
    tests/test_integration_live_write.py
    → both v4 lifecycle tests included.
  env -u YANDEX_DIRECT_LIVE_WRITE pytest <…lifecycle…>
    → SKIPPED via file-level skipif.

Codex review finding #1 (bare `pytest` hits live API when profile is
active) is intentionally deferred: project decision is that v4 live
read tests should run whenever a profile is available.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(v4_live): record VCR cassettes for v4wordstat/v4forecast lifecycle

Adds @pytest.mark.vcr to the two new v4 lifecycle tests and commits their
recorded cassettes so `pytest -m integration_live_write` no longer hits the
production API on every run, matching the contract documented in README
and pyproject.toml.

Also extends `_before_record_request` in tests/conftest.py to redact the
OAuth token from the request body: the v4 JSON API embeds `"token":"..."`
inside the payload (not just the Authorization header), so the existing
header-only filter would have leaked it into the recorded YAML.

Addresses Codex adversarial review feedback on #177.

* test(v4_live): move v4 report lifecycle to opt-in tier, add orphan store

Codex's second adversarial review flagged two real issues confirmed by
live verification:

1. tests/test_v4_live_contracts.py:_credentials() falls back to the
   active `direct auth` profile when env vars are missing. This is
   intentional but was never documented, and contradicts CLI's own
   priority chain (where the profile wins over env). Document the
   inverted, tests-only contract in _credentials() docstring, CLAUDE.md
   and README.md (EN+RU). No code change — behaviour is already correct
   (env wins; profile is fallback; otherwise skip).

2. test_live_draft_v4{wordstat,forecast}_lifecycle in
   tests/test_integration_live_write.py created account-level v4 reports
   under a marker that promises "only disposable draft resources". Move
   the two tests to tests/test_v4_live_contracts.py under the existing
   `_opt_in_write` pattern, gated by YANDEX_DIRECT_V4_LIVE_REPORT_WRITE=1.
   Delete the two corresponding cassettes (no replay for this tier —
   opt-in live only). Rename the remaining 18 v5 tests and cassettes
   from `test_live_draft_*` to `test_v5_live_draft_*` so v5 vs v4 is
   immediately visible at the test name level.

Add tests/_orphan_store.py: a small atomic JSON store at
~/.direct-cli/test-orphans.json that records created v4 report IDs.
Each opt-in test calls drain() on entry to retry deletions left over
from an interrupted previous run, add() right after create, and
remove() after a successful delete. If delete fails (network drop,
SIGKILL), the ID stays in the store and gets cleaned up next time.
Atomic write pattern modeled on direct_cli/auth.py:144-155.

Verified:
- pytest -m integration_live_write replay: 10 passed + 7 skipped (the
  same 18-test surface, minus the 2 removed v4 tests; cassettes resolve
  via new v5_ names).
- pytest tests/test_v4_live_contracts.py without env: both new tests
  skip as expected.
- orphan store: simulated network failure scenario — ID persists after
  failed delete, gets picked up by drain() on next run, store empties.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(v5): rename test_integration_live_write to test_v5_live_write

Align the file name with the test names. After the previous commit
renamed the 18 functions to `test_v5_live_draft_*`, the file itself
still said `test_integration_live_write.py` — function prefix and
file name now point at different things ("v5" vs. nothing). Renaming
makes the file/test/cassette directory all say v5.

The pytest marker `integration_live_write` is kept untouched — it
describes the run contract (`-m integration_live_write`), not the
file name, and renaming it would break the CI/dev invocation surface.

git mv:
- tests/test_integration_live_write.py → tests/test_v5_live_write.py
- tests/cassettes/test_integration_live_write/ →
  tests/cassettes/test_v5_live_write/
  (pytest-recording derives the cassette directory from the module
  name, so the directory has to track the file).

Doc references updated:
- tests/API_ISSUE_AUDIT.md
- tests/API_COVERAGE.md

Verified: 18 tests collect under the new path, replay returns the same
10 passed + 7 skipped as before, no leftover refs to the old name in
the repo (only in `direct_cli.egg-info/SOURCES.txt`, which is a build
artifact and not committed).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test: broaden exception handlers in test bootstrap and orphan store

Address two real defects flagged in PR #177 review on commit a200daa.

1. tests/test_v4_live_contracts.py::_credentials() previously caught
   only (ValueError, RuntimeError, ImportError) from
   direct_cli.auth.get_credentials(None, None). Tracing the call path
   showed two reachable leak points after a successful OAuth refresh:
   - OSError from save_auth_store → _write_json (os.chmod + mkstemp on
     ~/.direct-cli/ — unconditional, no swallow);
   - json.JSONDecodeError from parsing a malformed refresh-token
     response body (auth.py:534).
   Either would turn a "skip when creds unavailable" bootstrap into a
   hard test error on a dev machine with a stale profile but no env
   vars. Widen the except to include OSError and json.JSONDecodeError,
   and add a comment so the next reader sees why these two specifically.
   The other failure modes Copilot mentioned (HTTPError/URLError,
   KeyError/TypeError) are already trapped inside auth.py and cannot
   escape — confirmed by reading load_auth_store, get_oauth_profile,
   refresh_access_token, and validate_oauth_profile.

2. tests/_orphan_store.py::_read() had its list comprehension
   `[int(x) for x in v if isinstance(x, (int, str))]` outside the
   try/except, so a corrupted bucket like {"v4wordstat": ["abc"]}
   would raise ValueError up through add/remove/drain — contradicting
   the module docstring promise that "all read/write errors are
   swallowed silently so tests are never broken by store corruption."
   Move the comprehension inside the try and extend except with
   (ValueError, TypeError). Also drop redundant FileNotFoundError
   (subclass of OSError). Verified with hand-corrupted JSON: "abc"
   yields {}, [null, 456] yields {"v4wordstat": [456]}, plain garbage
   yields {}.

Codex's P2 (skip v4 tests entirely without explicit env credentials)
is left unaddressed: the env > profile > skip contract is the
documented and intended behavior — see CLAUDE.md, README.md (EN+RU),
and the _credentials() docstring. Reply posted on the thread.

Copilot's other comments triaged:
- "PR description doesn't reflect actual scope": process feedback, not
  a code issue.
- "Weak BannerID assert": intentional per the file's "confirm at first
  live run" pattern (matches sibling v4 read-only tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: axisrow <[email protected]>
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
axisrow added a commit that referenced this pull request May 20, 2026
Addresses PR #208 review:

* claude[bot] H12 over-constraint: StrategyAverageCpaBase, StrategyPayForConversionBase,
  StrategyAverageCrrBase, StrategyPayForConversionCrrBase, StrategyAverageCpaPerCampaignBase
  all carry GoalId minOccurs=0, and every Strategy*UpdateItem is an empty extension of its
  *Base. Per strict WSDL parity the update command MUST accept payloads without --goal-id,
  so the issue #198 H12 validation block is removed (with a comment explaining why the
  add-time minOccurs=1 check still stands).
* copilot #1: PayForConversionCrrAddItem schema is Crr + GoalId (no Cpa). Moved
  PayForConversionCrr out of PAY_FOR_CONVERSION_STRATEGY_TYPES into CRR_STRATEGY_TYPES so
  --average-crr now maps to Crr, not the WSDL-unknown Cpa.
* copilot #2: StrategyPayForConversionMultipleGoalsAddItem.GoalId is minOccurs=1, so the
  type is added to GOAL_ID_STRATEGY_TYPES; the add command now rejects payloads without
  --goal-id for that subtype. AverageCpaMultipleGoals stays out — its AddItem has no
  GoalId field at all.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
axisrow added a commit that referenced this pull request May 20, 2026
* fix(cli): close 12 High bugs from #198 audit + drop xfail markers

Per the systemic-audit issue #198, the WSDL parity gate landed in PR
#205 marked 8 commands as xfail-strict pending their fixes. This PR
applies the fixes one by one and drops every xfail / KNOWN_*_BUGS
exemption, so the gate now passes outright (37 passed, 8 skipped).

Patterns and fixes:

* H1, H5, H8, H9, H10 — empty-payload no-op guard:
  - ``ads update``, ``adgroups update``, ``keywords update``: if the
    built payload has only ``Id``, raise UsageError listing the
    available update fields.
  - ``bids set``, ``keywordbids set``: require at least one bid value
    before constructing the request.

* H2 — silent data loss on ``ads update``:
  - Per-type allow-list (TEXT_AD / TEXT_IMAGE_AD / MOBILE_APP_AD)
    rejects incompatible flags up front (e.g. ``--image-hash`` with
    ``--type TEXT_AD``) instead of silently dropping them.

* H4 — ``adgroups add --region-ids`` is now ``required=True``
  (WSDL ``AdGroupAddItem.RegionIds`` minOccurs=1).

* H6 — ``campaigns add --type SMART_CAMPAIGN --counter-id`` is now
  required (WSDL ``SmartCampaignAddItem.CounterId`` minOccurs=1).

* H7 — ``dynamicads add`` no longer requires ``--condition``; the WSDL
  field is minOccurs=0 and the CLI was over-constraining.

* H11 — ``STRATEGY_TYPES`` in ``direct_cli/commands/strategies.py`` is
  rebuilt from ``StrategyAddItem``: dropped 5 names that were not in
  the WSDL (WbMaximumClicksPerBid, WbMaximumConversionRatePerBid,
  AverageCrrPerCampaign, MaxProfitPerFilter, MaxProfitPerCampaign),
  added 5 that were missing (AverageCpcPerCampaign, AverageCpcPerFilter,
  PayForConversionCrr, AverageCpaMultipleGoals,
  PayForConversionMultipleGoals). Membership sets (CPA / CRR / PFC /
  multi-goal) updated accordingly.

* H12 — ``strategies update`` now validates ``--goal-id`` for the
  goal-id strategy family, mirroring ``strategies add``.

* H13 — verified false-positive. WSDL ``ClientUpdateItem`` does NOT
  declare ``ClientId`` minOccurs=1; the login goes through the
  Client-Login header, and ``clients update`` already enforces
  "Provide at least one field to update". No code change here.

The parity gate (``tests/test_wsdl_parity_gate.py``) drops every
``xfail`` marker and the ``KNOWN_REQUIRED_FIELD_BUGS`` exemption set,
so any regression of these patterns will now fail CI on the spot.

Three pre-existing tests updated to match the new validation surface:
- ``test_adgroups_add_case_insensitive_default_type`` now passes
  ``--region-ids`` (now required).
- ``test_campaigns_add_smart_requires_filter_average_cpc`` now passes
  ``--counter-id`` (now required, validated before
  ``--filter-average-cpc``).
- ``test_dynamicads_add_requires_condition`` rewritten as
  ``test_dynamicads_add_without_condition_omits_conditions_field``
  to reflect the new minOccurs=0 behaviour.

Refs #198. Closes PR 2 of milestone 0.3.9.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(strategies): align with WSDL on update GoalId and Crr-family

Addresses PR #208 review:

* claude[bot] H12 over-constraint: StrategyAverageCpaBase, StrategyPayForConversionBase,
  StrategyAverageCrrBase, StrategyPayForConversionCrrBase, StrategyAverageCpaPerCampaignBase
  all carry GoalId minOccurs=0, and every Strategy*UpdateItem is an empty extension of its
  *Base. Per strict WSDL parity the update command MUST accept payloads without --goal-id,
  so the issue #198 H12 validation block is removed (with a comment explaining why the
  add-time minOccurs=1 check still stands).
* copilot #1: PayForConversionCrrAddItem schema is Crr + GoalId (no Cpa). Moved
  PayForConversionCrr out of PAY_FOR_CONVERSION_STRATEGY_TYPES into CRR_STRATEGY_TYPES so
  --average-crr now maps to Crr, not the WSDL-unknown Cpa.
* copilot #2: StrategyPayForConversionMultipleGoalsAddItem.GoalId is minOccurs=1, so the
  type is added to GOAL_ID_STRATEGY_TYPES; the add command now rejects payloads without
  --goal-id for that subtype. AverageCpaMultipleGoals stays out — its AddItem has no
  GoalId field at all.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(strategies): route --average-cpc to FilterAverageCpc for AverageCpcPerFilter

Addresses PR #208 re-review:

* claude[bot] critical: StrategyAverageCpcPerFilterAddItem has no AverageCpc
  field — its WSDL schema is FilterAverageCpc + WeeklySpendLimit + ... . The
  CLI used to emit {AverageCpc: N} unconditionally, producing a WSDL-invalid
  payload for `--type AverageCpcPerFilter`. Added a FILTER_CPC_STRATEGY_TYPES
  branch mirroring the existing FILTER_CPA_STRATEGY_TYPES handling so
  `--average-cpc` maps to `FilterAverageCpc` for that subtype only.
* claude[bot] observation: AverageCpaMultipleGoals and PayForConversion-
  MultipleGoals AddItems have no Cpa/AverageCpa field at all. Now
  `_build_strategy_fields` raises `UsageError` if `--average-cpa` is passed
  with a multi-goal subtype, instead of silently emitting an unknown key.

Tests:

* test_strategies_add_average_cpc_per_filter_payload_uses_filter_field locks
  the new dispatch.
* test_strategies_add_pay_for_conversion_crr_payload_uses_crr_field confirms
  the earlier commit's CRR-family classification still routes through `Crr`.
* test_strategies_add_multi_goal_rejects_average_cpa exercises the
  multi-goal guard.
* test_strategies_add_pay_for_conversion_multi_goal_requires_goal_id
  exercises the GOAL_ID_STRATEGY_TYPES extension from the prior commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(strategies): reject empty-subtype no-op on update

PR #208 re-review found one more silent no-op of the same class this PR
closes for ads/adgroups/bids/keywordbids/keywords:

```
direct strategies update --id 42 --type AverageCpa
```

…built `{Id: 42, AverageCpa: {}}` and the live API accepted it as a 200 OK
no-op. A user mistyping a flag name got a successful response with no diff
applied.

Two guards now reject this:

* if `--type X` is present but `_build_strategy_fields(...)` returned `{}`,
  the command errors with "strategies update requires at least one field
  for --type X.";
* if after all flag merging `strategy_data` still contains only `Id` (no
  --name, no --counter-ids, no --priority-goal, no --attribution-model),
  the command errors with "strategies update requires at least one
  updatable field."

Also locked the regression down in the WSDL parity gate: added a
`strategies.update` probe to `EMPTY_PAYLOAD_PROBES` alongside the existing
five so future refactors cannot silently re-open the no-op.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: axisrow <[email protected]>
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
axisrow added a commit that referenced this pull request May 30, 2026
* 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]>
axisrow added a commit that referenced this pull request May 30, 2026
…ns (#488)

* fix(vendor): recover from tapi_yandex_direct 2026.5.29 bump regressions

The 2026.5.29 vendor bump (1ee8c2a) rebuilt direct_cli/_vendor via
update_vendor.sh's rm -rf + cp -R of the fork, wiping local patches and
desyncing the vendor from the tests. That introduced four regressions; CI
surfaced only the first because Quality fails on mypy before the test step:

1. timeout stub (mypy): the .pyi for YandexDirectClientExecutor.get/post
   dropped the `timeout` kwarg auth.py:631 passes, so mypy rejected the call
   even though tapi2 forwards it to requests at runtime.
2. to_columns IndexError: upstream reverted the short-row guard; report rows
   with fewer cells than the header crashed.
3. transport hosts: upstream turned DIRECT_API_PRODUCTION_ROOT into a {tld}
   template (v5 -> .com, v4 -> .ru); test_transport_contract compared against
   the un-substituted constant.
4. VCR cassettes: the v5 host moved .ru -> .com but all cassettes are .ru, so
   the host matcher failed and 42 replay tests broke under --record-mode=none.

Fixes:
- Re-apply #1 and #2 and make them self-healing in patch_vendor_imports.py
  (_patch_stub_text for the .pyi, _patch_runtime_text for to_columns) so the
  next vendor bump cannot silently drop them again; both are idempotent.
- Add two gates in test_vendor_imports.py (ast check on the shipped stub +
  idempotency of _patch_stub_text).
- Update test_transport_contract.py to the {tld} contract.
- Register a TLD-insensitive VCR host matcher in conftest.py so .ru cassettes
  replay against .com requests and vice versa.

Verified: mypy clean, ruff clean, pytest -m "not integration" all green
(2035 passed), self-heal restores both patches on a stripped tmp copy and is a
no-op on re-run.

Closes #487

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* refactor(vendor): boundary-aware class scope in _patch_stub_text

Review follow-up (#488): _patch_stub_text scoped the stub class with
line.startswith("class YandexDirectClientExecutor"), which also matches the
prefix-sharing class YandexDirectClientExecutorResponse. Harmless today (that
Response body has no get/post or :param data:), but a latent fragility if
upstream ever reorders classes. Replace the prefix check with a boundary-aware
regex ^class YandexDirectClientExecutor[:\(] so only the exact class is treated
as in-scope, and add a gate test feeding a prefix-sharing decoy class with the
exact executor signature (would fail on the old startswith check).

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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant