fix(prompt-eval): rate-limit aware backoff, circuit breaker and process-group termination

[NY1024]

Fixes #633

Problem

During prompt security evaluation, once the target model hits its QPM limit, request frequency surges far beyond the configured concurrency (80+ requests/min against a QPM=20 quota, even with concurrency set to 1), and requests keep being sent for a while after the task is stopped.

Root Causes

1. Blind short-backoff retry on rate-limit errors (AIG-PromptSecurity/cli/model_utils/openailike.py)

generate()/a_generate() retried any error (including 429) with a short fixed backoff (0.5s base, 3 tries), then returned "" silently. Each failing task finished within ~3.5s, letting the next test case fire immediately — a request storm that never gave the quota window a chance to recover. This also explains why fast-failing models (Minimax-M2.5, Deepseek-V4-Flash, Qwen3.6/3.8) triggered the storm while slower ones (Qwen3-30B-A3B-2507) mostly survived: the faster the failure cycle, the faster the request rate.

2. Unbounded concurrency in the attack-generation phase (deepteam/attacks/attack_simulator/attack_simulator.py)

In a_simulate(), the baseline stage spawned all vulnerabilities concurrently without the shared semaphore (only the enhance stage had it), so simulator traffic could burst beyond max_concurrent.

3. Orphaned Python process after task stop (common/utils/utils.go)

Stopping a task SIGKILLed only the direct child process (uv) via exec.CommandContext; the actual Python cli_run.py process survived as an orphan and kept sending requests until its queue drained.

Changes

File	Change
cli/model_utils/openailike.py	Rate-limit aware retry: detect 429/quota errors via message markers; honor Retry-After when present, otherwise back off 8s/16s/32s (cap 60s); non-rate errors keep original backoff; max_trial 3→5; track consecutive rate-limit failures per model
deepteam/red_teamer/red_teamer.py	Circuit breaker in _a_attack: skip remaining test cases once the target model accumulates 3 consecutive rate-limit failures (reset on success), instead of hammering a throttled model
deepteam/attacks/attack_simulator/attack_simulator.py	Wrap baseline simulation with the shared semaphore so the generation stage respects max_concurrent
common/utils/utils.go + utils_unix.go / utils_windows.go	Run subprocesses in their own process group (unix) and kill the whole group on context cancellation; stopping a task now terminates uv and its Python children together. Windows keeps exec default behavior (platform files are build-tag separated)

Testing

• Unit tests (mocked client): rate-limit error detection (429/“too many requests”/quota markers), Retry-After extraction, backoff schedule (8s-base exponential for rate limits, 1s for other errors), failure counter reset on success — all pass
• E2E simulation: 40 async tasks against a mock QPM=20 gateway (429 on excess) → 40/40 completed, only 4 rejections during initial window warm-up, no request storm
• Process-group test: cancelling RunCmdWithContext on a sh script that spawns a child sleep 300 → both parent and child killed, no orphan processes remain
• go build ./common/... and go test ./common/utils/ pass

Notes

• Rate-limit detection is intentionally marker-based (message text) rather than exception-type-based, so it works across OpenAI-compatible gateways that raise different exception types.
• The circuit breaker threshold (3) and backoff schedule are class attributes, easy to tune.

[boy-hack]

Thanks @NY1024 — this is a thorough, well-diagnosed fix and the problem description (request storm + orphaned Python process) matches what I'd expect from a QPM-throttled target. The process-group kill in Go and the semaphore around the baseline stage are both correct. A few notes before merge:

Bug: the circuit breaker never trips
The rate-limit counter is incremented on the OpenaiAlikeModel instance (self._consecutive_rate_limit_failures += 1 in openailike.py), but the circuit breaker in RedTeamer._a_attack reads a different counter — one stored on the RedTeamer instance (getattr(self, "_consecutive_rate_limit_failures", 0)). The RedTeamer counter is only ever reset to 0 on success and is never incremented anywhere, so consecutive_failures is always 0 and the breaker can never trigger. The two need to share state: either expose the counter from the model instance (e.g. model_callback wrapping the model, or have _a_attack read it off the model object), or move the increment into _a_attack itself.

Minor / non-blocking

• _retry_after_seconds reads getattr(exc, "retry_after", None) — the OpenAI SDK puts that on the exception's response/headers, not as a top-level attribute, so it'll almost always fall back to the computed schedule. Worth confirming against the SDK version you're pinning; if it's never populated, a plain min(schedule, max_wait_seconds) is clearer.
• RATE_LIMIT_MARKERS includes the bare substring "429" and "qpm"; a benign error message containing "429" anywhere (e.g. a request ID) would be misclassified as rate-limited and get the long backoff. Low risk, but substring matching on digits is a little loose.
• On Windows, setProcessGroup/killProcessGroup are no-ops, so the orphan-process problem remains on that platform. Fine if Windows isn't a supported eval target, but worth a one-line note in the PR/README.

