Skip to content

feat(masters): add direct masters archive (#633) - #645

Merged
axisrow merged 2 commits into
mainfrom
ao/issue-633-masters-delete
Aug 1, 2026
Merged

feat(masters): add direct masters archive (#633)#645
axisrow merged 2 commits into
mainfrom
ao/issue-633-masters-delete

Conversation

@axisrow

@axisrow axisrow commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds direct masters archive <ids> — the closest thing to "delete" Мастер кампаний has.
  • Live recon (documented in the issue #633 comment) confirmed via claude-in-chrome against a real account that Yandex's UI has no separate delete action for Мастер кампаний — neither the campaigns grid's row menu (Перейти/Редактировать/Статистика/Запустить-Остановить/Архивировать) nor the overview page's own "⋮" menu (Клонировать/Архивировать) has a "Удалить" item.
  • archive_master clicks the overview page's "⋮" menu then the "Архивировать" item, using confirmed-live data-testid selectors (CampaignHeader.MenuTrigger / CampaignHeader.Menu.archive) rather than text-matched candidates like suspend/resume use — these were read directly off the live DOM, not guessed.
  • Verifies success by re-reading the campaigns grid (fetch_masters_list) and confirming Status == "ARCHIVED", rather than trusting the click alone — mirrors the suspend/resume "click + verify" convention from direct masters suspend/resume — остановка и возобновление Мастера кампаний #630.
  • Idempotent (already-archived campaign → warning, no click).
  • Irreversible from this CLI — there is no masters unarchive. Classified DANGEROUS in smoke_matrix.py, same category as suspend/resume (no API ⇒ no --sandbox isolation).
  • README (EN+RU) and CHANGELOG updated.

Closes #633

Test plan

  • pytest -q — 2672 passed, 0 failed (62 pre-existing cassette-fixture errors unrelated to this change, confirmed present on main too)
  • pytest tests/test_masters.py -n0 — 101 passed (new TestArchiveMaster/TestMastersArchiveCommand classes: click+verify, idempotency, not-found, menu-trigger-missing, archive-item-missing, status-never-changes)
  • black --check / flake8 clean on all changed files
  • Live browser recon only (read-only menu inspection via claude-in-chrome) — no live mutation was exercised; masters archive itself is untested against a real account (no disposable test campaign yet, see direct masters add — создание Мастера кампаний #632 dependency noted in the original issue)

axisrow and others added 2 commits August 1, 2026 22:24
Live recon (issue #633) confirmed Мастер кампаний has no separate
"delete" action in Yandex's UI — neither the campaigns grid's row menu
nor the overview page's own menu has "Удалить", only "Архивировать".
archive_master clicks the overview page's "⋮" menu then the archive
item, using confirmed-live data-testid selectors
(CampaignHeader.MenuTrigger/CampaignHeader.Menu.archive) rather than
text-matched candidates, and verifies success via the campaigns grid's
primaryStatus before reporting the mutation as done. Idempotent, and
irreversible from this CLI (no masters unarchive) — classified
DANGEROUS in smoke_matrix.py like suspend/resume.

Closes #633

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QvEdRCGSbLnCgtkypCXd5X
Codex review of PR #645 found that `masters archive`'s per-ID loop was
a fail-fast list comprehension: if campaign N raised, `results` was
never assigned and format_output never ran, so already-archived (and
irreversible -- no `masters unarchive`) earlier IDs were silently
dropped from the report, leaving only the last error visible.

_archive_all now archives every ID in a loop, catching
BrowserSessionError/PlaywrightError per ID and recording either the
archived row or the error, then reports all outcomes before raising a
ClickException if any ID failed -- never losing the audit trail of
what was actually mutated in production.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QvEdRCGSbLnCgtkypCXd5X
@axisrow

axisrow commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

🔍 Local review (cycle 1)

Reviewed locally (/review + Codex companion), no bots pinged.

Verdict Reviewer Finding Location
FIX codex Bulk archive discards successful (irreversible) results after a later ID fails — silent partial mutation with no audit trail direct_cli/commands/masters.py:593
SKIP claude Verify loop re-navigates the whole campaigns grid on every 250ms poll instead of a lightweight status re-read direct_cli/browser/masters.py:591-597

FIX applied in a11d4ee: _archive_all now archives every ID in a loop, catching BrowserSessionError/PlaywrightError per ID and recording either the archived row or the error, then reports every outcome (not just the last error) before raising if any ID failed. New regression test test_archive_reports_earlier_successes_when_a_later_id_fails confirms all 4 IDs are attempted and the earlier successes remain visible in the output even when a later ID errors.

SKIP rationale: confirmed this is a documented architectural constraint, not a bug — there is no confirmed status-text marker for "archived" on the overview page (unlike suspend/resume's "Кампания остановлена"/"активна"), so fetch_masters_list (the grid API) is the only source of truth for verification, same as the initial post-click check. The error message already warns to verify manually if the budget is exceeded.

@axisrow

axisrow commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

📋 Review summary — all cycles

Cycle Reviewer Finding Verdict Resolution
1 codex Bulk archive discards successful (irreversible) results after a later ID fails — silent partial mutation with no audit trail FIX Fixed in a11d4ee — per-ID try/except loop reports every outcome, exits non-zero only after reporting all results
1 claude (/review) Verify loop re-navigates the whole campaigns grid on every 250ms poll instead of a lightweight status re-read SKIP Documented architectural constraint — no confirmed status-text marker for "archived" on the overview page, so the grid API is the only source of truth
2 codex (none — approve) Re-verified the round-1 fix is correct; no new findings
2 claude (/review) (none) No new issues found on re-review

Totals: 1 FIX (resolved), 1 SKIP (architectural), 0 UNVERIFIED.

Local review cycle complete (2 rounds: /review + Codex companion, no bots pinged). Full test suite green (2673 passed, 0 failed; 62 pre-existing cassette-fixture errors unrelated to this PR, confirmed present on main), black/flake8 clean. Merge is yours to trigger — local mode does not auto-merge.

@axisrow
axisrow merged commit b89bc7d into main Aug 1, 2026
6 checks passed
@axisrow
axisrow deleted the ao/issue-633-masters-delete branch August 1, 2026 15:48
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.

direct masters archive — архивация Мастера кампаний

1 participant