Skip to content

FEAT: Shared sending core - #2804

Merged
Roman Lutz (romanlutz) merged 6 commits into
microsoft:mainfrom
romanlutz:romanlutz-n-send-phase-2-core
Sep 25, 2026
Merged

Roman Lutz (romanlutz) merged 6 commits into
microsoft:mainfrom
romanlutz:romanlutz-n-send-phase-2-core

Conversation

@romanlutz

@romanlutz Roman Lutz (romanlutz) commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Description

Implements phase 2 only of the approved n-send split plan, building on the atomic conversation branching merged in #2740.

  • Move existing manual-message execution and its preparation, converter, media, and metadata helpers into MessageSendService. AttackService remains a thin synchronous response adapter, and PromptNormalizer retains conversion and request/response persistence responsibilities.
  • Share one process-wide scheduler across manual operations: at most 64 admitted operations and 4 executing operations, with FIFO execution. Targets retain their existing per-target request pacing. An optional normalizer guard protects shared converter instances only during actual conversion, without making the whole send exclusive or locking other converter instances.
  • Hold conversation ownership through attack-summary and conversation-response assembly, including send=false writes. Conflicting conversation use returns 409; full admission returns 429. Failures and cancellation release ownership, with offloaded writes joined before release.
  • Serialize the complete metadata read/merge/write per attack without serializing ordinary provider calls. This prevents concurrent conversations from overwriting newer response metadata or losing applied-converter history.
  • Use CentralMemory consistently across the facade, sender, and normalizer rather than permitting a sender-only memory override that could split persistence across stores.

POST /api/attacks/{id}/messages still waits for execution and returns the existing attack/conversation response shape. Store-only context roles, stored-error status, multipart ordering and lineage, preconverted-piece filtering, legacy converter inputs, response conversion, and target validation are preserved. Main-branch merges preserve converter-stage provenance, independent original/converted-media persistence, and the newer normalizer behavior.

No asynchronous submission/status API, batch/count/repetition support, background registry, frontend changes, schema migrations, or dependency changes are included in the PR diff.

Reviewability and line accounting: the behavior-preserving extraction is isolated in ce95635b1 (+2,015/-1,920 across five files), before scheduler behavior in f3041476a. 5788d18ab fixes metadata ordering and memory ownership. The latest update merges main at 005a8d836 in 5d6a89c, then addresses the two inline review comments in 8572f1c88 (+483/-124 across nine files).

The complete PR touches 12 files, +4,068/-2,491 against that main baseline. This is not a claim of a line-count reduction or 4,068 lines of new implementation.

Portion Unchanged relocated lines Edited lines within relocated functions Remaining added lines
Sending service 505 87 95
Sender tests, including 49 existing test methods 1,668 37 846
Shared test helpers 64 3 16
New scheduler 0 0 143
New scheduler tests 0 0 247
Facade, routes, normalizer hook, other tests, and documentation additions 0 0 357

Relocation counts compare paired function ranges against the merged-main baseline, normalizing indentation and test-owner names. Remaining lines include new coordination and regressions as well as imports, class headers, fixtures, adapter wiring, and edits to existing code; they are not all new production logic. Existing sending tests move with their owner instead of being duplicated in the facade suite.

Tests and Documentation

  • 662 focused tests passed, 4 skipped, covering the scheduler, sender, facade, API routes, converter service, complete prompt-normalizer test directory, and target pacing utilities.
  • New regressions pause rate-limit waits and response reads, verify progress for unrelated targets and converter instances, and check same-conversation ownership through both response-read boundaries. All eight relevant cases fail for the expected reasons when the exact pre-fix methods are restored only in memory.
  • Shared-converter protection is exercised for both request and response conversion, including skipped configurations and exception/cancellation cleanup. Default normalizer behavior and converter previews remain unchanged.
  • Existing real-SQLite regressions cover metadata ordering and converter-history merging; coverage also retains admission limits/fairness, store-only coordination, queued/active cancellation, threaded persistence cleanup, shared-memory ownership, stored target errors, and multipart/preconverted behavior.
  • Scheduler coverage is 100%; sender combined statement/branch coverage is 97%. Ruff checks, formatting, ty check pyrit, and commit hooks pass.
  • Updated the backend README with ownership through response assembly, synchronous behavior, process-local limits, 409/429 responses, target pacing, per-instance conversion guards, and metadata coordination. No real provider calls or unrelated frontend/documentation execution were required.

Move the existing preparation, converter, dispatch and metadata helpers with their tests. Keep AttackService as the synchronous response facade.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Share FIFO admission and execution limits across manual sends and store-only appends. Preserve normalizer persistence, join memory writes on cancellation, and merge current converter metadata under the exclusive send slot. Keep the existing synchronous API with explicit 409 and 429 admission responses.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Retain upstream converter provenance and media persistence behavior in the extracted sender, and relocate the matching tests with it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Serialize complete metadata updates per attack without serializing provider calls. Keep the metadata guard through cancellation cleanup and use CentralMemory consistently across the facade, sender, and normalizer. Add real-memory race and shared-memory regressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread pyrit/backend/services/message_send_service.py
Comment thread pyrit/backend/services/message_send_service.py
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use existing target-level pacing instead of whole-send exclusion. Guard shared converter instances only during actual conversion through an optional PromptNormalizer context. Keep conversation reservations through synchronous response assembly and cover concurrency, cancellation, and error cleanup with regression tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Sep 25, 2026
Merged via the queue into microsoft:main with commit 38af582 Sep 25, 2026
49 checks passed
@romanlutz
Roman Lutz (romanlutz) deleted the romanlutz-n-send-phase-2-core branch September 25, 2026 06:15
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.

3 participants