Once the counter sharing is fixed (and ideally a unit test asserting the breaker trips after N consecutive 429s), this is good to go. Nice work on the Go side especially.

[boy-hack]

Thanks @NY1024 — this is a thorough, well-reasoned fix and the marker-based rate-limit detection (rather than exception-type based) is the right call for OpenAI-compatible gateways that raise heterogeneous errors. The unit test suite for _is_rate_limit_error / _retry_after_seconds / breaker shared-state is genuinely good coverage.

A few points to confirm / consider (none blocking):

1. Process-group termination (utils_unix.go) — The Setpgid: true + syscall.Kill(-pid, SIGKILL) approach is correct for killing the uv→Python orphan tree on Linux/macOS. Two small notes:

   • cmd.Cancel now does both killProcessGroup and cmd.Process.Kill(). Since the child is in its own group, Kill() on the direct child is redundant but harmless; fine.
   • cmd.WaitDelay = 5 * time.Second means after context cancellation the command can linger up to 5s while stdio drains. For a "stop task" UX that's a ~5s tail where the UI may still show running — acceptable, just flagging so it's intentional.
   • Windows degrades to exec.CommandContext default (only direct child) — you've documented this in README and the code comment, so no surprise there.

2. Circuit-breaker coupling via model_callback.__self__ — This is elegant but fragile: it only works if the callback is a bound method of the OpenaiAlikeModel instance. If anyone wraps it (e.g. functools.partial, a lambda, or an adapter), __self__ resolves to the wrapper and the breaker silently no-ops (returns 0) — it degrades gracefully (no crash), but the protection would silently vanish. A short comment near _get_consecutive_rate_limit_failures documenting the "must be a bound method of the model instance" contract would prevent a future regression.

3. .gitignore change — Switching tests → /tests so AIG-PromptSecurity/tests/ is now tracked is the right move (it lets the new test file live there). Just double-check no large/fixture-laden tests/ dir gets pulled into the repo unexpectedly.

4. CI is showing pending at the head commit — can you confirm go build ./common/... + go test ./common/utils/ and the Python pytest tests/ actually pass in CI (rather than just locally)? The Go process-group kill is the kind of thing that's easy to verify locally but needs the CI job to actually run it.

Overall this is solid and ready once CI is green and point 2's contract is documented. Nice work on the request-storm root cause analysis.

[boy-hack]

Thanks for this — it's a well-scoped fix for a real problem (the request storm during rate limiting is nasty), and the design is sound. A few notes before merge:

Strengths

• Rate-limit detection is marker-based and deliberately avoids a bare 429 substring match (req_429ab3c / 4290ms are correctly excluded) — good call.
• Retry-After / retry-after-ms / x-ratelimit-reset-requests extraction with a sanity cap (≤600s) and graceful fallback to the 8s base is solid.
• The circuit breaker sharing its counter with the retry loop via the model instance (model_callback.__self__) is clean, and it degrades gracefully when the callback isn't a bound method (returns 0 → breaker off).
• Process-group kill on Unix (Setpgid + Kill(-pid, SIGKILL)) properly terminates uv and its Python children; WaitDelay is a nice safety net.
• Tests are thorough: detection, backoff schedule, Retry-After extraction, and breaker trip/reset/threshold cases are all covered.

Minor / needs confirmation

1. import re is done inside _is_rate_limit_error and _retry_after_seconds rather than at module top — hoist it for consistency and to avoid the per-call import.
2. The Windows path leaves setProcessGroup/killProcessGroup as no-ops, so orphan Python workers can still run briefly after stop. The PR description already calls this out and the README now warns users to prefer Linux/macOS or Docker — acceptable given exec.Cmd doesn't expose job objects. Just confirming it's intentional and not a regression to fix now.
3. cmd.Cancel calls killProcessGroup and cmd.Process.Kill(). On Unix the group kill already includes the direct child, so the second Kill is redundant (harmless). Worth a one-line comment, or drop it.
4. Model pairing check: the breaker reads the counter from the attack-target model. In deepteam, model_callback passed to _a_attack is the target model, while the simulator uses a separate simulator_model instance. So the breaker only trips on target-side 429s and won't be falsely triggered by simulator throttling — please confirm that's the intended pairing, since sharing one model instance between simulator and target would cross-contaminate the counters.

None of these block merge. Net: approve with the import re nit. Looking forward to a quick confirmation on (4).

(Review comment only — not merging.)