Skip to content

LaunchUpdateDialogAutoConfirmer: arm сообщает, взвёл ли он что-то, а сборка снимает только своё взведение (#618) - #704

Merged
DitriXNew merged 1 commit into
masterfrom
fix/618-autoconfirmer-arm-result
Oct 3, 2026
Merged

DitriXNew merged 1 commit into
masterfrom
fix/618-autoconfirmer-arm-result

Conversation

@Jimmo910

@Jimmo910 Jimmo910 commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Closes #618.

Что было

У LaunchUpdateDialogAutoConfirmer четыре void-перегрузки arm(...) вызывали boolean-реализацию и выбрасывали результат. А реализация возвращает false, ничего не взводя, когда workbench Display ещё нет. Единственный вызывающий void-варианта, BuildExternalObjectsTool, снимал взведение в finally безусловно. disarm зажимает счётчик по нулю, но не знает, чей счёт снимает: после взведения-пустышки он мог забрать счёт параллельного launch, и модальные окна того launch остались бы ждать человека. Подробности и границы окна — в ишью; блокер «не берём до #631» снят мержем 503a8161.

Что изменилось

  • Четыре бывших void arm(...) возвращают результат взведения (boolean); в javadoc — @return и правило «парный disarm только при true». Перегрузки не удалены, все существующие вызовы компилируются как раньше.
  • BuildExternalObjectsTool работает по тому же образцу autoConfirmerArmed, что уже применяют StandaloneServerSupport, LaunchTool, RunYaxunitTestsTool, UpdateDatabaseTool и ExportConfigurationToFileTool: arm стоит непосредственно перед try, снятие — только если взведение состоялось. Постановка задачи перенесена внутрь try, поэтому сбой планирования тоже снимает взведение (раньше он оставлял его до конца сессии). Порядок при прерывании прежний: сначала флаг и ответ, затем снятие.
  • Исправлен javadoc, который называл неверные перегрузки для update_database и путей запуска (они идут через семиаргументную перегрузку, трёхаргументную вне класса зовёт только build_external_objects).
  • Провод не меняется: описания, inputSchema, гайды, golden и MANIFEST не тронуты.

Граница — объявляю явно

  • Без Display нельзя закрепить тестом, что runBuild передаёт в шов именно armLaunchDialogs/disarmLaunchDialogs: в безголовой среде взведение всегда пустышка, и снятие не вызывается. Это видно по коду (шов scheduleAndJoinBuild → scheduleAndJoinArmed), не тестом.
  • Саму гонку со стартом плагина правка не устраняет. Она гарантирует ровно то, что требует ишью: вызов, который ничего не взвёл, не снимает чужой счёт.

Доказательства

  • Юнит, LaunchUpdateDialogAutoConfirmerTest: каждая из четырёх перегрузок в безголовой среде отвечает false; поведенческий пин дефекта — взведение-пустышка, затем параллельный launch поднимает счёт, затем условное снятие оставляет чужой счёт нетронутым. Для этого добавлены пакетно-приватные тестовые швы и помощник AutoConfirmerArmCounts в тестовом бандле.
  • Юнит, BuildExternalObjectsToolTest: пустышка не снимается; состоявшееся взведение снимается ровно один раз после ожидания; сбой планирования всё равно снимает; прерванное ожидание сначала отвечает, потом снимает; продуктовый armLaunchDialogs отдаёт результат конфирмера; снятие возвращает ровно те матчеры, что взял arm.
  • На master новые тесты не компилируются (типы void), поэтому поведенческую красноту доказывают мутации — 14: безусловный disarm; результат взведения проигнорирован; перегрузки по одной теряют результат делегата; планирование снова до try; планирование до взведения; снятие пропало; флаг прерывания потерян; и т.д. — каждая роняет свой пин. Восстановление по sha256, target/ тестового бандла чистится перед каждым прогоном.
  • Сборка: BUILD SUCCESS, 8760 юнит-тестов, 0 падений.
  • Ревью до push: два независимых гейта (корректность; честность тестов) — SHIP и 5 замечаний P3, все вшиты. PR-бот codex сейчас без квоты; ревью запрошу, как только она вернётся.

🤖 Generated with Claude Code

https://claude.ai/code/session_0174zH13nscxbTgBU4YfXcXK

…disarms only what it armed (#618)

The four void arm(...) overloads dropped the result of the boolean
implementation, which returns false without arming anything when no
workbench Display exists yet. Their only caller, BuildExternalObjectsTool,
then disarmed unconditionally in finally; disarm clamps at zero but cannot
tell whose count it takes, so a no-op arm could release the count a
concurrent launch had armed and leave that launch's modal dialogs waiting
for a human.

The overloads now return the arming result, and the build follows the
autoConfirmerArmed pattern every other caller already uses: arm right
before the try, disarm in finally only when the arm took effect. Job
scheduling moved inside the try, so a scheduling failure releases the arm
too. Javadoc that named the wrong overloads for update_database and the
launch paths is corrected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0174zH13nscxbTgBU4YfXcXK
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Test Results

8 760 tests  +12   8 755 ✅ +12   6m 19s ⏱️ +6s
  351 suites ± 0       5 💤 ± 0 
  351 files   ± 0       0 ❌ ± 0 

Results for commit d493d6b. ± Comparison against base commit d62c416.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

E2E Test Results (EDT 2026.2)

    4 files  ±0      4 suites  ±0   1h 9m 27s ⏱️ - 3m 2s
1 381 tests ±0  1 339 ✅  - 2  42 💤 +2  0 ❌ ±0 
1 384 runs  ±0  1 342 ✅  - 2  42 💤 +2  0 ❌ ±0 

Results for commit d493d6b. ± Comparison against base commit d62c416.

This pull request skips 2 tests.
create_git_branch::test_branch_already_exists_errors_without_creating_anything
switch_git_branch::test_switching_to_the_current_branch_is_rejected

@DitriXNew

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-03T20:24:49.095465Z d493d6b Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: d493d6b1bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@DitriXNew
DitriXNew marked this pull request as ready for review October 3, 2026 20:22
@DitriXNew
DitriXNew merged commit 048cdd1 into master Oct 3, 2026
16 checks passed
@Jimmo910
Jimmo910 deleted the fix/618-autoconfirmer-arm-result branch October 3, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants