Conversation
…concurrent prompts from cancelling each other Two independent AI-engine reliability fixes: (a) Guardrail retry guidance. When a network action is denied because only a different form of the host is in scope (an IP was used but the hostname is allowed, or vice versa), the model retried the SAME denied target on a loop. Add concise guidance to the <guardrails> prompt: never repeat a denied target; if the denied value was an IP retry the in-scope hostname (and vice versa), otherwise pick a different in-scope target. (b) Concurrent-prompt cancellation. A permission/deny prompt and a multi-choice follow-up prompt can be outstanding at the same time. `_expire_stale_pending` (run when any new pending prompt is built) expired BOTH the `follow_up` and `permission` pending docs indiscriminately, so building one prompt flipped the other to `timed_out` — orphaning it so it stopped awaiting an answer. Scope the expiry to the incoming prompt's own type: a permission prompt now only supersedes a prior pending permission, a follow_up only a prior follow_up. The same-type supersede (recovering a dead worker's stale prompt) is preserved. Updates the test that codified the old cross-type behaviour and adds tests proving the two prompt types no longer cancel each other. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: freelabz/secator/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. WalkthroughThe change scopes stale pending-prompt expiry to the incoming prompt type, preserves legacy blanket expiry, adds tests for both behaviors, and updates guidance for denied network actions. ChangesPrompt expiry scoping
Denied network action guidance
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit watched the prompts align Comment |
|
Superseded by #1415 (consolidated). |
…face scope in prompt (#1415) Consolidates three AI-engine improvements into one PR. **Supersedes #1413 and #1414** (both closed in favor of this). - **Parse text tool-calls.** Some models emit tool calls as `<tool_call><function=…><parameter=…>` XML in message *content* instead of the API's structured tool-call field. `parse_text_tool_calls()` recovers those blocks, dispatches them like real tool calls, and strips the XML from the displayed content so the transcript stays clean. (was #1413) - **Guardrail retry + concurrent-prompt fixes.** (a) Prompt guidance so a model whose target was denied for being out of scope does not repeat the identical denied target — it retries the in-scope host/IP form or picks a different in-scope target. (b) `_expire_stale_pending` now scopes expiry to the *same* prompt type, so a permission prompt no longer cancels a concurrently-pending follow_up (and vice versa). (was #1414) - **Surface authorized scope in the prompt.** Inject the run's in-scope (and out-of-scope, if present) targets into the system prompt via a `<scope>` block so the model knows the allowed scope up front — reducing guardrail-denied retries. Omitted entirely when no scope is configured. Tests: AI prompt / utils / interactivity unit suites pass together (131 passed for the touched files; new `TestScopeInPrompt` asserts scope appears when provided and is absent otherwise). The 6 pre-existing `test_ai_guardrails.py::TestEdgeCases` redirect-parsing failures are unrelated (present on `main`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Two small, independent AI-engine reliability fixes.
(a) Guardrail retry guidance
When a network action is denied because only a different form of the host is in scope (the model used a raw IP but only the hostname is allowed, or vice versa), the model kept retrying the same denied target in a loop, burning turns.
Added concise guidance to the
<guardrails>prompt (secator/ai/prompts/constraints/guardrails.txt):(b) Concurrent-prompt cancellation
Report: a user received a multi-choice follow-up prompt AND a permission/deny prompt at the same time; answering the deny prompt caused the follow-up prompt to stop awaiting an answer (orphaned).
Root cause: in the remote (DB-polling) backend,
_expire_stale_pendingis called whenever a new pending prompt is built (build_pending_prompt), to supersede a prior prompt left behind by a dead worker. But it expired bothfollow_upandpermissionpending docs indiscriminately. A permission and a follow-up prompt can legitimately be outstanding at the same time, so building/answering one flipped the other totimed_out— orphaning it. The existing testtest_build_pending_prompt_expires_prior_pendinghad even codified this cross-type expiry.Fix (smallest safe correction): scope the expiry to the incoming prompt's own type. Building a permission prompt now only supersedes a prior pending permission; building a follow-up only supersedes a prior follow_up. The intended same-type supersede (recovering a stale prompt from a mid-poll worker crash) is preserved;
_expire_stale_pending()with no type still expires both (legacy callers).Residual (documented, out of scope here): the answer-routing that resolves "the latest pending prompt" for a session lives in the API/persistence layer, not this engine. Poll correlation in this repo is already scoped per prompt via
extra_data.prompt_uuid, so within the engine the two prompts now poll independently. A durable belt-and-suspenders follow-up (already FLAGged in the code) is a DB-layer TTL index on pending prompt docs.Tests
tests/unit/test_ai_interactivity.py: replaced the test asserting cross-type expiry; added tests that a permission build does not expire a pending follow_up, a follow_up build does not expire a pending permission, and that the legacy no-arg call still expires both. Full AI interactivity + prompts suites green. (6 unrelatedtest_ai_guardrails.py::TestEdgeCasesredirect-detection failures are pre-existing in this environment — they need theshfmt/safecmdshell parser and fail identically on a pristine tree.)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
AI Guidance