fix(v4)!: restore --currency for live finance methods (regression from #441/#442/#443)#450
Conversation
…#441/#442/#443) PRs #441/#442/#443 dropped the obligatory Live 4 input parameter Currency from PayCampElement on transfer-money, pay-campaigns, and create-invoice; #442 additionally dropped the valid Overdraft PayMethod. The commits cited dg-v4/reference/<Method> as the verification source. That URL is legacy v4 (pre-Live), where PayCampElement is {CampaignID, Sum} only. The CLI ships these commands in the v4finance Live group and must mirror dg-v4/live/<Method>. Verified 2026-05-29 by curling the live pages: - dg-v4/live/TransferMoney → PayCampElement = {CampaignID, Sum, Currency}; changelog "Новое в версии Live 4: Входной параметр Currency стал обязательным". - dg-v4/live/PayCampaigns → PayCampElement = {CampaignID, Sum, Currency}; same changelog plus "Добавлен метод оплаты Overdraft". - dg-v4/live/CreateInvoice → PayCampElement = {CampaignID, Sum, Currency}. Restored: - --currency (required, Choice: RUB/CHF/EUR/KZT/TRY/UAH/USD/BYN) on transfer-money, pay-campaigns, create-invoice. - Currency emitted on every Payments[] / FromCampaigns[] / ToCampaigns[] item. - Overdraft re-enabled in V4_PAY_METHODS; --contract-id remains required only when --pay-method Bank. Updated v4_contracts.py example_param and notes (now pointing at dg-v4/live/*); updated tests/test_v4finance_money.py and tests/test_v4_contracts.py expectations; updated README, CHANGELOG (0.3.16 BREAKING CHANGES), scripts/test_dangerous_commands.sh checklist, tests/API_COVERAGE.md, and the opt-in live-write test for CreateInvoice. Bumped version to 0.3.16. Audit trail: #125 (comment) Co-Authored-By: Claude Opus 4.7 <[email protected]>
|
@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 finished @axisrow's task in 1m 16s —— View job Review: PR #450 — restore
|
|
Триаж ревью: блокирующих ( Два non-blocking замечания ревьюера (оба SKIP, изменений не требуют):
|
* chore(audit): add docs/wire-shape scanner + 2026-05-29 sweep (refs #111) New offline-rerunnable tool that parses Yandex Direct docs HTML, extracts the request JSON-schema, and compares with V4_METHOD_CONTRACTS.example_param and RESOURCE_MAPPING_V5. Closes the methodological gap that allowed the v4 Currency regression (root-caused in #125, fixed by #450) — that PR happened because verification was done against dg-v4/reference/ pages instead of dg-v4/live/ ones. Tooling ------- - scripts/audit_wire_shape.py — modes: --v4, --v5, --reports, --all. Uses a 3-retry × 5-second-pause captcha guard reusing the same markers as direct_cli.reports_coverage.fetch_reports_spec. Persistent captcha is recorded as `captcha-after-retries` rather than aborting the run. - tests/test_audit_wire_shape.py — 9 offline golden tests covering request-section extraction, schema parsing, response-side leak prevention, Live 4 hint detection, example_param key walk, captcha marker detection, and v5 per-method URL derivation. Sweep artefacts (2026-05-29) ---------------------------- - docs/audits/PROJECT_WIRE_SHAPE_AUDIT_2026-05-29.md — 174 findings across 32 v4 methods, ~112 v5 operations, 1 reports surface. - docs/audits/wire_shape.json — same data, machine-readable. Reproducibility --------------- python scripts/audit_wire_shape.py --all \ --json docs/audits/wire_shape.json \ --markdown docs/audits/PROJECT_WIRE_SHAPE_AUDIT_2026-05-29.md Summary writeup with severity-classified findings posted as a comment to issue #111 (link in the comment body). This PR adds the auditing capability and the dated report only. Individual contract / payload-builder fixes for the HIGH-severity gaps (GetEventsLog contract missing 11 fields, CreateNewForecast missing 3+ fields, GetBannersTags missing CampaignIDS) are deliberately deferred to follow-up PRs — one per gap, per the project's "1 issue = 1 PR" rule. Audit trail: #111 (comment) Co-Authored-By: Claude Opus 4.7 <[email protected]> * fix(audit): preserve resource name in v5 per-method URLs + dedup spec URL Review fixes for #451: - _v5_per_method_url chopped the trailing path segment unconditionally, so single-segment docs bases (vcards, smartadtargets, dynamictextadtargets, dynamicfeedadtargets) lost their resource name and resolved to `…/ru/get` — silently mis-auditing 4 v5 groups at 404/captcha URLs. Now the tail is chopped only when it duplicates its parent segment; single-segment bases append the method to the full base. - The test that locked in the buggy `…/ru/get` output is corrected and a vcards regression-guard case is added. - REPORTS_SPEC_URL now imports REPORTS_SPEC_URLS["spec"] from the registry instead of duplicating the literal, per CLAUDE.md "No URL literals outside the registry". Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix(audit): gate notes-points-at-reference on a verified live page The FINDING_NOTES_REFERENCE_URL check fired whenever the contract notes cited dg-v4/reference/ and url_used stayed live_url — but a captcha'd live page also leaves url_used == live_url, so the finding asserted "live exists, drop reference" on a page that was never actually read (15/16 v4 methods were captcha-blocked in the 2026-05-29 run). Require fetched.status == "ok" so the finding only fires when the live schema was genuinely verified. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: hapi <[email protected]> Co-authored-by: Claude Opus 4.7 <[email protected]> Co-authored-by: axisrow <[email protected]>
Summary
--currency(required) andCurrencywire-field onv4finance transfer-money/pay-campaigns/create-invoiceafter they were dropped in PRs feat(v4)!: drop --currency from transfer-money to match docs 1:1 (closes #432) #441/fix(v4): align pay-campaigns wire-body with docs; drop Currency (closes #433) #442/fix(v4): align create-invoice wire-body with docs; drop Currency (closes #434) #443 as «docs 1:1». The docs source those PRs cited wasdg-v4/reference/<Method>(legacy v4 pre-Live), while the CLI lives in thev4financeLive group and must mirrordg-v4/live/<Method>— whereCurrencyis explicitly marked obligatory in the Live 4 changelog.Overdraftas a valid--pay-methodforpay-campaigns(Live 4 changelog adds it for direct advertisers;--contract-idstays required only forBank).v4_contracts.pyexample_paramandnotesto point at the live URLs and the 2026-05-29 verification.0.3.16with aBREAKING CHANGESsection in CHANGELOG that explicitly frames the change as a revert.Root cause (verified, reproducible)
Live pages for all three methods (TransferMoney / PayCampaigns / CreateInvoice) contain:
Currency: (string)insidePayCampElement.Currency → required: Да.'Currency': 'RUB'.Reference URLs lack all four — they describe v4 before Live 4.
Full root-cause diff and per-URL comparison table is in the issue audit comment: #125 (comment)
Test plan
pytest tests/— 1908 passed, 84 skipped (existing skips: opt-in live writes).direct v4finance transfer-money --from-campaign-id 1 --to-campaign-id 2 --amount 100.50 --currency RUB --finance-token X --operation-num 1 --dry-run— JSON containsCurrency: "RUB"in both items.direct v4finance pay-campaigns --pay-method Overdraft ...accepted (without--contract-id).direct v4finance create-invoice --payment 1=10 --currency RUB ...emits Currency in eachPayments[]item.YANDEX_DIRECT_LIVE_FINANCE_WRITE=1once a sandbox campaign is available (test body already updated to includeCurrency: "RUB").Audit comment posted to #125
#125 (comment)