Skip to content

MAINT Separate server launch, readiness, and cleanup state (#2415) - #2445

Merged
Roman Lutz (romanlutz) merged 5 commits into
microsoft:mainfrom
zhenyu-02:codex/pyrit-2415
Aug 25, 2026
Merged

MAINT Separate server launch, readiness, and cleanup state (#2415)#2445
Roman Lutz (romanlutz) merged 5 commits into
microsoft:mainfrom
zhenyu-02:codex/pyrit-2415

Conversation

@zhenyu-02

@zhenyu-02 Zhenyu Wang (zhenyu-02) commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Description

Closes #2415.

This refactors ServerLauncher.start_async into three internal seams while preserving it as the public orchestrator:

  • a pure launch plan for validation, command construction, and Windows/Unix process settings;
  • explicit startup state for process ownership, PID persistence, readiness, timeout, diagnostics, and cleanup results;
  • one idempotent, cancellation-safe cleanup path shared by launch failure, readiness failure, timeout, cancellation, and stop().

The process-group, detached-launch, PID-record, log, and public return/exception behavior remain unchanged. Focused tests cover platform-specific planning, invalid timeouts, cancellation during spawn/PID persistence/health polling/cleanup, repeated cancellation, readiness success, crash diagnostics, timeout cleanup, cleanup failure retention, and startup reporting failures when the ready message cannot be printed.

The branch is based on current main (f9bcd1dd) and retains the merged #2457 dataset-preload timeout diagnostics: the timeout path reads the log tail once, reuses that tail for stderr output and the preload hint, and preserves the existing cleanup behavior.

No public API or documentation behavior changed.

Automated validation

  • uv run pytest tests/unit/cli/test_server_launcher.py -q — 70 passed, 1 skipped
  • uv run pytest tests/unit/cli -q — 454 passed, 1 skipped
  • uv run ruff check pyrit/cli/_server_launcher.py tests/unit/cli/test_server_launcher.py — passed
  • uv run ruff format --check pyrit/cli/_server_launcher.py tests/unit/cli/test_server_launcher.py — 2 files already formatted
  • uv run ty check pyrit/cli/_server_launcher.py tests/unit/cli/test_server_launcher.py — passed
  • uv run python -m compileall -q pyrit/cli/_server_launcher.py tests/unit/cli/test_server_launcher.py — passed
  • git diff --check — passed

These checks are local and CPU-only; no GPU or external service is required.

@zhenyu-02

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@romanlutz

Copy link
Copy Markdown
Contributor

Is this meant to be a draft PR or ready for review? Zhenyu Wang (@zhenyu-02)

@zhenyu-02 Zhenyu Wang (zhenyu-02) changed the title [DRAFT] MAINT Separate server launch, readiness, and cleanup state (#2415) MAINT Separate server launch, readiness, and cleanup state (#2415) Aug 24, 2026
@zhenyu-02
Zhenyu Wang (zhenyu-02) marked this pull request as ready for review August 24, 2026 07:33
@zhenyu-02

Copy link
Copy Markdown
Contributor Author

Roman Lutz (@romanlutz) I rebased this PR onto current main (7307d942) and integrated the merged #2457 change without dropping its dataset-preload timeout diagnostics. The timeout path reads the log tail once and reuses it for both diagnostic output and the preload hint.

Automated checks on the updated head (96d91dd3) pass:

  • uv run pytest tests/unit/cli/test_server_launcher.py -q — 68 passed, 1 skipped
  • uv run pytest tests/unit/cli -q — 432 passed, 1 skipped
  • ruff check and format check
  • ty check
  • compileall and git diff --check

The PR is now marked ready for review. Please take another look when convenient.

Comment thread pyrit/cli/_server_launcher.py Outdated
Comment thread pyrit/cli/_server_launcher.py Outdated
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Aug 25, 2026
Merged via the queue into microsoft:main with commit b8c8791 Aug 25, 2026
54 checks passed
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.

MAINT Separate server launch, readiness, and cleanup state

2 participants