Skip to content

Guardrails: make regex/LLM server-driven, combine custom guardrails per scope#163

Open
kowser-orkes wants to merge 9 commits into
mainfrom
feat/guardrail-server-driven
Open

Guardrails: make regex/LLM server-driven, combine custom guardrails per scope#163
kowser-orkes wants to merge 9 commits into
mainfrom
feat/guardrail-server-driven

Conversation

@kowser-orkes

@kowser-orkes kowser-orkes commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Describe the new behavior from this PR, and why it's needed

  • Fix: RegexGuardrail.Create / LLMGuardrail.Create no longer evaluate client-side — they're
    data-only, and the Conductor server evaluates them natively (regex → GraalJS, LLM → its own
    configured providers)
  • Removes the client-side LLM guardrail HTTP path (HttpClient, OPENAI_API_KEY), including a
    broken Anthropic branch that sent OpenAI-shaped requests to api.anthropic.com and always failed
  • Custom [Guardrail] guardrails now combine into one worker per agent/tool scope
    ({scope}_output_guardrail) instead of one worker per guardrail, removing a same-name worker
    collision hazard
  • Centralizes validation into GuardrailDef.Validate, closing a bypass where
    GuardrailRegistry.FromInstance skipped validation entirely
  • Adds a dedup guard on worker registration (keyed by task type + domain) so no worker type can be
    registered twice
  • Adds e2e coverage proving regex/LLM guardrails register no local worker, and that retry escalation
    actually fails a run instead of exhausting max turns
  • Breaking: LLMGuardrail.Create drops its apiKey parameter; custom guardrail worker task
    names change from the guardrail's own name to {scope}_output_guardrail
Before After
Regex / LLM guardrails evaluated client-side, one worker per guardrail evaluated server-side, no client worker
LLM guardrail HTTP path HttpClient + OPENAI_API_KEY; Anthropic branch always failed removed — server calls the model directly
Custom guardrail workers one per guardrail, name collisions possible one per agent/tool scope, deduped
GuardrailRegistry.FromInstance validation skipped entirely runs GuardrailDef.Validate

Issue #

Alternatives considered

Describe alternative implementation you have considered

  • Keep LLMGuardrail.Create's apiKey parameter for backward compatibility — dropped; the agent
    SDK has only ever shipped in the unreleased 3.0.0-rc2 tag, so there's no compatibility to
    preserve, and guardrails are meant to be server-driven end to end
  • Keep one worker per guardrail — dropped in favor of the combined per-scope model already used by
    the Java/Python/TS SDKs

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unittests 3.31% <ø> (+0.81%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Kowser added 5 commits July 21, 2026 18:35
- GuardrailDef: + Patterns/Mode/Message (regex), Model/Policy/MaxTokens (llm)
- ValidatePositionOnFail -> GuardrailDef.Validate: name, maxRetries, type-specific fields
- validate on every construction path incl. GuardrailRegistry.FromInstance (was skipped)
- RegexGuardrail.Create / LLMGuardrail.Create: data-only — no Handler, compiled
  Regex, HttpClient, or OPENAI_API_KEY; server evaluates natively
- SerializeGuardrail(g, scopeName): emits guardrailType + data; taskName only
  when a Handler exists — external guardrails no longer get one
- update serializer test asserts + stale api-key comments in two examples
- one {scope}_output_guardrail worker per agent/tool runs all local guardrails
  in declaration order, first failure wins
- handler exceptions propagate to the worker runner (were converted to a result)
- AddWorker: dedup on (TaskType, Domain) across all registration paths — no
  double-polling when the same agent/tool/guardrail registers twice
- ToolGuardrail_BlocksViolatingCall_RunFails: real GuardrailDef must block the
  call and fail the run (old test attached no GuardrailDef)
- Regex/Llm *_ServerEvaluated_NoLocalWorkerRegistered via new
  AgentRuntime.WorkerForTesting seam
- Agent/ToolGuardrail_RetryEscalatesToRaise_AfterMaxRetries; tool variant gated
  on E2E_TOOL_GUARDRAIL_ESCALATION until the server-side iteration-ref fix
- rename RegexGuardrail_FunctionBodyExecutes -> CustomGuardrail_DotNetRegexCheck_BlocksPii
- CHANGELOG: regex/llm server-evaluated (no worker, no api key); custom
  guardrails one worker per scope; LLMGuardrail.Create drops apiKey
- writing-agents.md: server-evaluated vs custom section
- api-reference.md: drop apiKey from LLMGuardrail.Create
@kowser-orkes kowser-orkes changed the title Feat/guardrail server driven Guardrails: make regex/LLM server-driven, combine custom guardrails per scope Jul 22, 2026
@kowser-orkes
kowser-orkes marked this pull request as draft July 22, 2026 17:02
Kowser added 4 commits July 22, 2026 18:22
The server's GET /agent/{id}/status response has no "error" or "reason"
field — only "reasonForIncompletion". AgentResult.Error and
AgentStatus.Reason read the wrong key and were always null on any
failed or terminated run, so PrintResult() silently showed a blank
result instead of the actual failure message.

Fixes BuildResult (feeds AgentResult.Error, used by RunAsync/WaitAsync)
and both GetStatusAsync overloads (feed AgentStatus.Reason). Also wires
up AgentStatus.Output, which neither GetStatusAsync populated despite
the record already having the field.
Stubs GET /agent/{id}/status via the existing HttpMessageHandler
pattern (OrkesAgentClientTests) and drives it through both real entry
points: AgentRuntime.GetStatusAsync and AgentHandle.WaitAsync. Covers
FAILED with reasonForIncompletion set, COMPLETED with none (regression
guard against over-firing), and Output being populated.

Verified these 3 of 5 tests fail without the previous commit's fix.
The existing 3.7 test already drives a run to Failed/Terminated via
guardrail retry escalation — it just never checked Error. One added
assertion turns it into a live counterfactual for the status-mapping
fix as well.
Java's buildResult only reads reasonForIncompletion when
status != COMPLETED, and aggregates ToolCalls by walking the
workflow's tasks (extractFromTasks: one entry per call_* task, name
= taskType, args = inputData with internal runtime fields stripped,
result = outputData.result). The previous commit's fix read
reasonForIncompletion unconditionally and never populated ToolCalls
at all — the property existed on AgentResult but nothing set it.

Adds IAgentClient.GetWorkflowWithTasksAsync (GET /workflow/{id}
?includeTasks=true, enrichment read, null on failure — mirrors the
existing GetWorkflowAsync/GetExecutionAsync pattern) and a new
BuildResult.ExtractToolCalls step. Gates Error/Reason on non-Completed
status in BuildResult and both GetStatusAsync overloads.

8 new unit tests: the Completed-with-stray-reasonForIncompletion case
(defensive gate, not just field-absent), ToolCalls extraction with
internal fields stripped, and a no-matching-tasks regression guard.
@kowser-orkes
kowser-orkes force-pushed the feat/guardrail-server-driven branch from 62f873a to aacb256 Compare July 23, 2026 07:05
@kowser-orkes
kowser-orkes marked this pull request as ready for review July 23, 2026 07:05
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.

2 participants